From 85309720453dfd9cfc8d1f790731b96ec1359af9 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Mon, 7 Sep 2026 09:51:01 -0700 Subject: [PATCH] fix(host-stats): reset delta bases and live caches on stop so restarts take the null-safe first-sample path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The subscriber-gated host-stats collectors previously cleared only their timers in the stop path (Node `stop()` in `server/host-stats/service.ts`, Rust `set_active(false)` in `crates/freshell-server/src/host_stats.rs`): the `prevCpu`/`prevDarwinCpu`/`prevVmstat`/`prevDisks`/`prevNet` (Rust: `prev_cpu`/`prev_vmstat`/`prev_disks`/`prev_net`) cumulative-counter delta bases and the merged live cache (including the slow-owned diskIo/network/ limits sections) survived a stop. When the last System Status viewer left and a viewer returned later, the restart's immediate tick computed CPU, paging, OOM, and network deltas across the whole unwatched interval instead of the documented null-safe first sample, and stale slow data was served until the first new slow tick. Both stop paths now additionally null every delta-base store and reset the merged live cache to the documented fresh-subscriber zero shape (`zeroLive(machine)` / `zero_live(&machine)`), and the Rust side clears queued drift samples (matching Node's histogram disable+null). The request-owned refresh/manual cache is deliberately retained on both sides. First-start and continuous-running behavior is unchanged, and the module headers in both languages now document the restart semantics. Each side's restart test asserts restarted VALUES, not reader call counts: run normally, stop for a simulated unwatched interval with the cumulative counters grown, restart, and assert the null-safe first sample (zero rates/ deltas with totals carried from the current sample, slow sections unavailable), then grow once more and assert later ticks re-base on the restart sample only (Node exact fake-timer values for cpu/paging/disk/net; Rust exact zero shapes plus dt-free witnesses: `oom_kills_delta == 1`, `rx_errors_delta == 1`, `weighted_await_ms == Some(8.0)`). Rust fmt/clippy evidence is carried in the task record because the QA script's `set -e` short-circuits its cargo gates on a known baseline-identity exit; the full-suite gate receipt for this commit is recorded by the factory. Darkforge run ex4c: plan and delta Fresh Eyes loops each PASSED on round 1 (fresh context, same model); task, fix, and whole-branch reviews approved. ## Plan # Host-Stats Restart Fresh-Start (prev-\*/slow-cache reset on stop) Implementation Plan > **For agentic workers:** Execute this plan task by task with a fresh > implementer and a specification-plus-quality review after every task. Track > progress with the checkbox steps below. ## User Request ### Requested result `server/host-stats/service.ts:290` (`crates/freshell-server/src/host_stats.rs:840-844` in the Rust mirror): `stop()` clears timers but retains `prevCpu`, `prevVmstat`, `prevDisks`, `prevNet` and the slow-section cache. When the last viewer leaves and a viewer returns later, the "immediate" tick computes CPU, paging, OOM, and network deltas across the whole unwatched interval (instead of the documented null-safe first sample), and stale slow data is reported until its timer runs. Clear the prev-* sample stores and slow cache in `stop()` so a restart behaves like a fresh start; assert restarted VALUES (not just reader call counts) in the restart tests; mirror in Rust. ### Explicit constraints - Clear the prev-* sample stores and the slow-section cache in the stop path so a restart behaves like a fresh start. - Assert restarted VALUES (not just reader call counts) in the restart tests. - Mirror the fix in the Rust mirror implementation. - Do not change normal first-start or continuous-running behavior; preserve the documented null-safe first-sample semantics. ### Accepted tradeoffs and residuals - None stated in the source. (Surfaced by an independent review round during the sidebar-status-sort run, PR #710 review cycle; filed as its own item.) **Goal:** A host-stats restart after the last viewer leaves reports the documented null-safe first sample (zero rates/deltas, carried totals, unavailable slow sections) instead of deltas spanning the unwatched interval, on both the Node and Rust servers. **Architecture:** On the stop edge, in addition to halting all cadence work, drop every cumulative-counter delta base (all five `prev*` stores on Node / all four `prev_*` stores plus queued drift samples on Rust) and reset the merged live snapshot cache to the documented fresh-subscriber zero shape (`zeroLive(machine)`). Each server's restart test stops a populated service, grows the cumulative counters while unwatched, restarts, and asserts the restarted snapshot VALUES — the immediate tick's null-safe shape, the absence of stale slow sections, and a positive re-base proof that the next tick deltas against the restart sample only. **Tech Stack:** TypeScript/Node (Express server, Vitest with full-module `vi.mock` of `server/host-stats/readers.js` + `vi.useFakeTimers()` driving intervals and `Date.now()`); Rust (tokio collector, path-injected pure readers in `freshell-platform`, real short cadences 25/50 ms in tests, `#[tokio::test]`). ## Global Constraints - **Node↔Rust parity is contractual** (`crates/freshell-server/src/host_stats.rs:1-8`, `crates/freshell-ws/src/host_stats_collector.rs:11-16`, plan-history Task 9): the lifecycle fix lands in BOTH implementations; never change only one side. `crates/freshell-ws` trait/dispatch (`set_active` cardinality edges) is untouched. - **Preserve documented first-start/continuous behavior**: `start()`/`set_active(true)` still run exactly ONE immediate fast tick; the slow tier still only ticks on its interval; readers are never called while stopped; `stop()`/`set_active(false)` remain idempotent and true zero-cost; the `refresh()` manual cache (`manualCache`/`manualAt` on Node, `manual` on Rust) is request-owned, not sampled state, and is intentionally retained across stop. - **Server code is NodeNext/ESM**: relative imports keep `.js` extensions (no new imports are expected). - **Node tests** run under `config/vitest/vitest.server.config.ts` (`sequence.shuffle: true` — tests must be order-independent) via the repo-owned path `npm run test:vitest -- ...`; ambient env stripping is handled by `config/vitest/sanitize-test-env.ts` (no manual env prefix). - **Rust tests** use real short cadences; tokio `time::pause/advance` is forbidden (no `test-util` feature). Workspace `rust-version = "1.96"`; installed toolchain is 1.96.0. std Mutexes are never held across `.await` (ticks are sync); the stop branch already holds `cadence` while taking the disjoint `prev_*`/`live`/`lag_samples` mutexes — preserve that lock order. - **TDD**: every task writes the failing behavioral test first and watches it fail for the intended reason (the retained prev/live state), then implements, then re-runs; never reduce existing coverage or skip tests. - **Factory gate**: the full QA gate is `darkforge qa` (full command `/home/dan/.local/share/darkforge/sources/freshell-qa.sh`) run against an exact clean commit. The completed baseline ledger records three PRE-EXISTING failure identities in `test/unit/server/fresh-agent/codex-adapter.test.ts` at base 66cf6f8987f286768dc27057e8625cccf86ebdfc; a nonzero gate exit is acceptable only when every parsed failure identity matches that ledger exactly. Those failures are not to be repaired to force green. --- ### Task 1: Node `HostStatsService.stop()` clears delta bases and the live cache; restart-values test **Files:** - Modify: `server/host-stats/service.ts:291-300` (`stop()`), `server/host-stats/service.ts:12-14` (module header lifecycle sentence) - Test: `test/unit/server/host-stats/service.test.ts` (add T2/T3 fixtures near line 129, add restart-values test in `describe('start/stop (contract points 1, 5)')` after line 336, correct one stale test title at line 365) **Interfaces:** - Consumes: existing `zeroLive(this.machine)` (`service.ts:182-195`); existing private fields `prevCpu`/`prevDarwinCpu`/`prevVmstat`/`prevDisks`/`prevNet` (`service.ts:236-240`); `liveCache` (`service.ts:226`). - Produces: no new exported interfaces. Behavioral contract change only: `stop()` additionally resets all five prev stores to `null` and `liveCache` to `zeroLive(this.machine)`, so `getSnapshot()` while stopped and at the next restart's immediate tick returns the documented fresh-subscriber shape (pre-start shape: all sections `available:false` with `machine` filled — pinned by `docs/plans/2026-08-25-host-pressure-pane.md:454`). - [ ] **Step 1: Write the failing behavioral test** Add the grown-counter fixtures next to `CPU_T1`/`VMSTAT_T1` (after `test/unit/server/host-stats/service.test.ts:129`): ```ts const CPU_T2 = { total: 6000, busy: 600, steal: 60, perCore: [ { total: 1500, busy: 150 }, { total: 1500, busy: 150 }, { total: 1500, busy: 150 }, { total: 1500, busy: 150 }, ], } const CPU_T3 = { total: 8000, busy: 1200, steal: 100, perCore: [ { total: 2000, busy: 300 }, { total: 2000, busy: 300 }, { total: 2000, busy: 300 }, { total: 2000, busy: 300 }, ], } // T2->T3 over one 2s fast tick: dBusy 600/dTotal 2000 = 30%; dSteal 40/2000 = 2%; // per-core 150/500 = 30%; swap in 24*4/2 = 48 KB/s; out 12*4/2 = 24 KB/s; // majfaults 40/2 = 20/s; oom 9->11. const VMSTAT_T2 = { pswpin: 200, pswpout: 80, pgmajfault: 100, oomKill: 9 } const VMSTAT_T3 = { pswpin: 224, pswpout: 92, pgmajfault: 140, oomKill: 11 } ``` Add this test immediately after the existing restart test (after `service.test.ts:336`), in the same `describe('start/stop (contract points 1, 5)')` block: ```ts it('restart after stop re-bases every delta family and resets the live cache (fresh-start restart)', () => { // Cycle 1: run the default T0 fixtures long enough for every prev-* store // AND the slow-owned liveCache keys to populate (fast ticks at 2/4/6s, the // first slow tick at 5s). const service = makeService() service.start() vi.advanceTimersByTime(6000) expect(service.getSnapshot().live.diskIo.available).toBe(true) // slow cache populated pre-stop service.stop() // A stopped service publishes the documented fresh-subscriber shape again: // no stale slow sections survive a stop. const stopped = service.getSnapshot().live expect(stopped.diskIo.available).toBe(false) expect(stopped.network.available).toBe(false) expect(stopped.limits.available).toBe(false) // The cumulative counters GROW while the service is unwatched — the // long-window bug case (values below are all grown from the T0 cycle). vi.advanceTimersByTime(20000) // Fake timers advance Date.now(), the dt basis. vi.mocked(readersMock.readCpuTimes).mockReturnValue(CPU_T2) vi.mocked(readersMock.readVmstat).mockReturnValue(VMSTAT_T2) service.start() // The restart's immediate tick is the documented null-safe first sample: // zero rates/deltas, totals carried from the CURRENT (T2) sample, and the // slow sections stay unavailable until their first new slow tick. const restarted = service.getSnapshot().live expect(restarted.cpu).toEqual({ available: true, usagePct: 0, stealPct: 0, perCorePct: [0, 0, 0, 0], freqMHz: null }) expect(restarted.paging).toEqual({ available: true, swapInKbps: 0, swapOutKbps: 0, majFaultsPerSec: 0, oomKillsDelta: 0, oomKillsTotal: 9 }) expect(restarted.diskIo.available).toBe(false) expect(restarted.network.available).toBe(false) expect(restarted.limits.available).toBe(false) // The next tick deltas against the RESTART sample (T2), never against the // pre-stop sample (T0): every value below derives from T2->T3 over dt=2s. vi.mocked(readersMock.readCpuTimes).mockReturnValue(CPU_T3) vi.mocked(readersMock.readVmstat).mockReturnValue(VMSTAT_T3) vi.advanceTimersByTime(2000) const live = service.getSnapshot().live expect(live.cpu).toEqual({ available: true, usagePct: 30, stealPct: 2, perCorePct: [30, 30, 30, 30], freqMHz: null }) expect(live.paging).toEqual({ available: true, swapInKbps: 48, swapOutKbps: 24, majFaultsPerSec: 20, oomKillsDelta: 2, oomKillsTotal: 11 }) }) ``` Also correct one now-stale test title in the same file: the histogram lifecycle test at `service.test.ts:365` is currently titled `'collects no lag samples while stopped (cache retains last tick), then resumes per-tick on restart'`. The `liveCache` no longer retains the last tick across `stop()`; retitle it (no assertion changes — its existing assertions still pass because the restart's immediate tick drains the re-enabled histogram): ```ts it('collects no lag samples while stopped, then resumes per-tick on restart', () => { ``` - [ ] **Step 2: Run the test and verify the intended failure** Run: `npm run test:vitest -- run test/unit/server/host-stats/service.test.ts --config config/vitest/vitest.server.config.ts` Expected: FAIL because `stop()` retains all prev stores and the `liveCache`: the new test's `stopped.diskIo.available === false` assertion fails first (the pre-stop slow sections are still there), and if that were fixed alone, the restart assertions would still fail with gap-spanning values — pre-fix restart values WOULD be `cpu.usagePct = (600-100)/(6000-1000)*100 = 10` (not 0), `paging.swapInKbps = (200-100)*4/20 = 20` (not 0), `oomKillsDelta = 9-2 = 7` (not 0). All failures trace to the retained prev-*/live state, never to a syntax or setup accident. - [ ] **Step 3: Add the minimal production implementation** Replace `stop()` at `server/host-stats/service.ts:291-300` with: ```ts stop(): void { if (!this.running) return // idempotent this.running = false if (this.fastTimer) clearInterval(this.fastTimer) this.fastTimer = undefined if (this.slowTimer) clearInterval(this.slowTimer) this.slowTimer = undefined this.histogram?.disable() this.histogram = null // Restart = fresh start (contract point 8: what a fresh subscriber // receives): drop every cumulative-counter delta base so the restart's // immediate tick takes the documented null-safe first-sample path instead // of spanning the whole unwatched interval, and reset the merged live // cache so no stale slow sections survive. the refresh-owned manualCache / // manualAt are request state, not sampled state, and are retained. this.prevCpu = null this.prevDarwinCpu = null this.prevVmstat = null this.prevDisks = null this.prevNet = null this.liveCache = zeroLive(this.machine) } ``` (`prevDarwinCpu` is cleared alongside the four named stores: it is the same delta-base class for the darwin fast path, and leaving it would reintroduce the identical stale-delta bug on darwin.) Update the module header lifecycle sentence at `server/host-stats/service.ts:12-14` to describe the new stop semantics: ```ts * start() runs ONE immediate fast tick (a fresh subscriber gets a shaped snapshot at once); * the slow tier only ticks on its own interval. stop() halts ALL collection (true zero * cost) and clears every delta base + the merged live cache, so a restart takes the * null-safe first-sample path like a fresh start. getSnapshot() never blocks on I/O — * ticks write caches, snapshots read caches. ``` - [ ] **Step 4: Run the focused test** Run: `npm run test:vitest -- run test/unit/server/host-stats/service.test.ts --config config/vitest/vitest.server.config.ts` Expected: PASS (the new restart-values test and every pre-existing test in the suite; pre-fix behavior was never pinned by any other assertion). - [ ] **Step 5: Refactor while green** No refactor expected: the nine added statements are the single clearing site, `zeroLive(this.machine)` reuses the existing constructor zero-shape, and the two delta families share no new abstraction. During the green pass, only re-derive anything if the implementer finds the clearing duplicated elsewhere (it is not; `stop()` is the only writer of these nulls). - [ ] **Step 6: Run impacted-test verification** The change alters what `HostStatsService.stop()` clears; the consumers are the host-stats unit suites (service + readers share the module mock patterns), the ws integration suite that runs the REAL service over the fixture tree (`test/server/ws-hoststats.test.ts` — subscribe/unsubscribe/stop lifecycle and snapshot schema/shape assertions), and nothing else (client mapping tests and e2e consume only snapshots, whose wire shape is unchanged). Run: `npm run test:vitest -- run test/unit/server/host-stats test/server/ws-hoststats.test.ts --config config/vitest/vitest.server.config.ts` Expected: PASS - [ ] **Step 7: Commit the task** ```bash git add server/host-stats/service.ts test/unit/server/host-stats/service.test.ts git commit -m "fix(host-stats): reset delta bases and live cache on stop() so restarts take the null-safe first-sample path" ``` --- ### Task 2: Rust `HostStatsCollectorService::set_active(false)` mirror (clear prev_\*, drift samples, live cache) + restart-values test **Files:** - Modify: `crates/freshell-server/src/host_stats.rs:840-844` (stop branch of `set_active`), `crates/freshell-server/src/host_stats.rs:21` (module header sentence) - Test: `crates/freshell-server/src/host_stats.rs` (in-file `#[cfg(test)] mod tests`: add `rewrite_counter` helper + one new `#[tokio::test]` after `host_stats_set_active_spawn_abort_lifecycle`, i.e. after line 2151) **Interfaces:** - Consumes: `zero_live(&self.ctx.machine)` (free fn at `host_stats.rs:1412-1425`, already used at construction, `:239`); `Share` mutexes `prev_cpu`/`prev_vmstat`/`prev_disks`/`prev_net`/`lag_samples`/`live` (`host_stats.rs:164-182`); test helpers `test_collector`, `copy_tree`, `write_rel` (`:1505`, `:1534`, `:1661`), `wait_until` (`:1518`), `fixtures()`/`proc_fixture()`/`sys_fixture()` (`:1476-1487`), fixture bytes `crates/freshell-server/tests/fixtures/host-stats/proc/{stat,vmstat}`. - Produces: no new public interfaces. Behavioral contract change only: `set_active(false)` additionally nulls the four `prev_*` stores, clears `lag_samples`, and resets `live` to `zero_live(&self.ctx.machine)`, mirroring Task 1. The existing `host_stats_set_active_spawn_abort_lifecycle` test is untouched and still passes. - [ ] **Step 1: Write the failing behavioral test** Add the helper and test to the in-file test module immediately after `host_stats_set_active_spawn_abort_lifecycle` (after `crates/freshell-server/src/host_stats.rs:2151`): ```rust // ----------------------------------------------------------------- // Lifecycle (parity): restart after set_active(false) re-bases every // delta family and resets the live cache (restart = fresh start); // Node mirror: test/unit/server/host-stats/service.test.ts. // ----------------------------------------------------------------- /// Rewrite the first `name value` line inside a copied fixture file — the /// overlay tree's counters grow in place to simulate an unwatched /// interval (Node suite parity: re-seeded reader mocks). fn rewrite_counter(path: &Path, name: &str, value: u64) { let text = std::fs::read_to_string(path).unwrap(); let mut out = String::new(); let mut replaced = false; for line in text.lines() { if !replaced && line.starts_with(&format!("{name} ")) { out.push_str(&format!("{name} {value}\n")); replaced = true; } else { out.push_str(line); out.push('\n'); } } assert!(replaced, "counter `{name}` present in {}", path.display()); std::fs::write(path, out).unwrap(); } #[tokio::test] async fn host_stats_set_active_restart_rebases_deltas_and_resets_live_cache() { // Full tmpdir overlay: the config roots are immutable after // construction, so the fixture counters grow via file rewrites. let tmp = tempfile::tempdir().unwrap(); let proc_root = tmp.path().join("proc"); let sys_root = tmp.path().join("sys"); copy_tree(&proc_fixture(), &proc_root); copy_tree(&sys_fixture(), &sys_root); let interest = HostStatsInterestRegistry::default(); let collector = test_collector(proc_root.clone(), sys_root, &interest); // Cycle 1: run until every prev_* store AND the slow-owned live // sections have populated from the committed fixture counters. collector.set_active(true); assert!( wait_until(Duration::from_millis(500), || { collector.snapshot().live.disk_io.available }) .await, "slow sections populate before stop" ); collector.set_active(false); // A stopped collector publishes the documented fresh-subscriber shape // again — no stale slow sections survive a stop. let stopped = collector.snapshot(); assert!(!stopped.live.disk_io.available); assert!(!stopped.live.network.available); assert!(!stopped.live.limits.available); // The unwatched counters GROW: aggregate cpu line x10 (busy 78850 / // total 1742360) plus pswpin/pswpout/pgmajfault/oom_kill growth. let stat_path = proc_root.join("stat"); let stat = std::fs::read_to_string(&stat_path).unwrap(); let grown = stat.replacen( "cpu 4705 356 1622 164331 2020 80 345 777 0 0", "cpu 47050 3560 16220 1643310 20200 800 3450 7770 0 0", 1, ); assert!(grown != stat, "aggregate cpu line rewritten"); std::fs::write(&stat_path, grown).unwrap(); rewrite_counter(&proc_root.join("vmstat"), "pswpin", 11234); rewrite_counter(&proc_root.join("vmstat"), "pswpout", 15678); rewrite_counter(&proc_root.join("vmstat"), "pgmajfault", 1890); rewrite_counter(&proc_root.join("vmstat"), "oom_kill", 13); collector.set_active(true); let restarted = collector.snapshot(); // The restart's immediate tick is the documented null-safe first // sample: zero rates/deltas, totals carried from the CURRENT (grown) // sample, slow sections still unavailable. assert!(restarted.live.cpu.available); assert_eq!(restarted.live.cpu.usage_pct, 0.0); assert_eq!(restarted.live.cpu.steal_pct, Some(0.0)); assert_eq!(restarted.live.paging.swap_in_kbps, 0.0); assert_eq!(restarted.live.paging.swap_out_kbps, 0.0); assert_eq!(restarted.live.paging.maj_faults_per_sec, 0.0); assert_eq!(restarted.live.paging.oom_kills_delta, 0); assert_eq!(restarted.live.paging.oom_kills_total, 13); assert!(!restarted.live.disk_io.available); assert!(!restarted.live.network.available); assert!(!restarted.live.limits.available); // Positive re-base proof: bump ONLY oom_kill by 1; the next fast tick // reports delta 1 against the RESTART sample — never 11 against the // pre-stop sample. rewrite_counter(&proc_root.join("vmstat"), "oom_kill", 14); assert!( wait_until(Duration::from_millis(500), || { collector.snapshot().live.paging.oom_kills_delta == 1 }) .await, "paging deltas re-base on the restart sample" ); assert_eq!(collector.snapshot().live.paging.oom_kills_total, 14); collector.set_active(false); } ``` - [ ] **Step 2: Run the test and verify the intended failure** Run: `cargo test -p freshell-server --locked -- host_stats_set_active_restart_rebases_deltas_and_resets_live_cache --nocapture` Expected: FAIL because the stop branch retains `live` and the `prev_*` stores: the first failing assertion is `!stopped.live.disk_io.available` (the pre-stop slow sections survive today). With only that fixed, the restart assertions would still fail on gap-spanning values — pre-fix `usage_pct = (78850-7885)/(1742360-174236)*100 ≈ 4.53` (not 0), `oom_kills_delta = 13-3 = 10` (not 0), and the re-base tick would report `oom_kills_delta = 14-3 = 11` (not 1). All failures trace to the retained prev/live state, never to a setup accident. - [ ] **Step 3: Add the minimal production implementation** Replace the stop branch of `set_active` at `crates/freshell-server/src/host_stats.rs:840-844` with: ```rust } else if let Some(handles) = cadence.take() { handles.fast.abort(); handles.slow.abort(); handles.drift.abort(); // Restart = fresh start (Node stop() parity): drop every // cumulative-counter delta base so the restart's immediate tick // reports the null-safe first sample instead of deltas spanning // the whole unwatched interval; clear queued drift samples (Node // disables/nulls its histogram at stop); reset the merged live // cache so no stale slow sections survive. The `manual` refresh // cache is request-owned (not sampled state) and stays. Cadence // is locked first; these are disjoint mutexes — no lock cycle. *self.ctx.share.prev_cpu.lock().unwrap() = None; *self.ctx.share.prev_vmstat.lock().unwrap() = None; *self.ctx.share.prev_disks.lock().unwrap() = None; *self.ctx.share.prev_net.lock().unwrap() = None; self.ctx.share.lag_samples.lock().unwrap().clear(); *self.ctx.share.live.lock().unwrap() = zero_live(&self.ctx.machine); } ``` Update the module header sentence at `crates/freshell-server/src/host_stats.rs:21` to describe the new stop semantics: ```rust //! `set_active(false)` aborts ALL collection tasks (true zero cost) and clears //! every delta base + the merged live cache, so a restart takes the null-safe //! first-sample path like a fresh start (Node stop() parity). ``` - [ ] **Step 4: Run the focused test** Run: `cargo test -p freshell-server --locked -- host_stats_set_active_restart_rebases_deltas_and_resets_live_cache --nocapture` Expected: PASS; then the complete in-file host-stats module: `cargo test -p freshell-server --locked -- host_stats` — PASS. - [ ] **Step 5: Refactor while green** No refactor expected: the six added statements are the single clearing site and reuse the existing `zero_live` free fn. If `cargo fmt` reflows the new comment or match arm, accept its form (`cargo fmt -p freshell-server`), keeping the code green. - [ ] **Step 6: Run impacted-test verification** `set_active` is the `freshell-ws` trait method (`crates/freshell-ws/src/host_stats_collector.rs:61`) driven by the interest registry's cardinality edges in `crates/freshell-ws/src/terminal.rs`; the trait/dispatch contract is unchanged, so the impacted set is the whole `freshell-server` crate (host_stats.rs is a central in-crate module with a large test module) plus the ws-side host_stats tests (interest edges + the `set_active` fake-collector dispatch test). Run: `cargo test -p freshell-server --locked && cargo test -p freshell-ws --locked -- host_stats` Expected: PASS Rust format/lint evidence is verified explicitly here, NOT delegated to the QA gate: `freshell-qa.sh` runs `npm run check` under `set -euo pipefail` BEFORE its cargo gates, so a baseline-exception gate exit (the known codex-adapter identities) never reaches `cargo fmt`/`clippy` (load-bearing finding LB-1, `reports/load-bearing-finder.md`). Run the same gates the QA script runs: Run: `cargo fmt --all --check && cargo clippy --workspace --exclude freshell-tauri --all-targets -- -D warnings` Expected: PASS - [ ] **Step 7: Commit the task** ```bash git add crates/freshell-server/src/host_stats.rs git commit -m "fix(host-stats): mirror restart reset in the Rust collector set_active(false)" ``` --- ## Final gate (after both tasks, prospective landing) 1. Oracle check (work-source candidate verification, safe and relevant): `npm run check` — typecheck + coordinated full Vitest suite must PASS. 2. Full QA gate on the exact clean HEAD: `darkforge qa --repo '/home/dan/code/freshell' --id 'ex4c'` — exit 0, or exit 1 only with parsed failure identities exactly matching the baseline ledger (the three `test/unit/server/fresh-agent/codex-adapter.test.ts` identities; confirmed unrelated — host-stats touches no fresh-agent code). Note (LB-1): on a baseline-exception exit, `set -e` aborts `freshell-qa.sh` before its cargo fmt/clippy tail; those gates are therefore run explicitly in Task 2 Step 6 on the exact HEAD (`cargo fmt --all --check && cargo clippy --workspace --exclude freshell-tauri --all-targets -- -D warnings`, both proven green at base 66cf6f89 — evidence in `/load-bearing-ledger.md`). --- crates/freshell-server/src/host_stats.rs | 213 +++++++++++++++++++- server/host-stats/service.ts | 16 +- test/unit/server/host-stats/service.test.ts | 113 ++++++++++- 3 files changed, 339 insertions(+), 3 deletions(-) diff --git a/crates/freshell-server/src/host_stats.rs b/crates/freshell-server/src/host_stats.rs index 370d79045..39196c867 100644 --- a/crates/freshell-server/src/host_stats.rs +++ b/crates/freshell-server/src/host_stats.rs @@ -18,7 +18,9 @@ //! //! `set_active(true)` runs ONE immediate fast tick (a fresh subscriber gets a //! shaped snapshot at once); the slow tier only ticks on its own interval. -//! `set_active(false)` aborts ALL collection tasks (true zero cost). +//! `set_active(false)` aborts ALL collection tasks (true zero cost) and clears +//! every delta base + the merged live cache, so a restart takes the null-safe +//! first-sample path like a fresh start (Node stop() parity). //! `snapshot()` never blocks on I/O — ticks write caches, snapshots read //! caches. //! @@ -841,6 +843,20 @@ impl HostStatsCollector for HostStatsCollectorService { handles.fast.abort(); handles.slow.abort(); handles.drift.abort(); + // Restart = fresh start (Node stop() parity): drop every + // cumulative-counter delta base so the restart's immediate tick + // reports the null-safe first sample instead of deltas spanning + // the whole unwatched interval; clear queued drift samples (Node + // disables/nulls its histogram at stop); reset the merged live + // cache so no stale slow sections survive. The `manual` refresh + // cache is request-owned (not sampled state) and stays. Cadence + // is locked first; these are disjoint mutexes — no lock cycle. + *self.ctx.share.prev_cpu.lock().unwrap() = None; + *self.ctx.share.prev_vmstat.lock().unwrap() = None; + *self.ctx.share.prev_disks.lock().unwrap() = None; + *self.ctx.share.prev_net.lock().unwrap() = None; + self.ctx.share.lag_samples.lock().unwrap().clear(); + *self.ctx.share.live.lock().unwrap() = zero_live(&self.ctx.machine); } } } @@ -2190,6 +2206,201 @@ mod tests { collector.set_active(false); } + /// Rewrite the first `name value` line inside a copied fixture file — the + /// overlay tree's counters grow in place to simulate an unwatched + /// interval (Node suite parity: re-seeded reader mocks). + fn rewrite_counter(path: &Path, name: &str, value: u64) { + let text = std::fs::read_to_string(path).unwrap(); + let mut out = String::new(); + let mut replaced = false; + for line in text.lines() { + if !replaced && line.starts_with(&format!("{name} ")) { + out.push_str(&format!("{name} {value}\n")); + replaced = true; + } else { + out.push_str(line); + out.push('\n'); + } + } + assert!(replaced, "counter `{name}` present in {}", path.display()); + std::fs::write(path, out).unwrap(); + } + + // Full-file fixture rewrites for the restart rebase test. `lo`/headers/ + // partitions/`docker0` lines are always identical to the committed + // fixtures; only the named device counters grow. (Trailing newline each.) + + /// diskstats growth over the unwatched interval (every device grows). + const DISKSTATS_G1: &str = r" 8 0 sda 6000 150 440000 7000 2400 75 240800 4000 0 4500 9000 + 8 1 sda1 4000 80 300000 5000 1500 40 150000 2500 0 3000 7500 + 7 0 loop0 100 0 800 10 0 0 0 0 0 10 10 + 259 0 nvme0n1 9000 200 700000 8000 3000 60 300000 4000 0 5000 12000 + 259 1 nvme0n1p1 8000 150 600000 7000 2500 55 250000 3500 0 4500 10500 +"; + + /// diskstats after the rebase proof bump — only the sda line changes + /// (dReads 100, dReadMs 2000, dWrites 400, dWriteMs 2000, dIosMs 1000). + const DISKSTATS_G2: &str = r" 8 0 sda 6100 150 491200 9000 2800 75 343200 6000 0 5500 9000 + 8 1 sda1 4000 80 300000 5000 1500 40 150000 2500 0 3000 7500 + 7 0 loop0 100 0 800 10 0 0 0 0 0 10 10 + 259 0 nvme0n1 9000 200 700000 8000 3000 60 300000 4000 0 5000 12000 + 259 1 nvme0n1p1 8000 150 600000 7000 2500 55 250000 3500 0 4500 10500 +"; + + /// net/dev growth over the unwatched interval — only eth0 errs 7->17. + const NET_DEV_G1: &str = r"Inter-| Receive | Transmit + face |bytes packets errs drop fifo frame compressed multicast|bytes packets errs drop fifo colls carrier compressed + lo: 1000000 10000 0 0 0 0 0 0 1000000 10000 0 0 0 0 0 0 + eth0: 5000000 50000 17 3 0 0 0 0 8000000 80000 11 4 0 0 0 0 +docker0: 2000000 20000 2 1 0 0 0 0 3000000 30000 5 2 0 0 0 0 +"; + + /// net/dev after the rebase proof bump — only eth0 errs 17->18. + const NET_DEV_G2: &str = r"Inter-| Receive | Transmit + face |bytes packets errs drop fifo frame compressed multicast|bytes packets errs drop fifo colls carrier compressed + lo: 1000000 10000 0 0 0 0 0 0 1000000 10000 0 0 0 0 0 0 + eth0: 5000000 50000 18 3 0 0 0 0 8000000 80000 11 4 0 0 0 0 +docker0: 2000000 20000 2 1 0 0 0 0 3000000 30000 5 2 0 0 0 0 +"; + + #[tokio::test] + async fn host_stats_set_active_restart_rebases_deltas_and_resets_live_cache() { + // Full tmpdir overlay: the config roots are immutable after + // construction, so the fixture counters grow via file rewrites. + let tmp = tempfile::tempdir().unwrap(); + let proc_root = tmp.path().join("proc"); + let sys_root = tmp.path().join("sys"); + copy_tree(&proc_fixture(), &proc_root); + copy_tree(&sys_fixture(), &sys_root); + + let interest = HostStatsInterestRegistry::default(); + let collector = test_collector(proc_root.clone(), sys_root, &interest); + + // Cycle 1: run until every prev_* store AND the slow-owned live + // sections have populated from the committed fixture counters. + collector.set_active(true); + assert!( + wait_until(Duration::from_millis(500), || { + collector.snapshot().live.disk_io.available + }) + .await, + "slow sections populate before stop" + ); + collector.set_active(false); + // A stopped collector publishes the documented fresh-subscriber shape + // again — no stale slow sections survive a stop. + let stopped = collector.snapshot(); + assert!(!stopped.live.disk_io.available); + assert!(!stopped.live.network.available); + assert!(!stopped.live.limits.available); + + // The unwatched counters GROW: aggregate cpu line x10 (busy 78850 / + // total 1742360) plus pswpin/pswpout/pgmajfault/oom_kill growth, and + // the diskstats/net-dev G1 files below. + let stat_path = proc_root.join("stat"); + let stat = std::fs::read_to_string(&stat_path).unwrap(); + let grown = stat.replacen( + "cpu 4705 356 1622 164331 2020 80 345 777 0 0", + "cpu 47050 3560 16220 1643310 20200 800 3450 7770 0 0", + 1, + ); + assert!(grown != stat, "aggregate cpu line rewritten"); + std::fs::write(&stat_path, grown).unwrap(); + rewrite_counter(&proc_root.join("vmstat"), "pswpin", 11234); + rewrite_counter(&proc_root.join("vmstat"), "pswpout", 15678); + rewrite_counter(&proc_root.join("vmstat"), "pgmajfault", 1890); + rewrite_counter(&proc_root.join("vmstat"), "oom_kill", 13); + std::fs::write(proc_root.join("diskstats"), DISKSTATS_G1).unwrap(); + std::fs::write(proc_root.join("net/dev"), NET_DEV_G1).unwrap(); + + collector.set_active(true); + let restarted = collector.snapshot(); + // The restart's immediate tick is the documented null-safe first + // sample: zero rates/deltas, totals carried from the CURRENT (grown) + // sample, slow sections still unavailable. + assert!(restarted.live.cpu.available); + assert_eq!(restarted.live.cpu.usage_pct, 0.0); + assert_eq!(restarted.live.cpu.steal_pct, Some(0.0)); + assert_eq!(restarted.live.paging.swap_in_kbps, 0.0); + assert_eq!(restarted.live.paging.swap_out_kbps, 0.0); + assert_eq!(restarted.live.paging.maj_faults_per_sec, 0.0); + assert_eq!(restarted.live.paging.oom_kills_delta, 0); + assert_eq!(restarted.live.paging.oom_kills_total, 13); + assert!(!restarted.live.disk_io.available); + assert!(!restarted.live.network.available); + assert!(!restarted.live.limits.available); + + // Positive re-base proof #1: bump ONLY oom_kill by 1; the next fast + // tick reports delta 1 against the RESTART sample — never 11 against + // the pre-stop sample. + rewrite_counter(&proc_root.join("vmstat"), "oom_kill", 14); + assert!( + wait_until(Duration::from_millis(500), || { + let paging = &collector.snapshot().live.paging; + paging.oom_kills_delta == 1 && paging.oom_kills_total == 14 + }) + .await, + "paging deltas re-base on the restart sample (delta 1, total 14)" + ); + + // Slow tier (Finding 1 disposition): the FIRST post-restart slow tick + // is the null-safe first slow sample — zero rates/deltas, totals + // carried from the G1 growth (eth0 rx errs 7->17 plus docker0 2 = 19). + assert!( + wait_until(Duration::from_millis(500), || { + collector.snapshot().live.disk_io.available + }) + .await, + "first post-restart slow tick populates the slow sections" + ); + assert!( + wait_until(Duration::from_millis(500), || { + let live = &collector.snapshot().live; + live.disk_io.available + && live.disk_io.read_bps == 0.0 + && live.disk_io.write_bps == 0.0 + && live.disk_io.util_pct.is_none() + && live.disk_io.weighted_await_ms.is_none() + && live.network.available + && live.network.rx_bps == 0.0 + && live.network.tx_bps == 0.0 + && live.network.rx_errors_total == 19 + && live.network.tx_errors_total == 16 + && live.network.rx_dropped_total == 4 + && live.network.tx_dropped_total == 6 + && live.network.rx_errors_delta == 0 + }) + .await, + "first post-restart slow tick is the null-safe first slow sample with G1 totals" + ); + + // Positive re-base proof #2 (dt-free witnesses only): grow eth0 rx + // errs by exactly 1 (17->18) and grow ONLY sda's diskstats counters + // (G1->G2: dReads 100, dReadMs 2000, dWrites 400, dWriteMs 2000, + // dIosMs 1000; every other device unchanged so sda is the worst-util + // device). The next slow tick must report counts/ratios against the + // RESTART samples: rx_errors_delta == 1 (never 11) and + // weighted_await_ms == (2000+2000)/(100+400) == 8.0 (never the + // gap-spanning value). BEFORE finalizing, verify the exact field + // mapping, loopback/partition exclusion, and worst-device selection + // in freshell-platform/src/host_stats_readers.rs and adjust THESE + // COMMENTS/VALUES only if the code contradicts them (record any + // adjustment in your report). + std::fs::write(proc_root.join("diskstats"), DISKSTATS_G2).unwrap(); + std::fs::write(proc_root.join("net/dev"), NET_DEV_G2).unwrap(); + assert!( + wait_until(Duration::from_millis(2000), || { + let live = &collector.snapshot().live; + live.network.rx_errors_delta == 1 + && live.network.rx_errors_total == 20 + && live.disk_io.weighted_await_ms == Some(8.0) + }) + .await, + "network/disk re-base on the restart sample (dt-free witnesses)" + ); + collector.set_active(false); + } + #[test] fn host_stats_snapshot_zero_shape_before_first_tick() { let interest = HostStatsInterestRegistry::default(); diff --git a/server/host-stats/service.ts b/server/host-stats/service.ts index 827d2a674..e8b51eccc 100644 --- a/server/host-stats/service.ts +++ b/server/host-stats/service.ts @@ -11,7 +11,9 @@ * * start() runs ONE immediate fast tick (a fresh subscriber gets a shaped snapshot at once); * the slow tier only ticks on its own interval. stop() halts ALL collection (true zero - * cost). getSnapshot() never blocks on I/O — ticks write caches, snapshots read caches. + * cost) and clears every delta base + the merged live cache, so a restart takes the + * null-safe first-sample path like a fresh start. getSnapshot() never blocks on I/O — + * ticks write caches, snapshots read caches. * * refresh() (on-request manual data — process table, disks, inotify, thermals/battery) is * single-flight with a 1s post-completion cooldown (connection-agnostic, R3M6). Section @@ -297,6 +299,18 @@ export class HostStatsService { this.slowTimer = undefined this.histogram?.disable() this.histogram = null + // Restart = fresh start (contract point 8: what a fresh subscriber + // receives): drop every cumulative-counter delta base so the restart's + // immediate tick takes the documented null-safe first-sample path instead + // of spanning the whole unwatched interval, and reset the merged live + // cache so no stale slow sections survive. The refresh-owned manualCache / + // manualAt are request state, not sampled state, and are retained. + this.prevCpu = null + this.prevDarwinCpu = null + this.prevVmstat = null + this.prevDisks = null + this.prevNet = null + this.liveCache = zeroLive(this.machine) } isRunning(): boolean { diff --git a/test/unit/server/host-stats/service.test.ts b/test/unit/server/host-stats/service.test.ts index 376622213..7eebee8b6 100644 --- a/test/unit/server/host-stats/service.test.ts +++ b/test/unit/server/host-stats/service.test.ts @@ -128,6 +128,46 @@ const DISK_T1 = new Map([ const NET_T0 = { rxBytes: 1_000_000, txBytes: 500_000, rxErr: 3, txErr: 1, rxDrop: 2, txDrop: 4 } const NET_T1 = { rxBytes: 1_500_000, txBytes: 600_000, rxErr: 5, txErr: 3, rxDrop: 3, txDrop: 5 } +const CPU_T2 = { + total: 6000, + busy: 600, + steal: 60, + perCore: [ + { total: 1500, busy: 150 }, + { total: 1500, busy: 150 }, + { total: 1500, busy: 150 }, + { total: 1500, busy: 150 }, + ], +} +const CPU_T3 = { + total: 8000, + busy: 1200, + steal: 100, + perCore: [ + { total: 2000, busy: 300 }, + { total: 2000, busy: 300 }, + { total: 2000, busy: 300 }, + { total: 2000, busy: 300 }, + ], +} +// T2->T3 over one 2s fast tick: dBusy 600/dTotal 2000 = 30%; dSteal 40/2000 = 2%; +// per-core 150/500 = 30%; swap in 24*4/2 = 48 KB/s; out 12*4/2 = 24 KB/s; +// majfaults 40/2 = 20/s; oom 9->11. +const VMSTAT_T2 = { pswpin: 200, pswpout: 80, pgmajfault: 100, oomKill: 9 } +const VMSTAT_T3 = { pswpin: 224, pswpout: 92, pgmajfault: 140, oomKill: 11 } +// Slow tier growth for the restart cycle (re-based T2->T3 values over 5s equal +// the familiar disk/net delta math): read 51200*512/5 = 5,242,880 B/s; write +// 102400*512/5 = 10,485,760 B/s; util 1000/5000 = 20%; await (2000+2000)/(100+400) = 8ms; +// rx (2.5M-2M)/5 = 100,000 B/s; tx (1.1M-1M)/5 = 20,000 B/s; err/drop deltas 2/3/1/2. +const DISK_T2 = new Map([ + ['sda', { readsCompleted: 2000, readMs: 8000, writesCompleted: 4000, writeMs: 16000, readSectors: 200_000, writtenSectors: 800_000, timeDoingIosMs: 1000 }], +]) +const DISK_T3 = new Map([ + ['sda', { readsCompleted: 2100, readMs: 10000, writesCompleted: 4400, writeMs: 18000, readSectors: 251_200, writtenSectors: 902_400, timeDoingIosMs: 2000 }], +]) +const NET_T2 = { rxBytes: 2_000_000, txBytes: 1_000_000, rxErr: 7, txErr: 2, rxDrop: 5, txDrop: 6 } +const NET_T3 = { rxBytes: 2_500_000, txBytes: 1_100_000, rxErr: 9, txErr: 5, rxDrop: 6, txDrop: 8 } + const TABLE = { top: [{ pid: 5, name: 'node', cpuPct: 12.3, rssBytes: 1e6, state: 'S' }], zombies: 1, @@ -334,6 +374,77 @@ describe('start/stop (contract points 1, 5)', () => { vi.advanceTimersByTime(2000) expect(readerFn('readCpuTimes')).toHaveBeenCalledTimes(2) }) + + it('restart after stop re-bases every delta family and resets the live cache (fresh-start restart)', () => { + // Cycle 1: run the default T0 fixtures long enough for every prev-* store + // AND the slow-owned liveCache keys to populate (fast ticks at 2/4/6s, the + // first slow tick at 5s). + const service = makeService() + service.start() + vi.advanceTimersByTime(6000) + expect(service.getSnapshot().live.diskIo.available).toBe(true) // slow cache populated pre-stop + + service.stop() + // A stopped service publishes the documented fresh-subscriber shape again: + // no stale slow sections survive a stop. + const stopped = service.getSnapshot().live + expect(stopped.diskIo.available).toBe(false) + expect(stopped.network.available).toBe(false) + expect(stopped.limits.available).toBe(false) + + // The cumulative counters GROW while the service is unwatched — the + // long-window bug case (all values below are grown from the T0 cycle). + // Fake timers advance Date.now(), the dt basis. + vi.advanceTimersByTime(20000) + vi.mocked(readersMock.readCpuTimes).mockReturnValue(CPU_T2) + vi.mocked(readersMock.readVmstat).mockReturnValue(VMSTAT_T2) + vi.mocked(readersMock.readDiskStats).mockReturnValue(DISK_T2) + vi.mocked(readersMock.readNetDev).mockReturnValue(NET_T2) + + service.start() + // The restart's immediate tick is the documented null-safe first sample: + // zero rates/deltas, totals carried from the CURRENT (T2) sample, and the + // slow sections stay unavailable until their first new slow tick. + const restarted = service.getSnapshot().live + expect(restarted.cpu).toEqual({ available: true, usagePct: 0, stealPct: 0, perCorePct: [0, 0, 0, 0], freqMHz: null }) + expect(restarted.paging).toEqual({ available: true, swapInKbps: 0, swapOutKbps: 0, majFaultsPerSec: 0, oomKillsDelta: 0, oomKillsTotal: 9 }) + expect(restarted.diskIo.available).toBe(false) + expect(restarted.network.available).toBe(false) + expect(restarted.limits.available).toBe(false) + + // The next fast tick deltas against the RESTART sample (T2), never against + // the pre-stop sample (T0): every value below derives from T2->T3, dt=2s. + vi.mocked(readersMock.readCpuTimes).mockReturnValue(CPU_T3) + vi.mocked(readersMock.readVmstat).mockReturnValue(VMSTAT_T3) + vi.advanceTimersByTime(2000) + const live = service.getSnapshot().live + expect(live.cpu).toEqual({ available: true, usagePct: 30, stealPct: 2, perCorePct: [30, 30, 30, 30], freqMHz: null }) + expect(live.paging).toEqual({ available: true, swapInKbps: 48, swapOutKbps: 24, majFaultsPerSec: 20, oomKillsDelta: 2, oomKillsTotal: 11 }) + + // First post-restart slow tick (t=+5000): the null-safe first slow sample + // — zero rates/deltas, carried totals from the GROWN samples. + vi.advanceTimersByTime(3000) + const firstSlow = service.getSnapshot().live + expect(firstSlow.diskIo).toEqual({ available: true, readBps: 0, writeBps: 0, utilPct: null, weightedAwaitMs: null }) + expect(firstSlow.network).toEqual({ + available: true, rxBps: 0, txBps: 0, + rxErrorsTotal: 7, txErrorsTotal: 2, rxDroppedTotal: 5, txDroppedTotal: 6, + rxErrorsDelta: 0, txErrorsDelta: 0, rxDroppedDelta: 0, txDroppedDelta: 0, + }) + + // Second post-restart slow tick (t=+10000): disk/net re-based on the FIRST + // post-restart slow sample (T2), never the pre-stop sample (T0). + vi.mocked(readersMock.readDiskStats).mockReturnValue(DISK_T3) + vi.mocked(readersMock.readNetDev).mockReturnValue(NET_T3) + vi.advanceTimersByTime(5000) + const rebased = service.getSnapshot().live + expect(rebased.diskIo).toEqual({ available: true, readBps: 5_242_880, writeBps: 10_485_760, utilPct: 20, weightedAwaitMs: 8 }) + expect(rebased.network).toEqual({ + available: true, rxBps: 100_000, txBps: 20_000, + rxErrorsTotal: 9, txErrorsTotal: 5, rxDroppedTotal: 6, txDroppedTotal: 8, + rxErrorsDelta: 2, txErrorsDelta: 3, rxDroppedDelta: 1, txDroppedDelta: 2, + }) + }) }) describe('event-loop lag histogram lifecycle (contract point 3)', () => { @@ -362,7 +473,7 @@ describe('event-loop lag histogram lifecycle (contract point 3)', () => { expect(fakeHistogram.disable).toHaveBeenCalledTimes(1) }) - it('collects no lag samples while stopped (cache retains last tick), then resumes per-tick on restart', () => { + it('collects no lag samples while stopped, then resumes per-tick on restart', () => { const service = makeService() service.start() service.stop()