From 1e26c9ce5ff2c9fb5281961652865e34e512c5c4 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Sun, 26 Jul 2026 23:48:07 -0700 Subject: [PATCH 1/7] docs: add implementation plan for idle-repaint-noise MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TDD plan: NoiseScanner repaint-noise fingerprinter (new module) + a last_meaningful_activity_at reap clock for enforce_idle_kills, plus DEV-0009 oracle deviation entry. Wire-visible lastActivityAt semantics unchanged. πŸ€– Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- docs/plans/2026-07-26-idle-repaint-noise.md | 802 ++++++++++++++++++++ 1 file changed, 802 insertions(+) create mode 100644 docs/plans/2026-07-26-idle-repaint-noise.md diff --git a/docs/plans/2026-07-26-idle-repaint-noise.md b/docs/plans/2026-07-26-idle-repaint-noise.md new file mode 100644 index 000000000..8da61ba30 --- /dev/null +++ b/docs/plans/2026-07-26-idle-repaint-noise.md @@ -0,0 +1,802 @@ +# Idle Repaint-Noise Reaping Implementation Plan + +> **For agentic workers:** This plan is executed task-by-task by the +> workflow's execute stage: a fresh implementer per task, with a spec + +> quality review after each task. Steps use checkbox (`- [ ]`) syntax +> for tracking. + +**Goal:** Make the idle auto-kill sweep reap detached terminals whose only "activity" is self-generated repaint noise (spinner frames, ticking elapsed-time counters, status-bar redraws), while never reaping detached terminals doing genuine work. + +**Architecture:** Add a small, self-contained, stateful per-terminal `NoiseScanner` (new module `crates/freshell-terminal/src/idle_noise.rs`) that fingerprints each PTY output frame's escape-stripped, digit/spinner-glyph-stripped text. `ingest()` keeps bumping the wire-visible `last_activity_at` on every frame (unchanged legacy semantics), but only frames the scanner classifies as *meaningful* refresh a new private field `last_meaningful_activity_at`, which becomes the clock `enforce_idle_kills()` reads. Detection fails open: anything not provably a repeat of recent content counts as activity. + +**Tech Stack:** Rust (crates/freshell-terminal), std-only (no new dependencies), FNV-1a hashing inline, colocated `#[cfg(test)]` unit tests, `cargo test` / `cargo clippy`. + +## Global Constraints + +- Rust server only: all code changes under `crates/`. The legacy `server/` tree is FROZEN β€” never modify it. +- Do NOT touch the client, and do NOT change attach/detach semantics (fixed in PR #534). +- Do NOT modify `crates/freshell-terminal/src/barrier_scanner.rs` β€” its module doc declares it "[PORT RISK β€” highest]"; batch-framing goldens depend on byte-exact behavior. The new scanner is a separate module. +- Wire-visible `lastActivityAt` semantics are UNCHANGED: still bumped on **every** PTY output frame and **every** input write (`port/machine/specs/terminal-core.md` Β§1.3). Only the reaper's clock changes; that deviation from legacy is recorded as DEV-0009 in `port/oracle/DEVIATIONS.md` (Task 3). +- `autoKillIdleMinutes <= 0` remains a disabled no-op, unchanged. +- No new settings field: `SettingsSafety` keeps exactly one field. All tuning knobs (fingerprint ring size, spinner-glyph set) are module constants. +- TDD Red-Green mandatory: write the failing test, run it, watch it fail, then implement. +- Test/build commands: `cargo test -p freshell-terminal` and `cargo clippy` run directly (no coordinator gate needed β€” that gate is only for broad Node suites, which this plan never touches). +- Many agents share this repo: never kill processes you don't own; do NOT restart the user's self-hosted freshell server. +- Conventional commits with crate scope, ending with the Amplifier trailer (exact text): + + ``` + πŸ€– Generated with [Amplifier](https://github.com/microsoft/amplifier) + + Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> + ``` + +--- + +## Background (read before any task) + +**The bug.** `enforce_idle_kills()` (`crates/freshell-terminal/src/registry.rs`, `pub fn enforce_idle_kills`, currently ~line 731) reaps every DETACHED (`subscribers.is_empty()`) RUNNING terminal with `now - last_activity_at >= autoKillIdleMinutes * 60_000`. But `ingest()` (same file, free fn `fn ingest`, ~line 2139) bumps `s.last_activity_at = now_ms()` unconditionally on **every** PTY output frame. A detached coding-CLI TUI that merely repaints β€” codex's braille spinner plus its ticking `(Ns β€’ esc to interrupt)` counter, claude's ticking `✻ Crunched for Ns` line, any status-bar clock β€” refreshes the stamp forever and is exempt from the reaper. This is inherited from legacy (`server/terminal-registry.ts:1684`), so the fix is a deliberate deviation requiring a `port/oracle/DEVIATIONS.md` entry (DEV-0009, Task 3). It caused the 2026-07-25 orphaned-terminal incident (10 detached CLIs alive 10–22h against a 3h threshold); the client-side half was fixed in PR #534. + +**Why a new module and a new field (investigated alternatives):** +- The existing per-terminal `BarrierScanner` (`s.scanner.scan(&frame.data)` inside `ingest()`) is a VT *parser-state* classifier for batch merge boundaries β€” a spinner frame classifies identically to real output (`barrier: true, reason: Control`), so it cannot distinguish noise, and its file is the highest port risk in the crate (must not be perturbed). +- The TERM-15/TERM-16 CLI busy detection (`ActivityEvent::Output` tap β†’ `freshell-ws/src/activity.rs`) is a separate mechanism, skipped entirely for `mode == "shell"`, and only does BEL turn-complete detection β€” no content-noise classification exists anywhere. We leave that tap completely untouched (no busy-status regression by construction). +- `last_activity_at` is wire-visible (`inventory()`, `DirectoryEntry`, sorting/pagination in `crates/freshell-server/src/terminals.rs`) and spec-pinned to "bumped on every PTY output". So the reaper gets its own private field instead of changing that one. + +**Frame delivery facts the design relies on:** one PTY reader thread per terminal invokes the sink once per framed message; frames are split (never truncated) at `MAX_REALTIME_MESSAGE_BYTES = 16 * 1024`. A single spinner repaint normally arrives as ONE frame; a large redraw splits into a repeating *cycle* of frames. Hence the scanner (a) keeps its escape-parsing state across frames, and (b) compares each frame's fingerprint against a small ring of *recent* fingerprints, not just the previous frame. + +**Detection model (what "meaningful" means):** For each frame, walk its chars with a minimal VT state machine (Ground / Esc / Csi / StringBody / StringEsc β€” mirrors the barrier scanner's modes without touching it). Only Ground-mode chars can contribute. A Ground char is *significant* unless it is whitespace, an ASCII digit, a Braille pattern (U+2800–U+28FF, codex's spinner), or one of a small explicit spinner-glyph set (claude's `✻`-family, the classic `|/-\` cycle). Fold significant chars into an FNV-1a 64-bit hash + count. A frame is **noise** when its significant count is 0 (pure cursor motion / erase / spinner glyph) OR its (hash, count) fingerprint matches one of the 8 most recent distinct fingerprints (a ticking counter differs only in digits β†’ identical fingerprint; a split big redraw cycles within the ring). Otherwise it is **meaningful** and its fingerprint enters the ring. First occurrence of any status line therefore counts as activity (fail-open β€” correct: the reap clock starts at "last genuinely-new content"). + +**Test conventions in `registry.rs`'s `#[cfg(test)] mod tests`** (~line 2243): headless terminals via `reg.insert_headless("T", "S")` (mode `"shell"`, `pty: None`); frames built by the local helper `frame(seq, data, stream_id)` and driven through `reg.feed(id, frame)` which calls `ingest()` directly; `reg.set_auto_kill_idle_minutes(1)`; `reg.backdate_last_activity("T", ts)` avoids real sleeps; sinks via `collector()`. The existing idle-kill test block sits at ~lines 3392–3482. + +Line numbers throughout this plan are current as of branch creation and may drift a few lines β€” always anchor by the quoted symbol/code, not the number. + +--- + +### Task 1: `NoiseScanner` module (`idle_noise.rs`) + +**Files:** +- Create: `crates/freshell-terminal/src/idle_noise.rs` +- Modify: `crates/freshell-terminal/src/lib.rs` (one line: declare the module) + +**Interfaces:** +- Consumes: nothing (std-only, self-contained). +- Produces (Task 2 relies on these exact names): + - `pub(crate) struct NoiseScanner` + - `pub(crate) fn NoiseScanner::new() -> NoiseScanner` + - `pub(crate) fn NoiseScanner::observe(&mut self, data: &str) -> bool` β€” `true` = meaningful activity, `false` = repaint noise. Stateful across calls (escape state and fingerprint ring persist). + +- [ ] **Step 1: Declare the module** + +Open `crates/freshell-terminal/src/lib.rs` and add, alongside the existing private module declarations (next to the line declaring the `barrier_scanner` module, matching its visibility style): + +```rust +mod idle_noise; +``` + +Create `crates/freshell-terminal/src/idle_noise.rs` containing ONLY the skeleton below (so the failing tests compile-fail on missing behavior, not missing files): + +```rust +//! Repaint-noise fingerprinting for the idle auto-kill sweep (DEV-0009). +//! +//! Distinguishes MEANINGFUL PTY output (genuinely new content: log lines, +//! streamed response text) from self-generated repaint noise (spinner frames, +//! ticking elapsed-time counters, status-bar redraws) so `enforce_idle_kills` +//! can reap detached terminals that are merely repainting. Detection fails +//! open: anything not provably a repeat of recent content counts as activity. +//! +//! Deliberately SEPARATE from `barrier_scanner.rs` ([PORT RISK β€” highest]): +//! that scanner decides `terminal.output.batch` merge boundaries and must stay +//! byte-exact with legacy; this one is a port-only addition (no wire-visible +//! output) and must never be merged into it. + +/// How many recent distinct frame fingerprints to remember. A full-screen +/// redraw larger than the 16 KiB frame budget splits into a repeating CYCLE +/// of frames; membership in a small ring (rather than equality with only the +/// previous frame) still classifies the cycle as noise. 8 also absorbs small +/// spinner-word rotations without meaningfully delaying recognition of +/// genuine new output. +const RECENT_FINGERPRINTS: usize = 8; +``` + +- [ ] **Step 2: Write the failing tests** + +Append to `crates/freshell-terminal/src/idle_noise.rs`: + +```rust +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn first_frame_with_new_text_is_meaningful() { + let mut n = NoiseScanner::new(); + assert!(n.observe("compiling freshell-terminal v0.1.0\n")); + } + + #[test] + fn repeated_identical_status_line_is_noise() { + let mut n = NoiseScanner::new(); + assert!(n.observe("\r\x1b[2Kbuilding... please wait")); + assert!(!n.observe("\r\x1b[2Kbuilding... please wait")); + assert!(!n.observe("\r\x1b[2Kbuilding... please wait")); + } + + #[test] + fn codex_style_spinner_and_ticking_counter_is_noise_after_first_frame() { + let mut n = NoiseScanner::new(); + // First paint of the status line is genuinely new content. + assert!(n.observe("\r\x1b[2Kβ ‹ (1s β€’ esc to interrupt)")); + // Subsequent repaints differ only in braille glyph + digits. + assert!(!n.observe("\r\x1b[2Kβ ™ (2s β€’ esc to interrupt)")); + assert!(!n.observe("\r\x1b[2Kβ Ή (3s β€’ esc to interrupt)")); + assert!(!n.observe("\r\x1b[2Kβ Έ (12s β€’ esc to interrupt)")); + } + + #[test] + fn claude_style_ticking_crunch_line_is_noise_after_first_frame() { + let mut n = NoiseScanner::new(); + assert!(n.observe("\r\x1b[2K✻ Crunched for 5s Β· 1.2k tokens Β· esc to interrupt")); + assert!(!n.observe("\r\x1b[2K✽ Crunched for 6s Β· 1.3k tokens Β· esc to interrupt")); + assert!(!n.observe("\r\x1b[2K✻ Crunched for 17s Β· 2.0k tokens Β· esc to interrupt")); + } + + #[test] + fn status_bar_clock_redraw_is_noise_after_first_frame() { + let mut n = NoiseScanner::new(); + assert!(n.observe("\x1b[1;70Hbash | 12:34:56")); + assert!(!n.observe("\x1b[1;70Hbash | 12:34:57")); + assert!(!n.observe("\x1b[1;70Hbash | 12:35:03")); + } + + #[test] + fn pure_cursor_motion_frame_is_noise_even_when_first() { + let mut n = NoiseScanner::new(); + assert!(!n.observe("\x1b[2K\x1b[1;1H\x1b[?25l")); + } + + #[test] + fn braille_only_spinner_frame_is_noise_even_when_first() { + let mut n = NoiseScanner::new(); + assert!(!n.observe("\rβ ‹")); + assert!(!n.observe("\rβ ™")); + } + + #[test] + fn empty_frame_is_noise() { + let mut n = NoiseScanner::new(); + assert!(!n.observe("")); + } + + #[test] + fn distinct_log_lines_are_each_meaningful() { + let mut n = NoiseScanner::new(); + assert!(n.observe("compiling freshell-terminal v0.1.0\n")); + assert!(n.observe("warning: unused variable `x`\n")); + assert!(n.observe(" Finished dev profile\n")); + } + + #[test] + fn streamed_response_text_chunks_are_meaningful() { + // A coding CLI mid-turn streaming a response: every chunk is new prose. + let mut n = NoiseScanner::new(); + assert!(n.observe("The registry keeps a per-terminal ")); + assert!(n.observe("replay ring whose frames are ")); + assert!(n.observe("classified by a persistent scanner.")); + } + + #[test] + fn osc_title_payload_never_contributes_to_the_fingerprint() { + // Terminal-title clock updates are string-body content, not Ground text. + let mut n = NoiseScanner::new(); + assert!(!n.observe("\x1b]0;bash β€” 12:34\x07")); + assert!(!n.observe("\x1b]0;bash β€” 12:35\x07")); + } + + #[test] + fn osc_split_across_frames_does_not_leak_payload_into_fingerprint() { + let mut n = NoiseScanner::new(); + // Frame boundary lands MID-OSC: a stateless scanner would treat + // "ock" in the second frame as Ground text and call it meaningful. + assert!(!n.observe("\x1b]0;my title β€” cl")); + assert!(!n.observe("ock\x07")); + } + + #[test] + fn csi_split_across_frames_does_not_corrupt_ground_text() { + let mut n = NoiseScanner::new(); + assert!(n.observe("hello")); + // "\x1b[2" then "Khello": the K is the CSI final byte, so the second + // frame's Ground text is exactly "hello" β€” already in the ring. + assert!(!n.observe("\x1b[2")); + assert!(!n.observe("Khello")); + } + + #[test] + fn alternating_repaint_cycle_is_noise_via_recent_ring() { + // A big redraw split into two frames A, B repeating: A B A B ... + // Comparing only against the immediately-previous frame would see + // alternation as forever-new; ring membership must not. + let mut n = NoiseScanner::new(); + let a = "\x1b[1;1Hpane one contents"; + let b = "\x1b[2;1Hstatus bar | ready"; + assert!(n.observe(a)); + assert!(n.observe(b)); + for _ in 0..5 { + assert!(!n.observe(a)); + assert!(!n.observe(b)); + } + } + + #[test] + fn digits_only_change_is_noise() { + let mut n = NoiseScanner::new(); + assert!(n.observe("Downloading 45% complete")); + assert!(!n.observe("Downloading 46% complete")); + assert!(!n.observe("Downloading 99% complete")); + } + + #[test] + fn ring_evicts_oldest_after_capacity() { + // Pins FIFO eviction: after RECENT_FINGERPRINTS distinct newer + // frames, the oldest fingerprint is forgotten and counts as new + // again (fail-open by design). Fillers must differ in LETTERS + // (digits are stripped from fingerprints). The array length is + // 8 == RECENT_FINGERPRINTS; if the constant changes, change this + // test with it β€” that is intentional pinning. + let mut n = NoiseScanner::new(); + assert!(n.observe("line zero")); + for word in ["alpha", "bravo", "charlie", "delta", "echo", "foxtrot", "golf", "hotel"] { + assert!(n.observe(&format!("filler {word}"))); + } + assert!(n.observe("line zero")); + } +} +``` + +- [ ] **Step 3: Run the tests to verify they fail** + +Run: `cargo test -p freshell-terminal idle_noise -- --nocapture` +Expected: **compilation FAILURE** β€” `NoiseScanner` not found in `idle_noise` (the skeleton has no struct yet). A compile error on the missing type is the RED state for a brand-new module. + +- [ ] **Step 4: Implement `NoiseScanner`** + +Insert between the `RECENT_FINGERPRINTS` constant and the `#[cfg(test)]` module in `crates/freshell-terminal/src/idle_noise.rs`: + +```rust +/// Minimal VT escape-consumption state, persisted ACROSS frames so an escape +/// sequence split at the 16 KiB frame boundary never leaks bytes into the +/// fingerprint. Mirrors the mode set of `barrier_scanner.rs` without touching +/// that file: OSC/DCS/APC/PM/SOS all share one string-body state because we +/// only need "not Ground", never which string it was. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum NoiseMode { + Ground, + Esc, + Csi, + /// Inside an OSC/DCS/APC/PM/SOS string body (until BEL or ESC `\`). + StringBody, + /// Saw ESC inside a string body (possible ST terminator). + StringEsc, +} + +/// One frame's content fingerprint: FNV-1a 64 over the frame's significant +/// Ground-mode code points, plus their count (collision belt-and-braces). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct Fingerprint { + hash: u64, + count: u32, +} + +const FNV_OFFSET_BASIS: u64 = 0xcbf2_9ce4_8422_2325; +const FNV_PRIME: u64 = 0x0000_0100_0000_01b3; + +/// Significant = printable Ground content NOT expected to vary between +/// repaints of the same status line. Excluded: whitespace (padding/alignment +/// shifts), ASCII digits (ticking counters, clocks, percentages, token +/// counts), the Braille Patterns block (codex's spinner glyphs), and a small +/// explicit spinner-glyph set (claude's ✻-family, the classic `|/-\` cycle). +/// Stripping affects only the fingerprint β€” real content always carries +/// letters/punctuation that survive the strip and change the hash. +fn is_significant(ch: char) -> bool { + if ch.is_whitespace() || ch.is_ascii_digit() { + return false; + } + let cp = ch as u32; + if (0x2800..=0x28ff).contains(&cp) { + return false; // Braille Patterns block (braille spinners) + } + !matches!( + ch, + '✻' | '✽' | '✳' | '✒' | 'Β·' | 'βˆ—' | '*' | '|' | '/' | '-' | '\\' + | '●' | 'β—‹' | '◐' | 'β—“' | 'β—‘' | 'β—’' | 'β—΄' | 'β—΅' | 'β—Ά' | 'β—·' + ) +} + +/// Per-terminal repaint-noise fingerprinter. See module docs. +#[derive(Debug)] +pub(crate) struct NoiseScanner { + mode: NoiseMode, + /// Ring of the most recent distinct frame fingerprints (FIFO, cap + /// [`RECENT_FINGERPRINTS`]). + recent: std::collections::VecDeque, +} + +impl NoiseScanner { + pub(crate) fn new() -> Self { + Self { + mode: NoiseMode::Ground, + recent: std::collections::VecDeque::with_capacity(RECENT_FINGERPRINTS), + } + } + + /// Feed one PTY output frame. Returns `true` when the frame carries + /// meaningful new content (refresh the idle reap clock), `false` when it + /// is repaint noise. + pub(crate) fn observe(&mut self, data: &str) -> bool { + let mut hash: u64 = FNV_OFFSET_BASIS; + let mut count: u32 = 0; + for ch in data.chars() { + let cp = ch as u32; + match self.mode { + NoiseMode::Ground => { + if cp == 0x1b { + self.mode = NoiseMode::Esc; + } else if cp < 0x20 || cp == 0x7f || (0x80..=0x9f).contains(&cp) { + // C0/C1 control (incl. \r \n \t): never significant. + } else if is_significant(ch) { + let mut buf = [0u8; 4]; + for b in ch.encode_utf8(&mut buf).bytes() { + hash ^= u64::from(b); + hash = hash.wrapping_mul(FNV_PRIME); + } + count = count.saturating_add(1); + } + } + NoiseMode::Esc => { + self.mode = match cp { + 0x1b => NoiseMode::Esc, + c if c == u32::from(b'[') => NoiseMode::Csi, + // OSC ] / DCS P / APC _ / PM ^ / SOS X + c if c == u32::from(b']') + || c == u32::from(b'P') + || c == u32::from(b'_') + || c == u32::from(b'^') + || c == u32::from(b'X') => + { + NoiseMode::StringBody + } + _ => NoiseMode::Ground, // two-char ESC sequence done + }; + } + NoiseMode::Csi => { + if (0x40..=0x7e).contains(&cp) { + self.mode = NoiseMode::Ground; // final byte + } + // else: parameter/intermediate byte β€” stay in Csi. + } + NoiseMode::StringBody => { + if cp == 0x07 { + self.mode = NoiseMode::Ground; // BEL terminator + } else if cp == 0x1b { + self.mode = NoiseMode::StringEsc; + } + } + NoiseMode::StringEsc => { + self.mode = match cp { + c if c == u32::from(b'\\') => NoiseMode::Ground, // ST + 0x1b => NoiseMode::StringEsc, + _ => NoiseMode::StringBody, + }; + } + } + } + + if count == 0 { + return false; // pure control/erase/spinner-glyph repaint + } + let fp = Fingerprint { hash, count }; + if self.recent.contains(&fp) { + return false; // same normalized content as a recent frame + } + if self.recent.len() == RECENT_FINGERPRINTS { + self.recent.pop_front(); + } + self.recent.push_back(fp); + true + } +} +``` + +- [ ] **Step 5: Run the tests to verify they pass** + +Run: `cargo test -p freshell-terminal idle_noise` +Expected: PASS β€” all 16 tests, 0 failures. (An `unused` warning for `NoiseScanner` outside tests is acceptable at this point; Task 2 wires it in. If the crate denies unused warnings, silence with `#[allow(dead_code)]` on the struct and remove that attribute in Task 2.) + +- [ ] **Step 6: Quality gates** + +Run: `cargo clippy -p freshell-terminal --all-targets` +Expected: no new warnings from `idle_noise.rs`. If clippy raises `new_without_default`, satisfy it with exactly: + +```rust +impl Default for NoiseScanner { + fn default() -> Self { + Self::new() + } +} +``` + +Run: `cargo fmt --all` then `git diff --stat` β€” only files you touched should be reformatted (if others change, revert them: `git checkout -- `). + +- [ ] **Step 7: Commit** + +```bash +cd /home/dan/code/freshell/.worktrees/idle-repaint-noise +git add crates/freshell-terminal/src/idle_noise.rs crates/freshell-terminal/src/lib.rs +git commit -m "feat(terminal): add NoiseScanner repaint-noise fingerprinter for the idle reap clock + +Stateful per-terminal frame classifier: escape-stripped, digit/spinner- +glyph-stripped FNV-1a fingerprints checked against a ring of the 8 most +recent frames. Repaint noise (spinners, ticking counters, status-bar +redraws) classifies false; genuinely new content classifies true. +Groundwork for keying enforce_idle_kills on meaningful activity +(DEV-0009). Fails open: unrecognized content counts as activity. + +πŸ€– Generated with [Amplifier](https://github.com/microsoft/amplifier) + +Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>" +``` + +--- + +### Task 2: Key the idle sweep on meaningful activity + +**Files:** +- Modify: `crates/freshell-terminal/src/registry.rs` β€” `TerminalShared` struct (~line 169), `enforce_idle_kills()` (~line 731), `input()` (~line 1086), `finish_pty_exit()` (~line 1382), `ingest()` (~line 2139), the two `TerminalShared { ... }` literal initializers (in `create()` ~line 821 and `register_headless()` ~line 1581), the test helper `backdate_last_activity` (~line 2599), and new tests in the idle-kill test block (~line 3392). + +**Interfaces:** +- Consumes (from Task 1): `crate::idle_noise::NoiseScanner` β€” `NoiseScanner::new() -> NoiseScanner`, `NoiseScanner::observe(&mut self, data: &str) -> bool` (true = meaningful). +- Produces: private field `TerminalShared::last_meaningful_activity_at: i64` (epoch ms) and field `TerminalShared::noise: NoiseScanner`. `enforce_idle_kills()` reads `last_meaningful_activity_at` instead of `last_activity_at`. NO public/wire API changes; `inventory()` and `DirectoryEntry` keep projecting `last_activity_at` exactly as today. + +- [ ] **Step 1: Write the failing tests** + +Add to the idle-kill test block in `registry.rs`'s `mod tests` (directly after the existing test `enforce_idle_kills_never_kills_an_attached_terminal`, ~line 3453): + +```rust + #[test] + fn enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise() { + let reg = TerminalRegistry::new(); + reg.insert_headless("T", "S"); + reg.set_auto_kill_idle_minutes(1); + // Warm-up: the FIRST paint of a status line is genuinely new content + // and legitimately counts as activity. + reg.feed("T", frame(1, "\r\x1b[2Kβ ‹ (1s β€’ esc to interrupt)", "S")); + reg.backdate_last_activity("T", now_ms() - 10 * 60_000); + // Codex-style repaint noise after the backdate: same status line, + // only the braille glyph and the digits tick. Each frame still bumps + // the wire-visible last_activity_at (unchanged legacy semantics) but + // must NOT refresh the reap clock. + for (i, paint) in [ + "\r\x1b[2Kβ ™ (2s β€’ esc to interrupt)", + "\r\x1b[2Kβ Ή (3s β€’ esc to interrupt)", + "\r\x1b[2Kβ Έ (14s β€’ esc to interrupt)", + "\r\x1b[2Kβ Ό (65s β€’ esc to interrupt)", + ] + .iter() + .enumerate() + { + reg.feed("T", frame(i as i64 + 2, paint, "S")); + } + + let killed = reg.enforce_idle_kills(); + + assert_eq!(killed, vec!["T".to_string()]); + assert!(reg.inventory().is_empty()); + } + + #[test] + fn enforce_idle_kills_spares_detached_terminal_streaming_genuine_output() { + let reg = TerminalRegistry::new(); + reg.insert_headless("T", "S"); + reg.set_auto_kill_idle_minutes(1); + reg.backdate_last_activity("T", now_ms() - 10 * 60_000); + // A long build streaming REAL new log lines: genuine work, must + // survive the sweep even while detached. + reg.feed("T", frame(1, " Compiling freshell-terminal v0.1.0\n", "S")); + reg.feed("T", frame(2, "warning: unused variable `x` in registry.rs\n", "S")); + + let killed = reg.enforce_idle_kills(); + + assert!(killed.is_empty()); + assert_eq!(reg.inventory().len(), 1); + } + + #[test] + fn input_write_resets_the_idle_reap_clock() { + // User keystrokes are always activity (headless => the PTY write is + // skipped but the activity bump still happens, matching input()). + let reg = TerminalRegistry::new(); + reg.insert_headless("T", "S"); + reg.set_auto_kill_idle_minutes(1); + reg.backdate_last_activity("T", now_ms() - 10 * 60_000); + reg.input("T", b"ls\n"); + + let killed = reg.enforce_idle_kills(); + + assert!(killed.is_empty()); + assert_eq!(reg.inventory().len(), 1); + } +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +Run: `cargo test -p freshell-terminal enforce_idle_kills -- --nocapture && cargo test -p freshell-terminal input_write_resets` +Expected: `enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise` **FAILS** β€” `killed` is empty because every `feed()` bumped `last_activity_at` to now (the bug, reproduced). `..._spares_detached_terminal_streaming_genuine_output` and `input_write_resets_the_idle_reap_clock` PASS for the wrong reason today (any frame/input exempts); they become regression guards once the reaper switches clocks. All PRE-EXISTING idle tests must still pass. + +- [ ] **Step 3: Implement the wiring** + +All edits in `crates/freshell-terminal/src/registry.rs`. + +3a. Import the scanner near the other crate-internal imports at the top of the file: + +```rust +use crate::idle_noise::NoiseScanner; +``` + +3b. In `struct TerminalShared`, add two fields β€” the clock next to `last_activity_at`, the scanner next to `scanner: BarrierScanner`: + +```rust + created_at: i64, + last_activity_at: i64, + /// The idle-kill reap clock (DEV-0009): last MEANINGFUL activity β€” user + /// input, or PTY output carrying genuinely new content per + /// [`NoiseScanner`]. Unlike `last_activity_at` (wire-visible via + /// `inventory()`/`DirectoryEntry` and spec-pinned to bump on EVERY + /// output frame, terminal-core.md Β§1.3), repaint noise (spinner frames, + /// ticking counters, status-bar redraws) does not refresh this. + /// Read ONLY by `enforce_idle_kills`. + last_meaningful_activity_at: i64, +``` + +```rust + scanner: BarrierScanner, + /// Per-terminal repaint-noise fingerprinter feeding + /// `last_meaningful_activity_at` (DEV-0009). Independent of the barrier + /// scanner: separate state, separate concern (reaping, not batching). + noise: NoiseScanner, +``` + +3c. Initializers β€” the compiler now flags every `TerminalShared { ... }` literal; there are exactly two. In `create()` (~line 821), next to `last_activity_at: now,` add: + +```rust + last_meaningful_activity_at: now, +``` + +and next to the `scanner:` initializer add: + +```rust + noise: NoiseScanner::new(), +``` + +In `register_headless()` (~line 1581), next to `last_activity_at: created_at,` add: + +```rust + last_meaningful_activity_at: created_at, +``` + +and likewise `noise: NoiseScanner::new(),` next to its `scanner:` initializer. + +3d. `ingest()` β€” replace the unconditional bump: + +```rust + let mut s = shared.lock().expect("terminal lock"); + s.head_seq = s.head_seq.max(frame.seq_end); + s.last_activity_at = now_ms(); +``` + +with: + +```rust + let mut s = shared.lock().expect("terminal lock"); + s.head_seq = s.head_seq.max(frame.seq_end); + s.last_activity_at = now_ms(); + // DEV-0009: only genuinely-new content refreshes the idle-kill reap + // clock. Spinner repaints / ticking counters / status-bar redraws still + // bump the wire-visible last_activity_at above (terminal-core.md Β§1.3 + // holds for every consumer except the reaper) but must not exempt a + // detached terminal from enforce_idle_kills forever. + if s.noise.observe(&frame.data) { + s.last_meaningful_activity_at = s.last_activity_at; + } +``` + +3e. `input()` β€” replace `s.last_activity_at = now_ms();` with: + +```rust + let now = now_ms(); + s.last_activity_at = now; + // User keystrokes are always meaningful (DEV-0009). + s.last_meaningful_activity_at = now; +``` + +3f. `finish_pty_exit()` β€” next to the existing `s.last_activity_at = now;` add (consistency; status is Exited so the reaper ignores it either way): + +```rust + s.last_meaningful_activity_at = now; +``` + +3g. `enforce_idle_kills()` β€” swap the clock. Replace: + +```rust + if now.saturating_sub(s.last_activity_at) < idle_threshold_ms { + return None; // not idle long enough yet + } +``` + +with: + +```rust + // DEV-0009: idleness is measured against the MEANINGFUL + // activity clock, not the every-frame last_activity_at β€” + // otherwise a detached animated TUI (spinner / ticking + // counter) is exempt from this sweep forever. + if now.saturating_sub(s.last_meaningful_activity_at) < idle_threshold_ms { + return None; // not idle long enough yet + } +``` + +Also update the function's doc comment: after the sentence about the disabled state, add the line: + +```rust + /// Idleness is measured against `last_meaningful_activity_at` (DEV-0009): + /// self-generated repaint noise does not keep a detached terminal alive. +``` + +3h. Test helper `backdate_last_activity` β€” backdate BOTH clocks so every existing idle test keeps its meaning (they were written when the two clocks were one): + +```rust + /// Test-only: force a terminal's `lastActivityAt` AND its DEV-0009 + /// meaningful-activity reap clock to an arbitrary value so idle-kill + /// sweep tests don't need to sleep for real minutes. + fn backdate_last_activity(&self, terminal_id: &str, last_activity_at: i64) { + let inner = self.inner.lock().unwrap(); + let handle = inner.terminals.get(terminal_id).unwrap(); + let mut s = handle.shared.lock().unwrap(); + s.last_activity_at = last_activity_at; + s.last_meaningful_activity_at = last_activity_at; + } +``` + +- [ ] **Step 4: Run the full crate test suite** + +Run: `cargo test -p freshell-terminal` +Expected: PASS β€” all three new tests green AND every pre-existing test green, in particular the existing idle block (`enforce_idle_kills_kills_detached_terminal_past_threshold`, `..._leaves_terminal_under_threshold_running`, `..._never_kills_an_attached_terminal`, `..._disabled_when_minutes_zero`, `..._disabled_when_minutes_negative`, and the DIAG tracing pair) and the batch/replay golden tests (the barrier scanner is untouched, so any failure there means you modified the wrong thing β€” stop and revert). + +- [ ] **Step 5: Quality gates** + +Run: `cargo clippy -p freshell-terminal --all-targets` +Expected: no new warnings. +Run: `cargo test -p freshell-ws -p freshell-server` +Expected: PASS (these crates consume the registry; `spawn_idle_monitor` wiring is untouched but this proves no cross-crate breakage). +Run: `cargo fmt --all` then `git diff --stat` β€” revert any files you didn't touch. + +- [ ] **Step 6: Commit** + +```bash +cd /home/dan/code/freshell/.worktrees/idle-repaint-noise +git add crates/freshell-terminal/src/registry.rs +git commit -m "fix(terminal): key idle auto-kill on meaningful activity so repaint noise can't exempt detached terminals + +enforce_idle_kills now reads a new private last_meaningful_activity_at +clock, refreshed by input writes and by output frames the NoiseScanner +classifies as genuinely new content. Spinner frames, ticking elapsed +counters, and status-bar redraws no longer reset the reap clock, so an +abandoned detached animated TUI is reaped after autoKillIdleMinutes +(DEV-0009; second server-side hole behind the 2026-07-25 orphaned- +terminal incident β€” the client-side hole was PR #534). + +Wire-visible lastActivityAt semantics are unchanged: still bumped on +every PTY output frame and every input write (terminal-core.md Β§1.3). +Attached terminals stay exempt; autoKillIdleMinutes <= 0 stays a no-op. + +πŸ€– Generated with [Amplifier](https://github.com/microsoft/amplifier) + +Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>" +``` + +--- + +### Task 3: Record deviation DEV-0009 in the oracle ledger + +**Files:** +- Modify: `port/oracle/DEVIATIONS.md` β€” append a new entry after the `### DEV-0008` block, immediately BEFORE the `## E2E-discovered intentional divergences (EDEV-xx)` heading (~line 655 of that file). + +**Interfaces:** +- Consumes: the test names from Task 2 and module path from Task 1 (referenced verbatim in the entry's `pinning_test`). +- Produces: ledger entry `DEV-0009`, status `proposed` (the antagonist reviewer, not the implementer, adjudicates acceptance β€” do not mark it `accepted`). + +- [ ] **Step 1: Verify the pinning tests exist and pass (the ledger references them)** + +Run: `cargo test -p freshell-terminal enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise enforce_idle_kills_spares_detached_terminal_streaming_genuine_output` +Expected: PASS (2 tests). If either name errs, fix the entry text below to match reality β€” never reference a test that does not exist. + +- [ ] **Step 2: Append the ledger entry** + +Insert into `port/oracle/DEVIATIONS.md`, after the end of the DEV-0008 block and before the `## E2E-discovered intentional divergences (EDEV-xx)` heading: + +```markdown +### DEV-0009 β€” idle auto-kill reap clock ignores self-generated repaint noise (original never reaps an animated detached TUI) + +- **objective_defect:** *resource leak* β€” `server/terminal-registry.ts:1684` bumps `lastActivityAt` + on **every** PTY output frame, and `enforceIdleKills` (`terminal-registry.ts:1406-1425`) keys + idleness on that stamp. Any detached terminal whose program merely repaints (codex's braille + spinner + ticking `(Ns β€’ esc to interrupt)` counter, claude's ticking `✻ Crunched for Ns` line, + any status-bar clock) refreshes the stamp continuously, so `settings.safety.autoKillIdleMinutes` + can never reap it: the PTY, its child process tree, and its replay buffer are retained + indefinitely β€” precisely the leak the setting exists to prevent. Observed in production + 2026-07-25: 10 detached CLIs alive 10-22h against a 3h threshold (the client-side half of that + incident was PR #534; this entry is the server-side half). +- **original_behavior:** idleness = `now - lastActivityAt`, where `lastActivityAt` is refreshed by + every PTY output frame regardless of content; a detached animated TUI is exempt from the idle + sweep forever. +- **port_behavior:** the port keeps `lastActivityAt`'s wire semantics identical (still bumped on + every output frame and every input write β€” terminal-core.md Β§1.3 holds for `inventory`, the + directory projection, and sorting) but gives `enforce_idle_kills` its own reap clock, + `last_meaningful_activity_at` (`crates/freshell-terminal/src/registry.rs`), refreshed by input + writes and by output frames carrying genuinely new content per the stateful per-terminal + `NoiseScanner` (`crates/freshell-terminal/src/idle_noise.rs`): a frame whose escape-stripped + text β€” minus whitespace, ASCII digits, Braille spinner glyphs (U+2800-U+28FF), and a small + spinner-glyph set β€” is empty or fingerprint-identical to one of the 8 most recent frames counts + as repaint noise and does not refresh the reap clock. Detection fails open (anything not + provably a repeat counts as activity); attached terminals stay exempt and + `autoKillIdleMinutes <= 0` stays disabled, both unchanged. +- **fingerprint:** behavior/timing-only β€” no wire message, field, or schema change; the only + observable divergence is that the port's idle sweep reaps a detached repaint-only terminal after + the threshold where the original never would (surfaces as a `terminal.killed by=idle` / + `terminal.exit` for such a terminal, and its absence from subsequent inventories). +- **pinning_test:** `crates/freshell-terminal/src/registry.rs` tests + `enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise` and + `enforce_idle_kills_spares_detached_terminal_streaming_genuine_output`, plus the `NoiseScanner` + unit suite in `crates/freshell-terminal/src/idle_noise.rs` (split-escape statefulness, ring + membership, digits-only ticks, first-paint-counts semantics). +- **adjudicated_by:** pending antagonist review. +- **status:** proposed. +``` + +- [ ] **Step 3: Final full verification** + +Run: `cargo test -p freshell-terminal -p freshell-ws -p freshell-server` +Expected: PASS, zero failures. +Run: `cargo clippy -p freshell-terminal --all-targets` +Expected: no new warnings. + +- [ ] **Step 4: Commit** + +```bash +cd /home/dan/code/freshell/.worktrees/idle-repaint-noise +git add port/oracle/DEVIATIONS.md +git commit -m "docs(oracle): record DEV-0009 idle-reap repaint-noise deviation + +Ledger entry (status: proposed) for keying enforce_idle_kills on the +meaningful-activity clock instead of the every-frame lastActivityAt. +Objective defect bar: resource leak (detached animated TUIs were never +reapable, 2026-07-25 incident). Pinning tests referenced and passing. + +πŸ€– Generated with [Amplifier](https://github.com/microsoft/amplifier) + +Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>" +``` + +--- + +## Acceptance-contract traceability + +| Spec requirement | Proven by | +|---|---| +| 1. Detached + repaint-noise-only >= threshold β†’ reaped | Task 2 `enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise` (production `ingest()` + production `enforce_idle_kills()`, real frames through `feed`); Task 1 noise classification suite (codex spinner, claude tick line, status-bar clock, alternating split redraw) | +| 2. Detached + genuine work β†’ NOT reaped (build logs, mid-turn streaming) | Task 2 `enforce_idle_kills_spares_detached_terminal_streaming_genuine_output`; Task 1 `distinct_log_lines_are_each_meaningful`, `streamed_response_text_chunks_are_meaningful`, fail-open ring eviction test | +| 3. Input counts as activity; attached terminals stay exempt (unchanged) | Task 2 `input_write_resets_the_idle_reap_clock`; existing `enforce_idle_kills_never_kills_an_attached_terminal` kept green (Task 2 Step 4) | +| 4. `autoKillIdleMinutes <= 0` stays a disabled no-op | Existing `..._disabled_when_minutes_zero` / `..._disabled_when_minutes_negative` kept green (Task 2 Step 4) β€” the disabled short-circuit is untouched | +| No regression to busy-status classification (TERM-15/16) | The `ActivityEvent::Output` tap and `freshell-ws/src/activity.rs` are not modified anywhere in this plan; `cargo test -p freshell-ws` in Task 2 Step 5 | +| Resilient to frame batching/splitting | `NoiseScanner` state persists across frames (Task 1 cycle-B split-OSC/split-CSI tests) and ring membership handles split-redraw cycles (`alternating_repaint_cycle_is_noise_via_recent_ring`) | +| Legacy `server/` frozen; deviation recorded per oracle conventions | Only `crates/` and `port/oracle/DEVIATIONS.md` are modified; DEV-0009 (Task 3) follows the eight-field DEV schema, next free ID after DEV-0008, status `proposed` | + +**Semantics note (intentional, fail-open):** the FIRST paint of any status line counts as meaningful β€” so the reap clock effectively starts at "last genuinely-new content", and an abandoned animated TUI is reaped ~one threshold after its last real content, exactly the product intent. A TUI that continuously emits *fresh prose* (letters changing, not just digits/spinner glyphs) is treated as genuine work and never reaped β€” that is requirement 2's side of the contract and the deliberate failure direction. From e97d0cca4b3bfacf3c9a146e32c0d9ecc1e7888b Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Mon, 27 Jul 2026 00:46:15 -0700 Subject: [PATCH 2/7] docs(plans): harden idle-repaint-noise plan with load-bearing validation findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage-2 assumption validation (6 verified, 6 falsified; ledger + evidence in .worktrees/.the-usual-logs/idle-repaint-noise/). Plan fixes: - Ring 8 -> 32: live capture of codex 0.145.0 shows its shimmer header emits ~13-16 letter-subset fingerprints per cycle; a ring of 8 never settles and codex would never be reaped. New shimmer pinning test. - Strip set gains claude's real glyph (U+2736) and bullets U+2022/U+25E6. - New detach-grace wiring (detach() + remove_connection(), gated bump) with two pinning tests: a freshly-detached terminal gets one full threshold instead of dying on the next 30s sweep. - Frame-delivery facts corrected to measured reality (kernel-read boundaries <=4095 B, coalescing multiplicities, env-override limit). - Numeric-only-progress reaping (curl/dd-class) recorded as an explicit accepted product decision (AD-1) in contract, semantics note, DEV-0009. - DEV-0009 reshaped to the oracle's canonical seven-field unbolded schema; lib.rs module-visibility wording fixed. - Cross-crate 'Expected: PASS' re-scoped to 'no NEW failures' over the recorded pre-existing 18-test freshell-ws baseline. πŸ€– Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- docs/plans/2026-07-26-idle-repaint-noise.md | 263 +++++++++++++++----- 1 file changed, 206 insertions(+), 57 deletions(-) diff --git a/docs/plans/2026-07-26-idle-repaint-noise.md b/docs/plans/2026-07-26-idle-repaint-noise.md index 8da61ba30..c672767c6 100644 --- a/docs/plans/2026-07-26-idle-repaint-noise.md +++ b/docs/plans/2026-07-26-idle-repaint-noise.md @@ -20,7 +20,8 @@ - `autoKillIdleMinutes <= 0` remains a disabled no-op, unchanged. - No new settings field: `SettingsSafety` keeps exactly one field. All tuning knobs (fingerprint ring size, spinner-glyph set) are module constants. - TDD Red-Green mandatory: write the failing test, run it, watch it fail, then implement. -- Test/build commands: `cargo test -p freshell-terminal` and `cargo clippy` run directly (no coordinator gate needed β€” that gate is only for broad Node suites, which this plan never touches). +- Test/build commands: `cargo test -p freshell-terminal` and `cargo clippy` run directly (verified: AGENTS.md's coordinator gate covers only npm/Vitest suites, which this plan never touches). +- **Recorded baseline (verified 2026-07-26, see the load-bearing ledger A11):** `cargo test -p freshell-terminal` (131 tests) and `cargo test -p freshell-server` (319 tests) are green and `cargo clippy -p freshell-terminal --all-targets` is clean β€” but **freshell-ws has 18 PRE-EXISTING failing integration tests** on this host (lib tests 255/255 green; dominant panic `common/mod.rs:662: timed out waiting for a terminal.created frame`, e.g. `claude_restore_unavailable`, `session_ref_singleflight`, `pane_ledger_triggers`; pre-exists this branch, whose delta over origin/main is docs-only). Wherever this plan says a freshell-ws run is "Expected: PASS", read: **no NEW failures beyond that recorded set**. Do NOT attempt to fix those tests on this branch. - Many agents share this repo: never kill processes you don't own; do NOT restart the user's self-hosted freshell server. - Conventional commits with crate scope, ending with the Amplifier trailer (exact text): @@ -41,9 +42,11 @@ - The TERM-15/TERM-16 CLI busy detection (`ActivityEvent::Output` tap β†’ `freshell-ws/src/activity.rs`) is a separate mechanism, skipped entirely for `mode == "shell"`, and only does BEL turn-complete detection β€” no content-noise classification exists anywhere. We leave that tap completely untouched (no busy-status regression by construction). - `last_activity_at` is wire-visible (`inventory()`, `DirectoryEntry`, sorting/pagination in `crates/freshell-server/src/terminals.rs`) and spec-pinned to "bumped on every PTY output". So the reaper gets its own private field instead of changing that one. -**Frame delivery facts the design relies on:** one PTY reader thread per terminal invokes the sink once per framed message; frames are split (never truncated) at `MAX_REALTIME_MESSAGE_BYTES = 16 * 1024`. A single spinner repaint normally arrives as ONE frame; a large redraw splits into a repeating *cycle* of frames. Hence the scanner (a) keeps its escape-parsing state across frames, and (b) compares each frame's fingerprint against a small ring of *recent* fingerprints, not just the previous frame. +**Frame delivery facts the design relies on** (verified against the code and a live PTY experiment β€” see the load-bearing ledger, A2/A2b/A5): one PTY reader thread per terminal reads into an 8192-byte buffer (`pty.rs`, reader loop ~line 485) and invokes the sink once per read with no intermediate buffering, so **frame boundaries ≑ kernel read boundaries** (Linux caps a single pty read at 4095 bytes; the serialized `MAX_REALTIME_MESSAGE_BYTES = 16 * 1024` split in `fragment.rs` is essentially unreachable for live reads). When the reader keeps up β€” the norm for a detached terminal β€” one small status-line write arrives as ONE frame (54/54 observed). Under reader delay, whole repaints COALESCE into one read (2, 3, N repaints per frame); each digit-stripped concatenation multiplicity is a stable fingerprint, absorbed by the ring after one occurrence. Hence the scanner (a) keeps its escape-parsing state across frames, and (b) compares each frame's fingerprint against a ring of *recent* fingerprints, not just the previous frame. Known fail-open limits (accepted, ledger AD-2): a backlog >4095 bytes splits repaints at drifting offsets, and `fragment.rs` honors env `TERMINAL_STREAM_BATCH_MAX_BYTES` (clamped β‰₯1024) which can make a full redraw cycle exceed the ring β€” in both cases frames classify as meaningful and the terminal is simply never reaped, which is exactly the legacy status quo. -**Detection model (what "meaningful" means):** For each frame, walk its chars with a minimal VT state machine (Ground / Esc / Csi / StringBody / StringEsc β€” mirrors the barrier scanner's modes without touching it). Only Ground-mode chars can contribute. A Ground char is *significant* unless it is whitespace, an ASCII digit, a Braille pattern (U+2800–U+28FF, codex's spinner), or one of a small explicit spinner-glyph set (claude's `✻`-family, the classic `|/-\` cycle). Fold significant chars into an FNV-1a 64-bit hash + count. A frame is **noise** when its significant count is 0 (pure cursor motion / erase / spinner glyph) OR its (hash, count) fingerprint matches one of the 8 most recent distinct fingerprints (a ticking counter differs only in digits β†’ identical fingerprint; a split big redraw cycles within the ring). Otherwise it is **meaningful** and its fingerprint enters the ring. First occurrence of any status line therefore counts as activity (fail-open β€” correct: the reap clock starts at "last genuinely-new content"). +**Detection model (what "meaningful" means):** For each frame, walk its chars with a minimal VT state machine (Ground / Esc / Csi / StringBody / StringEsc β€” same mode SET as the barrier scanner, without touching it; note the barrier scanner additionally tracks ESC-intermediate bytes, which this scanner deliberately does not need: a mishandled `ESC ( B` leaks the same 'B' into the fingerprint on every byte-identical repaint, so fingerprint STABILITY is unaffected β€” verified by hand-traces, ledger A4). Only Ground-mode chars can contribute. A Ground char is *significant* unless it is whitespace, an ASCII digit, a Braille pattern (U+2800–U+28FF, codex's spinner), or one of a small explicit spinner-glyph set (claude's `✻`/`✢`-family, the classic `|/-\` cycle, the bullets `Β·`/`β€’`/`β—¦` β€” real claude 2.1.220 uses `✢` U+2736, and codex's status row separates elapsed time from the hint with `β€’` U+2022; ledger A1). Fold significant chars into an FNV-1a 64-bit hash + count. A frame is **noise** when its significant count is 0 (pure cursor motion / erase / spinner glyph) OR its (hash, count) fingerprint matches one of the 32 most recent distinct fingerprints (a ticking counter differs only in digits β†’ identical fingerprint; a coalesced or split redraw cycles within the ring). The ring is 32 β€” NOT smaller β€” because real codex (0.145.0, measured via live PTY capture) renders its "Working" header with a shimmer animation through ratatui cell-diffing, emitting per-frame LETTER-SUBSET runs of the word (`'W'`, `'Wo'`, …, `'orking'`): ~13–16 distinct fingerprints per ~2 s cycle. A ring of 8 never settles (codex would never be reaped β€” the primary target!); 32 absorbs the cycle with headroom (ledger A1). Otherwise the frame is **meaningful** and its fingerprint enters the ring. First occurrence of any status line therefore counts as activity (fail-open β€” correct: the reap clock starts at "last genuinely-new content"). + +**Known accepted limitation (deliberate product decision β€” ledger AD-1):** a workload whose ONLY output novelty is numeric β€” curl/dd single-transfer progress meters, rsync single-file `--progress`, a job logging bare numeric steps β€” fingerprints identically for hours and WILL be reaped after the threshold, even though it is genuine work. This is inherent: at the fingerprint level, progress digits are indistinguishable from the ticking counters this feature exists to defeat, and the evaluated alternative (a pre-kill child-CPU check) fails because animated TUIs burn CPU rendering spinners β€” it would exempt exactly the stuck CLIs we must reap. Mitigations: the threshold is user-configured, first paint of any new format counts as activity, and unit flips (`999k`β†’`1.0M`) reset the clock. Bar-style meters (wget's growing `=`, tqdm, pip, docker pull, cargo) are safe β€” bar growth changes the significant-char count every update. DEV-0009 and the acceptance contract state this explicitly. **Test conventions in `registry.rs`'s `#[cfg(test)] mod tests`** (~line 2243): headless terminals via `reg.insert_headless("T", "S")` (mode `"shell"`, `pty: None`); frames built by the local helper `frame(seq, data, stream_id)` and driven through `reg.feed(id, frame)` which calls `ingest()` directly; `reg.set_auto_kill_idle_minutes(1)`; `reg.backdate_last_activity("T", ts)` avoids real sleeps; sinks via `collector()`. The existing idle-kill test block sits at ~lines 3392–3482. @@ -66,7 +69,7 @@ Line numbers throughout this plan are current as of branch creation and may drif - [ ] **Step 1: Declare the module** -Open `crates/freshell-terminal/src/lib.rs` and add, alongside the existing private module declarations (next to the line declaring the `barrier_scanner` module, matching its visibility style): +Open `crates/freshell-terminal/src/lib.rs` and add, next to the line declaring the `barrier_scanner` module (note: the existing declarations there are all `pub mod …;` β€” this one is deliberately PRIVATE, since `NoiseScanner` is `pub(crate)` and has no external consumers): ```rust mod idle_noise; @@ -88,13 +91,19 @@ Create `crates/freshell-terminal/src/idle_noise.rs` containing ONLY the skeleton //! byte-exact with legacy; this one is a port-only addition (no wire-visible //! output) and must never be merged into it. -/// How many recent distinct frame fingerprints to remember. A full-screen -/// redraw larger than the 16 KiB frame budget splits into a repeating CYCLE -/// of frames; membership in a small ring (rather than equality with only the -/// previous frame) still classifies the cycle as noise. 8 also absorbs small -/// spinner-word rotations without meaningfully delaying recognition of -/// genuine new output. -const RECENT_FINGERPRINTS: usize = 8; +/// How many recent distinct frame fingerprints to remember. Sized for the +/// worst REAL repaint cycle measured: codex (0.145.0) renders its "Working" +/// header with a shimmer animation through ratatui cell-diffing, emitting +/// per-frame LETTER-SUBSET runs of the word β€” ~13–16 distinct fingerprints +/// per ~2 s cycle. A ring of 8 never settles (codex would never be reaped); +/// 32 absorbs the cycle with headroom, and also absorbs read-coalescing +/// multiplicities and split-redraw cycles, while still recognizing genuine +/// new output on its first occurrence. Known fail-open limit (accepted): +/// redraw cycles longer than the ring (e.g. a full-screen repainter under a +/// shrunken `TERMINAL_STREAM_BATCH_MAX_BYTES` env override, floor 1024 B, or +/// a >4 KiB/read noise firehose) classify as meaningful forever β€” the +/// terminal is simply never reaped, which is the legacy status quo. +const RECENT_FINGERPRINTS: usize = 32; ``` - [ ] **Step 2: Write the failing tests** @@ -228,24 +237,59 @@ mod tests { #[test] fn digits_only_change_is_noise() { + // NOTE: this deliberately encodes ledger decision AD-1 β€” a workload + // whose only novelty is numeric (curl/dd meters, numeric step logs) + // classifies as noise and IS reaped after the threshold. Recorded as + // an explicit product decision in DEV-0009; do not "fix" this test. let mut n = NoiseScanner::new(); assert!(n.observe("Downloading 45% complete")); assert!(!n.observe("Downloading 46% complete")); assert!(!n.observe("Downloading 99% complete")); } + #[test] + fn codex_shimmer_letter_subset_cycle_is_noise_after_first_cycle() { + // Real codex renders its "Working" header via ratatui cell-diffing + // with a shimmer effect: each animation frame repaints a different + // LETTER SUBSET of the word (measured on codex 0.145.0: ~13-16 + // distinct fingerprints per ~2s cycle, repeating). The ring must be + // large enough to absorb one full cycle; with a ring of 8 these + // frames churn forever and codex is never reaped. + let variants: Vec = (0..16) + .map(|i| { + let word = "Working"; + let end = 1 + (i % word.len()); + let start = i % end; + format!("\x1b[3;5H\x1b[38;2;153;153;153m{}\x1b[39m", &word[start..end]) + }) + .collect(); + let mut n = NoiseScanner::new(); + // First cycle: each distinct subset is new content (fail-open). + for v in &variants { + n.observe(v); + } + // Every later cycle must classify as pure repaint noise. + for _ in 0..3 { + for v in &variants { + assert!(!n.observe(v)); + } + } + } + #[test] fn ring_evicts_oldest_after_capacity() { // Pins FIFO eviction: after RECENT_FINGERPRINTS distinct newer // frames, the oldest fingerprint is forgotten and counts as new // again (fail-open by design). Fillers must differ in LETTERS - // (digits are stripped from fingerprints). The array length is - // 8 == RECENT_FINGERPRINTS; if the constant changes, change this - // test with it β€” that is intentional pinning. + // (digits are stripped from fingerprints), so generate two-letter + // suffixes. The loop count is RECENT_FINGERPRINTS on purpose; the + // test pins the constant's semantics, whatever its value. let mut n = NoiseScanner::new(); assert!(n.observe("line zero")); - for word in ["alpha", "bravo", "charlie", "delta", "echo", "foxtrot", "golf", "hotel"] { - assert!(n.observe(&format!("filler {word}"))); + for i in 0..RECENT_FINGERPRINTS { + let c1 = (b'a' + (i / 26) as u8) as char; + let c2 = (b'a' + (i % 26) as u8) as char; + assert!(n.observe(&format!("filler {c1}{c2}"))); } assert!(n.observe("line zero")); } @@ -263,10 +307,15 @@ Insert between the `RECENT_FINGERPRINTS` constant and the `#[cfg(test)]` module ```rust /// Minimal VT escape-consumption state, persisted ACROSS frames so an escape -/// sequence split at the 16 KiB frame boundary never leaks bytes into the -/// fingerprint. Mirrors the mode set of `barrier_scanner.rs` without touching -/// that file: OSC/DCS/APC/PM/SOS all share one string-body state because we -/// only need "not Ground", never which string it was. +/// sequence split at a frame boundary never leaks bytes into the fingerprint. +/// Same mode SET as `barrier_scanner.rs` (without touching that file): +/// OSC/DCS/APC/PM/SOS all share one string-body state because we only need +/// "not Ground", never which string it was. Unlike the barrier scanner we +/// deliberately do NOT track ESC-intermediate bytes (`ESC ( B` etc.): a +/// mishandled charset/single-shift sequence leaks the SAME stray char into +/// the fingerprint on every byte-identical repaint, so fingerprint stability +/// β€” the only property this scanner needs β€” is unaffected (verified by +/// hand-traces; see the load-bearing ledger, A4). #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum NoiseMode { Ground, @@ -293,7 +342,10 @@ const FNV_PRIME: u64 = 0x0000_0100_0000_01b3; /// repaints of the same status line. Excluded: whitespace (padding/alignment /// shifts), ASCII digits (ticking counters, clocks, percentages, token /// counts), the Braille Patterns block (codex's spinner glyphs), and a small -/// explicit spinner-glyph set (claude's ✻-family, the classic `|/-\` cycle). +/// explicit spinner-glyph set: claude's ✻-family INCLUDING `✢` U+2736 (the +/// glyph the real claude 2.1.220 binary uses β€” it contains zero `✻`), the +/// classic `|/-\` cycle, and the separator bullets `Β·` U+00B7 / `β€’` U+2022 / +/// `β—¦` U+25E6 (codex's status row uses `β€’` between elapsed time and hint). /// Stripping affects only the fingerprint β€” real content always carries /// letters/punctuation that survive the strip and change the hash. fn is_significant(ch: char) -> bool { @@ -306,7 +358,7 @@ fn is_significant(ch: char) -> bool { } !matches!( ch, - '✻' | '✽' | '✳' | '✒' | 'Β·' | 'βˆ—' | '*' | '|' | '/' | '-' | '\\' + '✻' | '✽' | '✳' | '✒' | '✢' | 'Β·' | 'β€’' | 'β—¦' | 'βˆ—' | '*' | '|' | '/' | '-' | '\\' | '●' | 'β—‹' | '◐' | 'β—“' | 'β—‘' | 'β—’' | 'β—΄' | 'β—΅' | 'β—Ά' | 'β—·' ) } @@ -409,7 +461,7 @@ impl NoiseScanner { - [ ] **Step 5: Run the tests to verify they pass** Run: `cargo test -p freshell-terminal idle_noise` -Expected: PASS β€” all 16 tests, 0 failures. (An `unused` warning for `NoiseScanner` outside tests is acceptable at this point; Task 2 wires it in. If the crate denies unused warnings, silence with `#[allow(dead_code)]` on the struct and remove that attribute in Task 2.) +Expected: PASS β€” all 17 tests, 0 failures. (An `unused` warning for `NoiseScanner` outside tests is acceptable at this point; Task 2 wires it in. If the crate denies unused warnings, silence with `#[allow(dead_code)]` on the struct and remove that attribute in Task 2.) - [ ] **Step 6: Quality gates** @@ -434,9 +486,11 @@ git add crates/freshell-terminal/src/idle_noise.rs crates/freshell-terminal/src/ git commit -m "feat(terminal): add NoiseScanner repaint-noise fingerprinter for the idle reap clock Stateful per-terminal frame classifier: escape-stripped, digit/spinner- -glyph-stripped FNV-1a fingerprints checked against a ring of the 8 most -recent frames. Repaint noise (spinners, ticking counters, status-bar -redraws) classifies false; genuinely new content classifies true. +glyph-stripped FNV-1a fingerprints checked against a ring of the 32 most +recent frames (sized for codex's shimmer animation, which emits ~16 +distinct letter-subset fingerprints per cycle). Repaint noise (spinners, +shimmer churn, ticking counters, status-bar redraws) classifies false; +genuinely new content classifies true. Groundwork for keying enforce_idle_kills on meaningful activity (DEV-0009). Fails open: unrecognized content counts as activity. @@ -450,7 +504,7 @@ Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.co ### Task 2: Key the idle sweep on meaningful activity **Files:** -- Modify: `crates/freshell-terminal/src/registry.rs` β€” `TerminalShared` struct (~line 169), `enforce_idle_kills()` (~line 731), `input()` (~line 1086), `finish_pty_exit()` (~line 1382), `ingest()` (~line 2139), the two `TerminalShared { ... }` literal initializers (in `create()` ~line 821 and `register_headless()` ~line 1581), the test helper `backdate_last_activity` (~line 2599), and new tests in the idle-kill test block (~line 3392). +- Modify: `crates/freshell-terminal/src/registry.rs` β€” `TerminalShared` struct (~line 169), `enforce_idle_kills()` (~line 731), `input()` (~line 1086), `finish_pty_exit()` (~line 1382), `ingest()` (~line 2139), the two `TerminalShared { ... }` literal initializers (in `create()` ~line 821 and `register_headless()` ~line 1581), the two transition-to-detached paths `detach()` (~line 1048) and `remove_connection()` (~line 1067), the test helper `backdate_last_activity` (~line 2599), and new tests in the idle-kill test block (~line 3392). **Interfaces:** - Consumes (from Task 1): `crate::idle_noise::NoiseScanner` β€” `NoiseScanner::new() -> NoiseScanner`, `NoiseScanner::observe(&mut self, data: &str) -> bool` (true = meaningful). @@ -524,12 +578,69 @@ Add to the idle-kill test block in `registry.rs`'s `mod tests` (directly after t assert!(killed.is_empty()); assert_eq!(reg.inventory().len(), 1); } + + #[test] + fn detach_grants_full_idle_threshold_of_grace() { + // A user may WATCH a spinner-only terminal attached for hours: the + // attached exemption forbids reaping, but nothing refreshes the + // meaningful clock, so it pre-expires underneath them. Detaching + // must therefore grant one full threshold of grace (DEV-0009) β€” + // otherwise the very next 30s sweep kills a terminal the user + // deliberately backgrounded seconds earlier (legacy never reaped it; + // terminal-core.md A13: "terminal stays running"). + let reg = TerminalRegistry::new(); + reg.insert_headless("T", "S"); + let (sink, _seen) = collector(); + let outcome = reg.attach("T", 1, sink, Some("a".into()), 0, false, None); + assert!(outcome.found); + reg.set_auto_kill_idle_minutes(1); + reg.backdate_last_activity("T", now_ms() - 10 * 60_000); + reg.detach("T", 1); + + // Freshly detached: the transition bump spares it a full threshold. + assert!(reg.enforce_idle_kills().is_empty()); + assert_eq!(reg.inventory().len(), 1); + + // Once it goes stale again AFTER the detach, it is reaped normally. + reg.backdate_last_activity("T", now_ms() - 10 * 60_000); + assert_eq!(reg.enforce_idle_kills(), vec!["T".to_string()]); + } + + #[test] + fn disconnect_grants_full_idle_threshold_of_grace() { + // Socket-close cleanup (remove_connection) is the other live + // transition-to-detached path and must grant the same grace as an + // explicit detach. The bump is gated on "this connection actually + // subscribed here AND the set became empty AND status is Running" β€” + // remove_connection iterates EVERY terminal, and an unconditional + // bump would reset unrelated detached terminals' countdowns on + // every socket close. + let reg = TerminalRegistry::new(); + reg.insert_headless("T", "S"); + let (sink, _seen) = collector(); + let outcome = reg.attach("T", 1, sink, Some("a".into()), 0, false, None); + assert!(outcome.found); + // A second, already-detached terminal whose countdown must NOT be + // disturbed by conn 1's disconnect. + reg.insert_headless("U", "S"); + reg.set_auto_kill_idle_minutes(1); + reg.backdate_last_activity("T", now_ms() - 10 * 60_000); + reg.backdate_last_activity("U", now_ms() - 10 * 60_000); + reg.remove_connection(1); + + let killed = reg.enforce_idle_kills(); + + // T was freshly detached by the disconnect => spared one threshold. + // U never had a subscriber => its stale countdown stands => reaped. + assert_eq!(killed, vec!["U".to_string()]); + assert_eq!(reg.inventory().len(), 1); + } ``` - [ ] **Step 2: Run the tests to verify they fail** -Run: `cargo test -p freshell-terminal enforce_idle_kills -- --nocapture && cargo test -p freshell-terminal input_write_resets` -Expected: `enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise` **FAILS** β€” `killed` is empty because every `feed()` bumped `last_activity_at` to now (the bug, reproduced). `..._spares_detached_terminal_streaming_genuine_output` and `input_write_resets_the_idle_reap_clock` PASS for the wrong reason today (any frame/input exempts); they become regression guards once the reaper switches clocks. All PRE-EXISTING idle tests must still pass. +Run: `cargo test -p freshell-terminal enforce_idle_kills -- --nocapture && cargo test -p freshell-terminal input_write_resets && cargo test -p freshell-terminal grants_full_idle_threshold` +Expected: `enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise` **FAILS** β€” `killed` is empty because every `feed()` bumped `last_activity_at` to now (the bug, reproduced). `detach_grants_full_idle_threshold_of_grace` and `disconnect_grants_full_idle_threshold_of_grace` also **FAIL** today (no grace bump exists: the backdated terminal is killed the moment it detaches β€” the DEV-0009 behavior cliff, reproduced). `..._spares_detached_terminal_streaming_genuine_output` and `input_write_resets_the_idle_reap_clock` PASS for the wrong reason today (any frame/input exempts); they become regression guards once the reaper switches clocks. All PRE-EXISTING idle tests must still pass. - [ ] **Step 3: Implement the wiring** @@ -650,7 +761,27 @@ Also update the function's doc comment: after the sentence about the disabled st /// self-generated repaint noise does not keep a detached terminal alive. ``` -3h. Test helper `backdate_last_activity` β€” backdate BOTH clocks so every existing idle test keeps its meaning (they were written when the two clocks were one): +3h. Detach grace (DEV-0009) β€” a terminal's meaningful clock can go stale while a watcher is attached (attached terminals are exempt from the reaper, and spinner-only output never refreshes the new clock), so the transition to detached must grant one full threshold of grace; otherwise the next 30-second sweep kills a terminal the user deliberately backgrounded moments earlier (legacy never reaped it; `terminal-core.md` A13 promises "terminal stays running" on detach). There are exactly two live transition-to-detached paths, and BOTH get the bump: + +In `detach()` (~line 1048), where the subscriber is removed, gate on "actually removed AND now empty AND still running" and refresh the reap clock: + +```rust + if s.subscribers.remove(&conn_id).is_some() + && s.subscribers.is_empty() + && s.status == TerminalRunStatus::Running + { + // DEV-0009: a freshly-detached terminal gets a full idle + // threshold of grace β€” its meaningful clock may have expired + // while a watcher was attached (attached => reaper-exempt). + s.last_meaningful_activity_at = s.last_meaningful_activity_at.max(now_ms()); + } +``` + +(Adapt to the function's existing removal line β€” keep its other behavior identical; match the exact `Running`-status check used by `enforce_idle_kills()` ~line 745 if the enum path differs.) + +In `remove_connection()` (~line 1067), which iterates EVERY terminal on socket close, apply the identical gated bump where it removes `conn_id` from a terminal's subscriber set. The `.is_some()` gate is essential here: an unconditional bump would reset the countdown of unrelated, already-detached terminals on every socket close. + +3i. Test helper `backdate_last_activity` β€” backdate BOTH clocks so every existing idle test keeps its meaning (they were written when the two clocks were one): ```rust /// Test-only: force a terminal's `lastActivityAt` AND its DEV-0009 @@ -668,14 +799,14 @@ Also update the function's doc comment: after the sentence about the disabled st - [ ] **Step 4: Run the full crate test suite** Run: `cargo test -p freshell-terminal` -Expected: PASS β€” all three new tests green AND every pre-existing test green, in particular the existing idle block (`enforce_idle_kills_kills_detached_terminal_past_threshold`, `..._leaves_terminal_under_threshold_running`, `..._never_kills_an_attached_terminal`, `..._disabled_when_minutes_zero`, `..._disabled_when_minutes_negative`, and the DIAG tracing pair) and the batch/replay golden tests (the barrier scanner is untouched, so any failure there means you modified the wrong thing β€” stop and revert). +Expected: PASS β€” all five new tests green AND every pre-existing test green, in particular the existing idle block (`enforce_idle_kills_kills_detached_terminal_past_threshold`, `..._leaves_terminal_under_threshold_running`, `..._never_kills_an_attached_terminal`, `..._disabled_when_minutes_zero`, `..._disabled_when_minutes_negative`, and the DIAG tracing pair) and the batch/replay golden tests (the barrier scanner is untouched, so any failure there means you modified the wrong thing β€” stop and revert). - [ ] **Step 5: Quality gates** Run: `cargo clippy -p freshell-terminal --all-targets` Expected: no new warnings. Run: `cargo test -p freshell-ws -p freshell-server` -Expected: PASS (these crates consume the registry; `spawn_idle_monitor` wiring is untouched but this proves no cross-crate breakage). +Expected: freshell-server fully green; freshell-ws **no NEW failures beyond the recorded baseline** (18 pre-existing integration-test failures on this host β€” see Global Constraints; lib tests must stay 255/255 green). These crates consume the registry; `spawn_idle_monitor` wiring is untouched but this proves no cross-crate breakage. Run: `cargo fmt --all` then `git diff --stat` β€” revert any files you didn't touch. - [ ] **Step 6: Commit** @@ -693,6 +824,11 @@ abandoned detached animated TUI is reaped after autoKillIdleMinutes (DEV-0009; second server-side hole behind the 2026-07-25 orphaned- terminal incident β€” the client-side hole was PR #534). +Transition-to-detached (explicit detach or socket close) grants one +full threshold of grace: the meaningful clock may have gone stale while +a watcher was attached (attached terminals are reaper-exempt), and a +just-backgrounded terminal must not be killed by the next 30s sweep. + Wire-visible lastActivityAt semantics are unchanged: still bumped on every PTY output frame and every input write (terminal-core.md Β§1.3). Attached terminals stay exempt; autoKillIdleMinutes <= 0 stays a no-op. @@ -715,8 +851,10 @@ Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.co - [ ] **Step 1: Verify the pinning tests exist and pass (the ledger references them)** -Run: `cargo test -p freshell-terminal enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise enforce_idle_kills_spares_detached_terminal_streaming_genuine_output` -Expected: PASS (2 tests). If either name errs, fix the entry text below to match reality β€” never reference a test that does not exist. +Run: `cargo test -p freshell-terminal enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise enforce_idle_kills_spares_detached_terminal_streaming_genuine_output && cargo test -p freshell-terminal grants_full_idle_threshold` +Expected: PASS (2 tests, then 2 tests). If any name errs, fix the entry text below to match reality β€” never reference a test that does not exist. + +Note on entry format: the file's canonical schema (used by DEV-0006..0008 and the end-of-file template) is a `### DEV-NNNN β€” title` heading plus seven UNBOLDED dash fields in fixed order: objective_defect, original_behavior, port_behavior, fingerprint, pinning_test, adjudicated_by, status. The entry below follows it; keep the field names unbolded. - [ ] **Step 2: Append the ledger entry** @@ -725,7 +863,7 @@ Insert into `port/oracle/DEVIATIONS.md`, after the end of the DEV-0008 block and ```markdown ### DEV-0009 β€” idle auto-kill reap clock ignores self-generated repaint noise (original never reaps an animated detached TUI) -- **objective_defect:** *resource leak* β€” `server/terminal-registry.ts:1684` bumps `lastActivityAt` +- objective_defect: *resource leak* β€” `server/terminal-registry.ts:1684` bumps `lastActivityAt` on **every** PTY output frame, and `enforceIdleKills` (`terminal-registry.ts:1406-1425`) keys idleness on that stamp. Any detached terminal whose program merely repaints (codex's braille spinner + ticking `(Ns β€’ esc to interrupt)` counter, claude's ticking `✻ Crunched for Ns` line, @@ -734,37 +872,48 @@ Insert into `port/oracle/DEVIATIONS.md`, after the end of the DEV-0008 block and indefinitely β€” precisely the leak the setting exists to prevent. Observed in production 2026-07-25: 10 detached CLIs alive 10-22h against a 3h threshold (the client-side half of that incident was PR #534; this entry is the server-side half). -- **original_behavior:** idleness = `now - lastActivityAt`, where `lastActivityAt` is refreshed by +- original_behavior: idleness = `now - lastActivityAt`, where `lastActivityAt` is refreshed by every PTY output frame regardless of content; a detached animated TUI is exempt from the idle sweep forever. -- **port_behavior:** the port keeps `lastActivityAt`'s wire semantics identical (still bumped on +- port_behavior: the port keeps `lastActivityAt`'s wire semantics identical (still bumped on every output frame and every input write β€” terminal-core.md Β§1.3 holds for `inventory`, the directory projection, and sorting) but gives `enforce_idle_kills` its own reap clock, `last_meaningful_activity_at` (`crates/freshell-terminal/src/registry.rs`), refreshed by input - writes and by output frames carrying genuinely new content per the stateful per-terminal - `NoiseScanner` (`crates/freshell-terminal/src/idle_noise.rs`): a frame whose escape-stripped - text β€” minus whitespace, ASCII digits, Braille spinner glyphs (U+2800-U+28FF), and a small - spinner-glyph set β€” is empty or fingerprint-identical to one of the 8 most recent frames counts - as repaint noise and does not refresh the reap clock. Detection fails open (anything not - provably a repeat counts as activity); attached terminals stay exempt and - `autoKillIdleMinutes <= 0` stays disabled, both unchanged. -- **fingerprint:** behavior/timing-only β€” no wire message, field, or schema change; the only + writes, by transition-to-detached (a freshly detached or socket-orphaned terminal gets one full + threshold of grace β€” its clock may have gone stale while a watcher was attached, since attached + terminals are reaper-exempt), and by output frames carrying genuinely new content per the + stateful per-terminal `NoiseScanner` (`crates/freshell-terminal/src/idle_noise.rs`): a frame + whose escape-stripped text β€” minus whitespace, ASCII digits, Braille spinner glyphs + (U+2800-U+28FF), and a small spinner-glyph set (`✻`-family incl. `✢`, `|/-\`, bullets + `Β·`/`β€’`/`β—¦`, geometric spinners) β€” is empty or fingerprint-identical to one of the 32 most + recent distinct frames counts as repaint noise and does not refresh the reap clock (ring sized + for codex's shimmer animation, which cycles ~13-16 letter-subset fingerprints; measured on + codex 0.145.0). Detection fails open (anything not provably a repeat counts as activity); + attached terminals stay exempt and `autoKillIdleMinutes <= 0` stays disabled, both unchanged. + Known accepted limitation (deliberate): a detached workload whose ONLY output novelty is + numeric (curl/dd-style single-transfer meters, bare numeric step logs) fingerprints identically + and is reaped after the threshold despite being genuine work β€” at fingerprint level such output + is indistinguishable from the ticking counters this deviation exists to defeat; bar-style and + prose-emitting workloads are unaffected. +- fingerprint: behavior/timing-only β€” no wire message, field, or schema change; the only observable divergence is that the port's idle sweep reaps a detached repaint-only terminal after the threshold where the original never would (surfaces as a `terminal.killed by=idle` / `terminal.exit` for such a terminal, and its absence from subsequent inventories). -- **pinning_test:** `crates/freshell-terminal/src/registry.rs` tests - `enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise` and - `enforce_idle_kills_spares_detached_terminal_streaming_genuine_output`, plus the `NoiseScanner` +- pinning_test: `crates/freshell-terminal/src/registry.rs` tests + `enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise`, + `enforce_idle_kills_spares_detached_terminal_streaming_genuine_output`, + `detach_grants_full_idle_threshold_of_grace`, and + `disconnect_grants_full_idle_threshold_of_grace`, plus the `NoiseScanner` unit suite in `crates/freshell-terminal/src/idle_noise.rs` (split-escape statefulness, ring - membership, digits-only ticks, first-paint-counts semantics). -- **adjudicated_by:** pending antagonist review. -- **status:** proposed. + membership, digits-only ticks, codex shimmer letter-subset cycle, first-paint-counts semantics). +- adjudicated_by: pending antagonist review. +- status: proposed. ``` - [ ] **Step 3: Final full verification** Run: `cargo test -p freshell-terminal -p freshell-ws -p freshell-server` -Expected: PASS, zero failures. +Expected: freshell-terminal and freshell-server fully green; freshell-ws no NEW failures beyond the recorded 18-test baseline (see Global Constraints; lib tests 255/255 green). Run: `cargo clippy -p freshell-terminal --all-targets` Expected: no new warnings. @@ -791,12 +940,12 @@ Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.co | Spec requirement | Proven by | |---|---| -| 1. Detached + repaint-noise-only >= threshold β†’ reaped | Task 2 `enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise` (production `ingest()` + production `enforce_idle_kills()`, real frames through `feed`); Task 1 noise classification suite (codex spinner, claude tick line, status-bar clock, alternating split redraw) | -| 2. Detached + genuine work β†’ NOT reaped (build logs, mid-turn streaming) | Task 2 `enforce_idle_kills_spares_detached_terminal_streaming_genuine_output`; Task 1 `distinct_log_lines_are_each_meaningful`, `streamed_response_text_chunks_are_meaningful`, fail-open ring eviction test | -| 3. Input counts as activity; attached terminals stay exempt (unchanged) | Task 2 `input_write_resets_the_idle_reap_clock`; existing `enforce_idle_kills_never_kills_an_attached_terminal` kept green (Task 2 Step 4) | +| 1. Detached + repaint-noise-only >= threshold β†’ reaped | Task 2 `enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise` (production `ingest()` + production `enforce_idle_kills()`, real frames through `feed`); Task 1 noise classification suite (codex spinner + measured codex shimmer letter-subset cycle, claude tick line, status-bar clock, alternating split redraw) | +| 2. Detached + genuine work β†’ NOT reaped, for work with any non-numeric output novelty (build logs, mid-turn streaming, bar-style meters). KNOWN EXCEPTION (deliberate, ledger AD-1): numeric-only-novelty workloads classify as noise and ARE reaped β€” recorded in DEV-0009 | Task 2 `enforce_idle_kills_spares_detached_terminal_streaming_genuine_output`; Task 1 `distinct_log_lines_are_each_meaningful`, `streamed_response_text_chunks_are_meaningful`, fail-open ring eviction test; the exception pinned by `digits_only_change_is_noise` | +| 3. Input counts as activity; attached terminals stay exempt (unchanged); freshly-detached terminals get one full threshold of grace | Task 2 `input_write_resets_the_idle_reap_clock`, `detach_grants_full_idle_threshold_of_grace`, `disconnect_grants_full_idle_threshold_of_grace`; existing `enforce_idle_kills_never_kills_an_attached_terminal` kept green (Task 2 Step 4) | | 4. `autoKillIdleMinutes <= 0` stays a disabled no-op | Existing `..._disabled_when_minutes_zero` / `..._disabled_when_minutes_negative` kept green (Task 2 Step 4) β€” the disabled short-circuit is untouched | | No regression to busy-status classification (TERM-15/16) | The `ActivityEvent::Output` tap and `freshell-ws/src/activity.rs` are not modified anywhere in this plan; `cargo test -p freshell-ws` in Task 2 Step 5 | | Resilient to frame batching/splitting | `NoiseScanner` state persists across frames (Task 1 cycle-B split-OSC/split-CSI tests) and ring membership handles split-redraw cycles (`alternating_repaint_cycle_is_noise_via_recent_ring`) | -| Legacy `server/` frozen; deviation recorded per oracle conventions | Only `crates/` and `port/oracle/DEVIATIONS.md` are modified; DEV-0009 (Task 3) follows the eight-field DEV schema, next free ID after DEV-0008, status `proposed` | +| Legacy `server/` frozen; deviation recorded per oracle conventions | Only `crates/` and `port/oracle/DEVIATIONS.md` are modified; DEV-0009 (Task 3) follows the file's canonical seven-field unbolded DEV schema (verified against DEV-0006..0008 and the file's template), next free ID after DEV-0008, status `proposed` | -**Semantics note (intentional, fail-open):** the FIRST paint of any status line counts as meaningful β€” so the reap clock effectively starts at "last genuinely-new content", and an abandoned animated TUI is reaped ~one threshold after its last real content, exactly the product intent. A TUI that continuously emits *fresh prose* (letters changing, not just digits/spinner glyphs) is treated as genuine work and never reaped β€” that is requirement 2's side of the contract and the deliberate failure direction. +**Semantics note (intentional, fail-open):** the FIRST paint of any status line counts as meaningful β€” so the reap clock effectively starts at "last genuinely-new content" (or at the moment of detach, whichever is later β€” the detach-grace bump), and an abandoned animated TUI is reaped ~one threshold after its last real content. A TUI that continuously emits *fresh prose* (letters changing, not just digits/spinner glyphs) is treated as genuine work and never reaped β€” that is requirement 2's side of the contract and the deliberate failure direction. Two bounded exceptions are recorded as explicit product decisions in the load-bearing ledger (`.worktrees/.the-usual-logs/idle-repaint-noise/load-bearing-ledger.md`) and in DEV-0009: numeric-only-novelty workloads ARE reaped (AD-1), and pathological repaint volumes that overflow the fingerprint ring are NEVER reaped, matching legacy (AD-2). From a0f66656861ec99427cf9169d73b0c5a6c53d065 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:02:59 -0700 Subject: [PATCH 3/7] docs(plans): fix two executable verification gates flagged by fresh-eyes review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Task 3 Step 1: cargo test rejects two positional TESTNAME filters; split into three single-filter invocations and correct the expected test counts (1, 1, then 2). - Task 1 Steps 1/5/6: the not-yet-wired pub(crate) NoiseScanner emits dead_code warnings, contradicting Step 6's 'no new warnings' gate. Make the #[allow(dead_code)] unconditional on the mod idle_noise; declaration in Step 1, align Step 5/6 expectations, and have Task 2 remove the attribute (Files list, Step 3a, git add, commit message) when registry.rs wires the scanner in. πŸ€– Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- docs/plans/2026-07-26-idle-repaint-noise.md | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/docs/plans/2026-07-26-idle-repaint-noise.md b/docs/plans/2026-07-26-idle-repaint-noise.md index c672767c6..fe12ef4cb 100644 --- a/docs/plans/2026-07-26-idle-repaint-noise.md +++ b/docs/plans/2026-07-26-idle-repaint-noise.md @@ -72,9 +72,12 @@ Line numbers throughout this plan are current as of branch creation and may drif Open `crates/freshell-terminal/src/lib.rs` and add, next to the line declaring the `barrier_scanner` module (note: the existing declarations there are all `pub mod …;` β€” this one is deliberately PRIVATE, since `NoiseScanner` is `pub(crate)` and has no external consumers): ```rust +#[allow(dead_code)] // temporary: nothing uses the scanner until Task 2 wires it into registry.rs; Task 2 removes this attribute mod idle_noise; ``` +The `#[allow(dead_code)]` is required, not optional: the crate does not deny warnings, but without it `cargo test`/`cargo clippy` emit `dead_code` warnings for the not-yet-wired `pub(crate)` scanner, which would violate Step 6's "no new warnings" gate. Placing it on the module declaration covers the struct, its methods, and the module's constants in one place. + Create `crates/freshell-terminal/src/idle_noise.rs` containing ONLY the skeleton below (so the failing tests compile-fail on missing behavior, not missing files): ```rust @@ -461,12 +464,12 @@ impl NoiseScanner { - [ ] **Step 5: Run the tests to verify they pass** Run: `cargo test -p freshell-terminal idle_noise` -Expected: PASS β€” all 17 tests, 0 failures. (An `unused` warning for `NoiseScanner` outside tests is acceptable at this point; Task 2 wires it in. If the crate denies unused warnings, silence with `#[allow(dead_code)]` on the struct and remove that attribute in Task 2.) +Expected: PASS β€” all 17 tests, 0 failures, and no `dead_code`/unused warnings (the module-level `#[allow(dead_code)]` added in Step 1 silences them while the scanner is unwired; Task 2 removes that attribute when `registry.rs` starts consuming the scanner). - [ ] **Step 6: Quality gates** Run: `cargo clippy -p freshell-terminal --all-targets` -Expected: no new warnings from `idle_noise.rs`. If clippy raises `new_without_default`, satisfy it with exactly: +Expected: no new warnings from `idle_noise.rs` (achievable because Step 1's module-level `#[allow(dead_code)]` covers the not-yet-wired scanner). If clippy raises `new_without_default`, satisfy it with exactly: ```rust impl Default for NoiseScanner { @@ -505,6 +508,7 @@ Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.co **Files:** - Modify: `crates/freshell-terminal/src/registry.rs` β€” `TerminalShared` struct (~line 169), `enforce_idle_kills()` (~line 731), `input()` (~line 1086), `finish_pty_exit()` (~line 1382), `ingest()` (~line 2139), the two `TerminalShared { ... }` literal initializers (in `create()` ~line 821 and `register_headless()` ~line 1581), the two transition-to-detached paths `detach()` (~line 1048) and `remove_connection()` (~line 1067), the test helper `backdate_last_activity` (~line 2599), and new tests in the idle-kill test block (~line 3392). +- Modify: `crates/freshell-terminal/src/lib.rs` β€” remove the temporary `#[allow(dead_code)]` attribute (and its comment) from the `mod idle_noise;` declaration added in Task 1 (the scanner now has a consumer). **Interfaces:** - Consumes (from Task 1): `crate::idle_noise::NoiseScanner` β€” `NoiseScanner::new() -> NoiseScanner`, `NoiseScanner::observe(&mut self, data: &str) -> bool` (true = meaningful). @@ -652,6 +656,8 @@ All edits in `crates/freshell-terminal/src/registry.rs`. use crate::idle_noise::NoiseScanner; ``` +Also open `crates/freshell-terminal/src/lib.rs` and remove the temporary `#[allow(dead_code)]` attribute (and its trailing comment) from above `mod idle_noise;` β€” the scanner is now used, so the declaration returns to plain `mod idle_noise;`. + 3b. In `struct TerminalShared`, add two fields β€” the clock next to `last_activity_at`, the scanner next to `scanner: BarrierScanner`: ```rust @@ -813,7 +819,7 @@ Run: `cargo fmt --all` then `git diff --stat` β€” revert any files you didn't to ```bash cd /home/dan/code/freshell/.worktrees/idle-repaint-noise -git add crates/freshell-terminal/src/registry.rs +git add crates/freshell-terminal/src/registry.rs crates/freshell-terminal/src/lib.rs git commit -m "fix(terminal): key idle auto-kill on meaningful activity so repaint noise can't exempt detached terminals enforce_idle_kills now reads a new private last_meaningful_activity_at @@ -832,6 +838,8 @@ just-backgrounded terminal must not be killed by the next 30s sweep. Wire-visible lastActivityAt semantics are unchanged: still bumped on every PTY output frame and every input write (terminal-core.md Β§1.3). Attached terminals stay exempt; autoKillIdleMinutes <= 0 stays a no-op. +Also drops the temporary #[allow(dead_code)] on mod idle_noise now that +the scanner has a consumer. πŸ€– Generated with [Amplifier](https://github.com/microsoft/amplifier) @@ -851,8 +859,9 @@ Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.co - [ ] **Step 1: Verify the pinning tests exist and pass (the ledger references them)** -Run: `cargo test -p freshell-terminal enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise enforce_idle_kills_spares_detached_terminal_streaming_genuine_output && cargo test -p freshell-terminal grants_full_idle_threshold` -Expected: PASS (2 tests, then 2 tests). If any name errs, fix the entry text below to match reality β€” never reference a test that does not exist. +Run: `cargo test -p freshell-terminal enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise && cargo test -p freshell-terminal enforce_idle_kills_spares_detached_terminal_streaming_genuine_output && cargo test -p freshell-terminal grants_full_idle_threshold` +(Three separate invocations on purpose: cargo accepts only ONE positional TESTNAME filter per `cargo test` call β€” passing two positional names errors with `unexpected argument`.) +Expected: PASS (1 test, 1 test, then 2 tests). If any name errs, fix the entry text below to match reality β€” never reference a test that does not exist. Note on entry format: the file's canonical schema (used by DEV-0006..0008 and the end-of-file template) is a `### DEV-NNNN β€” title` heading plus seven UNBOLDED dash fields in fixed order: objective_defect, original_behavior, port_behavior, fingerprint, pinning_test, adjudicated_by, status. The entry below follows it; keep the field names unbolded. From 1a6b2ef5cd92cee574b0a6dcb05e94e3b5717932 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:19:11 -0700 Subject: [PATCH 4/7] feat(terminal): add NoiseScanner repaint-noise fingerprinter for the idle reap clock MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stateful per-terminal frame classifier: escape-stripped, digit/spinner- glyph-stripped FNV-1a fingerprints checked against a ring of the 32 most recent frames (sized for codex's shimmer animation, which emits ~16 distinct letter-subset fingerprints per cycle). Repaint noise (spinners, shimmer churn, ticking counters, status-bar redraws) classifies false; genuinely new content classifies true. Groundwork for keying enforce_idle_kills on meaningful activity (DEV-0009). Fails open: unrecognized content counts as activity. πŸ€– Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- crates/freshell-terminal/src/idle_noise.rs | 385 +++++++++++++++++++++ crates/freshell-terminal/src/lib.rs | 3 + 2 files changed, 388 insertions(+) create mode 100644 crates/freshell-terminal/src/idle_noise.rs diff --git a/crates/freshell-terminal/src/idle_noise.rs b/crates/freshell-terminal/src/idle_noise.rs new file mode 100644 index 000000000..cbd6b872b --- /dev/null +++ b/crates/freshell-terminal/src/idle_noise.rs @@ -0,0 +1,385 @@ +//! Repaint-noise fingerprinting for the idle auto-kill sweep (DEV-0009). +//! +//! Distinguishes MEANINGFUL PTY output (genuinely new content: log lines, +//! streamed response text) from self-generated repaint noise (spinner frames, +//! ticking elapsed-time counters, status-bar redraws) so `enforce_idle_kills` +//! can reap detached terminals that are merely repainting. Detection fails +//! open: anything not provably a repeat of recent content counts as activity. +//! +//! Deliberately SEPARATE from `barrier_scanner.rs` ([PORT RISK β€” highest]): +//! that scanner decides `terminal.output.batch` merge boundaries and must stay +//! byte-exact with legacy; this one is a port-only addition (no wire-visible +//! output) and must never be merged into it. + +/// How many recent distinct frame fingerprints to remember. Sized for the +/// worst REAL repaint cycle measured: codex (0.145.0) renders its "Working" +/// header with a shimmer animation through ratatui cell-diffing, emitting +/// per-frame LETTER-SUBSET runs of the word β€” ~13–16 distinct fingerprints +/// per ~2 s cycle. A ring of 8 never settles (codex would never be reaped); +/// 32 absorbs the cycle with headroom, and also absorbs read-coalescing +/// multiplicities and split-redraw cycles, while still recognizing genuine +/// new output on its first occurrence. Known fail-open limit (accepted): +/// redraw cycles longer than the ring (e.g. a full-screen repainter under a +/// shrunken `TERMINAL_STREAM_BATCH_MAX_BYTES` env override, floor 1024 B, or +/// a >4 KiB/read noise firehose) classify as meaningful forever β€” the +/// terminal is simply never reaped, which is the legacy status quo. +const RECENT_FINGERPRINTS: usize = 32; + +/// Minimal VT escape-consumption state, persisted ACROSS frames so an escape +/// sequence split at a frame boundary never leaks bytes into the fingerprint. +/// Same mode SET as `barrier_scanner.rs` (without touching that file): +/// OSC/DCS/APC/PM/SOS all share one string-body state because we only need +/// "not Ground", never which string it was. Unlike the barrier scanner we +/// deliberately do NOT track ESC-intermediate bytes (`ESC ( B` etc.): a +/// mishandled charset/single-shift sequence leaks the SAME stray char into +/// the fingerprint on every byte-identical repaint, so fingerprint stability +/// β€” the only property this scanner needs β€” is unaffected (verified by +/// hand-traces; see the load-bearing ledger, A4). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum NoiseMode { + Ground, + Esc, + Csi, + /// Inside an OSC/DCS/APC/PM/SOS string body (until BEL or ESC `\`). + StringBody, + /// Saw ESC inside a string body (possible ST terminator). + StringEsc, +} + +/// One frame's content fingerprint: FNV-1a 64 over the frame's significant +/// Ground-mode code points, plus their count (collision belt-and-braces). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct Fingerprint { + hash: u64, + count: u32, +} + +const FNV_OFFSET_BASIS: u64 = 0xcbf2_9ce4_8422_2325; +const FNV_PRIME: u64 = 0x0000_0100_0000_01b3; + +/// Significant = printable Ground content NOT expected to vary between +/// repaints of the same status line. Excluded: whitespace (padding/alignment +/// shifts), ASCII digits (ticking counters, clocks, percentages, token +/// counts), the Braille Patterns block (codex's spinner glyphs), and a small +/// explicit spinner-glyph set: claude's ✻-family INCLUDING `✢` U+2736 (the +/// glyph the real claude 2.1.220 binary uses β€” it contains zero `✻`), the +/// classic `|/-\` cycle, and the separator bullets `Β·` U+00B7 / `β€’` U+2022 / +/// `β—¦` U+25E6 (codex's status row uses `β€’` between elapsed time and hint). +/// Stripping affects only the fingerprint β€” real content always carries +/// letters/punctuation that survive the strip and change the hash. +fn is_significant(ch: char) -> bool { + if ch.is_whitespace() || ch.is_ascii_digit() { + return false; + } + let cp = ch as u32; + if (0x2800..=0x28ff).contains(&cp) { + return false; // Braille Patterns block (braille spinners) + } + !matches!( + ch, + '✻' | '✽' + | '✳' + | '✒' + | '✢' + | 'Β·' + | 'β€’' + | 'β—¦' + | 'βˆ—' + | '*' + | '|' + | '/' + | '-' + | '\\' + | '●' + | 'β—‹' + | '◐' + | 'β—“' + | 'β—‘' + | 'β—’' + | 'β—΄' + | 'β—΅' + | 'β—Ά' + | 'β—·' + ) +} + +/// Per-terminal repaint-noise fingerprinter. See module docs. +#[derive(Debug)] +pub(crate) struct NoiseScanner { + mode: NoiseMode, + /// Ring of the most recent distinct frame fingerprints (FIFO, cap + /// [`RECENT_FINGERPRINTS`]). + recent: std::collections::VecDeque, +} + +impl NoiseScanner { + pub(crate) fn new() -> Self { + Self { + mode: NoiseMode::Ground, + recent: std::collections::VecDeque::with_capacity(RECENT_FINGERPRINTS), + } + } + + /// Feed one PTY output frame. Returns `true` when the frame carries + /// meaningful new content (refresh the idle reap clock), `false` when it + /// is repaint noise. + pub(crate) fn observe(&mut self, data: &str) -> bool { + let mut hash: u64 = FNV_OFFSET_BASIS; + let mut count: u32 = 0; + for ch in data.chars() { + let cp = ch as u32; + match self.mode { + NoiseMode::Ground => { + if cp == 0x1b { + self.mode = NoiseMode::Esc; + } else if cp < 0x20 || cp == 0x7f || (0x80..=0x9f).contains(&cp) { + // C0/C1 control (incl. \r \n \t): never significant. + } else if is_significant(ch) { + let mut buf = [0u8; 4]; + for b in ch.encode_utf8(&mut buf).bytes() { + hash ^= u64::from(b); + hash = hash.wrapping_mul(FNV_PRIME); + } + count = count.saturating_add(1); + } + } + NoiseMode::Esc => { + self.mode = match cp { + 0x1b => NoiseMode::Esc, + c if c == u32::from(b'[') => NoiseMode::Csi, + // OSC ] / DCS P / APC _ / PM ^ / SOS X + c if c == u32::from(b']') + || c == u32::from(b'P') + || c == u32::from(b'_') + || c == u32::from(b'^') + || c == u32::from(b'X') => + { + NoiseMode::StringBody + } + _ => NoiseMode::Ground, // two-char ESC sequence done + }; + } + NoiseMode::Csi => { + if (0x40..=0x7e).contains(&cp) { + self.mode = NoiseMode::Ground; // final byte + } + // else: parameter/intermediate byte β€” stay in Csi. + } + NoiseMode::StringBody => { + if cp == 0x07 { + self.mode = NoiseMode::Ground; // BEL terminator + } else if cp == 0x1b { + self.mode = NoiseMode::StringEsc; + } + } + NoiseMode::StringEsc => { + self.mode = match cp { + c if c == u32::from(b'\\') => NoiseMode::Ground, // ST + 0x1b => NoiseMode::StringEsc, + _ => NoiseMode::StringBody, + }; + } + } + } + + if count == 0 { + return false; // pure control/erase/spinner-glyph repaint + } + let fp = Fingerprint { hash, count }; + if self.recent.contains(&fp) { + return false; // same normalized content as a recent frame + } + if self.recent.len() == RECENT_FINGERPRINTS { + self.recent.pop_front(); + } + self.recent.push_back(fp); + true + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn first_frame_with_new_text_is_meaningful() { + let mut n = NoiseScanner::new(); + assert!(n.observe("compiling freshell-terminal v0.1.0\n")); + } + + #[test] + fn repeated_identical_status_line_is_noise() { + let mut n = NoiseScanner::new(); + assert!(n.observe("\r\x1b[2Kbuilding... please wait")); + assert!(!n.observe("\r\x1b[2Kbuilding... please wait")); + assert!(!n.observe("\r\x1b[2Kbuilding... please wait")); + } + + #[test] + fn codex_style_spinner_and_ticking_counter_is_noise_after_first_frame() { + let mut n = NoiseScanner::new(); + // First paint of the status line is genuinely new content. + assert!(n.observe("\r\x1b[2Kβ ‹ (1s β€’ esc to interrupt)")); + // Subsequent repaints differ only in braille glyph + digits. + assert!(!n.observe("\r\x1b[2Kβ ™ (2s β€’ esc to interrupt)")); + assert!(!n.observe("\r\x1b[2Kβ Ή (3s β€’ esc to interrupt)")); + assert!(!n.observe("\r\x1b[2Kβ Έ (12s β€’ esc to interrupt)")); + } + + #[test] + fn claude_style_ticking_crunch_line_is_noise_after_first_frame() { + let mut n = NoiseScanner::new(); + assert!(n.observe("\r\x1b[2K✻ Crunched for 5s Β· 1.2k tokens Β· esc to interrupt")); + assert!(!n.observe("\r\x1b[2K✽ Crunched for 6s Β· 1.3k tokens Β· esc to interrupt")); + assert!(!n.observe("\r\x1b[2K✻ Crunched for 17s Β· 2.0k tokens Β· esc to interrupt")); + } + + #[test] + fn status_bar_clock_redraw_is_noise_after_first_frame() { + let mut n = NoiseScanner::new(); + assert!(n.observe("\x1b[1;70Hbash | 12:34:56")); + assert!(!n.observe("\x1b[1;70Hbash | 12:34:57")); + assert!(!n.observe("\x1b[1;70Hbash | 12:35:03")); + } + + #[test] + fn pure_cursor_motion_frame_is_noise_even_when_first() { + let mut n = NoiseScanner::new(); + assert!(!n.observe("\x1b[2K\x1b[1;1H\x1b[?25l")); + } + + #[test] + fn braille_only_spinner_frame_is_noise_even_when_first() { + let mut n = NoiseScanner::new(); + assert!(!n.observe("\rβ ‹")); + assert!(!n.observe("\rβ ™")); + } + + #[test] + fn empty_frame_is_noise() { + let mut n = NoiseScanner::new(); + assert!(!n.observe("")); + } + + #[test] + fn distinct_log_lines_are_each_meaningful() { + let mut n = NoiseScanner::new(); + assert!(n.observe("compiling freshell-terminal v0.1.0\n")); + assert!(n.observe("warning: unused variable `x`\n")); + assert!(n.observe(" Finished dev profile\n")); + } + + #[test] + fn streamed_response_text_chunks_are_meaningful() { + // A coding CLI mid-turn streaming a response: every chunk is new prose. + let mut n = NoiseScanner::new(); + assert!(n.observe("The registry keeps a per-terminal ")); + assert!(n.observe("replay ring whose frames are ")); + assert!(n.observe("classified by a persistent scanner.")); + } + + #[test] + fn osc_title_payload_never_contributes_to_the_fingerprint() { + // Terminal-title clock updates are string-body content, not Ground text. + let mut n = NoiseScanner::new(); + assert!(!n.observe("\x1b]0;bash β€” 12:34\x07")); + assert!(!n.observe("\x1b]0;bash β€” 12:35\x07")); + } + + #[test] + fn osc_split_across_frames_does_not_leak_payload_into_fingerprint() { + let mut n = NoiseScanner::new(); + // Frame boundary lands MID-OSC: a stateless scanner would treat + // "ock" in the second frame as Ground text and call it meaningful. + assert!(!n.observe("\x1b]0;my title β€” cl")); + assert!(!n.observe("ock\x07")); + } + + #[test] + fn csi_split_across_frames_does_not_corrupt_ground_text() { + let mut n = NoiseScanner::new(); + assert!(n.observe("hello")); + // "\x1b[2" then "Khello": the K is the CSI final byte, so the second + // frame's Ground text is exactly "hello" β€” already in the ring. + assert!(!n.observe("\x1b[2")); + assert!(!n.observe("Khello")); + } + + #[test] + fn alternating_repaint_cycle_is_noise_via_recent_ring() { + // A big redraw split into two frames A, B repeating: A B A B ... + // Comparing only against the immediately-previous frame would see + // alternation as forever-new; ring membership must not. + let mut n = NoiseScanner::new(); + let a = "\x1b[1;1Hpane one contents"; + let b = "\x1b[2;1Hstatus bar | ready"; + assert!(n.observe(a)); + assert!(n.observe(b)); + for _ in 0..5 { + assert!(!n.observe(a)); + assert!(!n.observe(b)); + } + } + + #[test] + fn digits_only_change_is_noise() { + // NOTE: this deliberately encodes ledger decision AD-1 β€” a workload + // whose only novelty is numeric (curl/dd meters, numeric step logs) + // classifies as noise and IS reaped after the threshold. Recorded as + // an explicit product decision in DEV-0009; do not "fix" this test. + let mut n = NoiseScanner::new(); + assert!(n.observe("Downloading 45% complete")); + assert!(!n.observe("Downloading 46% complete")); + assert!(!n.observe("Downloading 99% complete")); + } + + #[test] + fn codex_shimmer_letter_subset_cycle_is_noise_after_first_cycle() { + // Real codex renders its "Working" header via ratatui cell-diffing + // with a shimmer effect: each animation frame repaints a different + // LETTER SUBSET of the word (measured on codex 0.145.0: ~13-16 + // distinct fingerprints per ~2s cycle, repeating). The ring must be + // large enough to absorb one full cycle; with a ring of 8 these + // frames churn forever and codex is never reaped. + let variants: Vec = (0..16) + .map(|i| { + let word = "Working"; + let end = 1 + (i % word.len()); + let start = i % end; + format!( + "\x1b[3;5H\x1b[38;2;153;153;153m{}\x1b[39m", + &word[start..end] + ) + }) + .collect(); + let mut n = NoiseScanner::new(); + // First cycle: each distinct subset is new content (fail-open). + for v in &variants { + n.observe(v); + } + // Every later cycle must classify as pure repaint noise. + for _ in 0..3 { + for v in &variants { + assert!(!n.observe(v)); + } + } + } + + #[test] + fn ring_evicts_oldest_after_capacity() { + // Pins FIFO eviction: after RECENT_FINGERPRINTS distinct newer + // frames, the oldest fingerprint is forgotten and counts as new + // again (fail-open by design). Fillers must differ in LETTERS + // (digits are stripped from fingerprints), so generate two-letter + // suffixes. The loop count is RECENT_FINGERPRINTS on purpose; the + // test pins the constant's semantics, whatever its value. + let mut n = NoiseScanner::new(); + assert!(n.observe("line zero")); + for i in 0..RECENT_FINGERPRINTS { + let c1 = (b'a' + (i / 26) as u8) as char; + let c2 = (b'a' + (i % 26) as u8) as char; + assert!(n.observe(&format!("filler {c1}{c2}"))); + } + assert!(n.observe("line zero")); + } +} diff --git a/crates/freshell-terminal/src/lib.rs b/crates/freshell-terminal/src/lib.rs index 01f2b69d0..e5bd8caf4 100644 --- a/crates/freshell-terminal/src/lib.rs +++ b/crates/freshell-terminal/src/lib.rs @@ -40,6 +40,9 @@ pub mod chunk_ring; pub mod decode; pub mod fragment; pub mod framing; +#[allow(dead_code)] +// temporary: nothing uses the scanner until Task 2 wires it into registry.rs; Task 2 removes this attribute +mod idle_noise; pub mod output_queue; pub mod pty; pub mod registry; From 2b25af3819b1fb54776c314c923e200437197fd3 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:30:39 -0700 Subject: [PATCH 5/7] fix(terminal): key idle auto-kill on meaningful activity so repaint noise can't exempt detached terminals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit enforce_idle_kills now reads a new private last_meaningful_activity_at clock, refreshed by input writes and by output frames the NoiseScanner classifies as genuinely new content. Spinner frames, ticking elapsed counters, and status-bar redraws no longer reset the reap clock, so an abandoned detached animated TUI is reaped after autoKillIdleMinutes (DEV-0009; second server-side hole behind the 2026-07-25 orphaned- terminal incident β€” the client-side hole was PR #534). Transition-to-detached (explicit detach or socket close) grants one full threshold of grace: the meaningful clock may have gone stale while a watcher was attached (attached terminals are reaper-exempt), and a just-backgrounded terminal must not be killed by the next 30s sweep. Wire-visible lastActivityAt semantics are unchanged: still bumped on every PTY output frame and every input write (terminal-core.md Β§1.3). Attached terminals stay exempt; autoKillIdleMinutes <= 0 stays a no-op. Also drops the temporary #[allow(dead_code)] on mod idle_noise now that the scanner has a consumer. πŸ€– Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- crates/freshell-terminal/src/lib.rs | 2 - crates/freshell-terminal/src/registry.rs | 209 +++++++++++++++++++++-- 2 files changed, 194 insertions(+), 17 deletions(-) diff --git a/crates/freshell-terminal/src/lib.rs b/crates/freshell-terminal/src/lib.rs index e5bd8caf4..f053be332 100644 --- a/crates/freshell-terminal/src/lib.rs +++ b/crates/freshell-terminal/src/lib.rs @@ -40,8 +40,6 @@ pub mod chunk_ring; pub mod decode; pub mod fragment; pub mod framing; -#[allow(dead_code)] -// temporary: nothing uses the scanner until Task 2 wires it into registry.rs; Task 2 removes this attribute mod idle_noise; pub mod output_queue; pub mod pty; diff --git a/crates/freshell-terminal/src/registry.rs b/crates/freshell-terminal/src/registry.rs index 6a8d0c80a..9de7cd5f4 100644 --- a/crates/freshell-terminal/src/registry.rs +++ b/crates/freshell-terminal/src/registry.rs @@ -60,6 +60,7 @@ use crate::batch::{ BatchInputFrame, }; use crate::fragment::terminal_stream_batch_max_bytes; +use crate::idle_noise::NoiseScanner; use crate::pty::{MessageSink, PtyTerminal}; /// Deliver one serverβ†’client message to a single attached connection's socket. @@ -197,6 +198,10 @@ struct TerminalShared { /// The per-terminal stateful VT [`BarrierScanner`] (`replay-ring.ts:48`). Classifies /// each ingested frame in order; its mode/CSI/string state persists across frames. scanner: BarrierScanner, + /// Per-terminal repaint-noise fingerprinter feeding + /// `last_meaningful_activity_at` (DEV-0009). Independent of the barrier + /// scanner: separate state, separate concern (reaping, not batching). + noise: NoiseScanner, /// Highest `seqEnd` produced (drives `attach.ready.headSeq`). head_seq: i64, status: TerminalRunStatus, @@ -208,6 +213,14 @@ struct TerminalShared { exit_code: Option, created_at: i64, last_activity_at: i64, + /// The idle-kill reap clock (DEV-0009): last MEANINGFUL activity β€” user + /// input, or PTY output carrying genuinely new content per + /// [`NoiseScanner`]. Unlike `last_activity_at` (wire-visible via + /// `inventory()`/`DirectoryEntry` and spec-pinned to bump on EVERY + /// output frame, terminal-core.md Β§1.3), repaint noise (spinner frames, + /// ticking counters, status-bar redraws) does not refresh this. + /// Read ONLY by `enforce_idle_kills`. + last_meaningful_activity_at: i64, /// Current PTY geometry + epoch (`Β§5.3`): epoch starts 1, +1 only on a real change after the first client geometry record. cols: u16, rows: u16, @@ -719,6 +732,8 @@ impl TerminalRegistry { /// `enforceIdleKills()` (`terminal-registry.ts:1406-1425`): auto-kill every /// DETACHED **running** terminal idle beyond the configured threshold. /// `auto_kill_idle_minutes() <= 0` is legacy's disabled state -- a no-op. + /// Idleness is measured against `last_meaningful_activity_at` (DEV-0009): + /// self-generated repaint noise does not keep a detached terminal alive. /// "Detached" mirrors `term.clients.size > 0` continue-guard: any attached /// subscriber exempts the terminal regardless of idle time. Returns the /// killed terminal ids (empty when nothing was eligible), for callers that @@ -748,7 +763,11 @@ impl TerminalRegistry { if !s.subscribers.is_empty() { return None; // only detached } - if now.saturating_sub(s.last_activity_at) < idle_threshold_ms { + // DEV-0009: idleness is measured against the MEANINGFUL + // activity clock, not the every-frame last_activity_at β€” + // otherwise a detached animated TUI (spinner / ticking + // counter) is exempt from this sweep forever. + if now.saturating_sub(s.last_meaningful_activity_at) < idle_threshold_ms { return None; // not idle long enough yet } Some(id.clone()) @@ -814,11 +833,13 @@ impl TerminalRegistry { // from `settings.terminal.scrollback` at boot). max_replay_chars: self.scrollback_max_bytes().max(0) as usize, scanner: BarrierScanner::new(), + noise: NoiseScanner::new(), head_seq: 0, status: TerminalRunStatus::Running, exit_code: None, created_at: now, last_activity_at: now, + last_meaningful_activity_at: now, cols: spec.cols, rows: spec.rows, geometry_epoch: 1, @@ -1054,11 +1075,16 @@ impl TerminalRegistry { .map(|h| Arc::clone(&h.shared)) }; if let Some(shared) = shared { - shared - .lock() - .expect("terminal lock") - .subscribers - .remove(&conn_id); + let mut s = shared.lock().expect("terminal lock"); + if s.subscribers.remove(&conn_id).is_some() + && s.subscribers.is_empty() + && s.status == TerminalRunStatus::Running + { + // DEV-0009: a freshly-detached terminal gets a full idle + // threshold of grace β€” its meaningful clock may have expired + // while a watcher was attached (attached => reaper-exempt). + s.last_meaningful_activity_at = s.last_meaningful_activity_at.max(now_ms()); + } } } @@ -1075,11 +1101,20 @@ impl TerminalRegistry { .collect() }; for shared in shareds { - shared - .lock() - .expect("terminal lock") - .subscribers - .remove(&conn_id); + let mut s = shared.lock().expect("terminal lock"); + if s.subscribers.remove(&conn_id).is_some() + && s.subscribers.is_empty() + && s.status == TerminalRunStatus::Running + { + // DEV-0009: a freshly-detached terminal gets a full idle + // threshold of grace β€” its meaningful clock may have expired + // while a watcher was attached (attached => reaper-exempt). + // The `.is_some()` gate is essential here: this sweep visits + // EVERY terminal, and an unconditional bump would reset the + // countdown of unrelated, already-detached terminals on + // every socket close. + s.last_meaningful_activity_at = s.last_meaningful_activity_at.max(now_ms()); + } } } @@ -1094,7 +1129,10 @@ impl TerminalRegistry { let _ = pty.write_input(data); } let mut s = handle.shared.lock().expect("terminal lock"); - s.last_activity_at = now_ms(); + let now = now_ms(); + s.last_activity_at = now; + // User keystrokes are always meaningful (DEV-0009). + s.last_meaningful_activity_at = now; s.mode != "shell" } None => false, @@ -1383,6 +1421,7 @@ impl TerminalRegistry { s.status = TerminalRunStatus::Exited; s.exit_code = Some(exit_code); s.last_activity_at = now; + s.last_meaningful_activity_at = now; let respawn_key = s.create_request_id.clone(); let lifetime_ms = now.saturating_sub(s.created_at); let exit = ServerMessage::TerminalExit(TerminalExit { @@ -1574,11 +1613,13 @@ impl TerminalRegistry { replay_chars: 0, max_replay_chars: self.scrollback_max_bytes().max(0) as usize, scanner: BarrierScanner::new(), + noise: NoiseScanner::new(), head_seq: 0, status: TerminalRunStatus::Running, exit_code: None, created_at, last_activity_at: created_at, + last_meaningful_activity_at: created_at, cols: 120, rows: 30, geometry_epoch: 1, @@ -2148,6 +2189,14 @@ fn ingest(shared: &Arc>, msg: ServerMessage) { let mut s = shared.lock().expect("terminal lock"); s.head_seq = s.head_seq.max(frame.seq_end); s.last_activity_at = now_ms(); + // DEV-0009: only genuinely-new content refreshes the idle-kill reap + // clock. Spinner repaints / ticking counters / status-bar redraws still + // bump the wire-visible last_activity_at above (terminal-core.md Β§1.3 + // holds for every consumer except the reaper) but must not exempt a + // detached terminal from enforce_idle_kills forever. + if s.noise.observe(&frame.data) { + s.last_meaningful_activity_at = s.last_activity_at; + } // Classify with the persistent per-terminal scanner (state persists across frames, // `replay-ring.ts:62-79`). Non-truncated frames (every graded chunk) classify by @@ -2594,12 +2643,15 @@ mod tests { }); } - /// Test-only: force a terminal's `lastActivityAt` to an arbitrary value so - /// idle-kill sweep tests don't need to sleep for real minutes. + /// Test-only: force a terminal's `lastActivityAt` AND its DEV-0009 + /// meaningful-activity reap clock to an arbitrary value so idle-kill + /// sweep tests don't need to sleep for real minutes. fn backdate_last_activity(&self, terminal_id: &str, last_activity_at: i64) { let inner = self.inner.lock().unwrap(); let handle = inner.terminals.get(terminal_id).unwrap(); - handle.shared.lock().unwrap().last_activity_at = last_activity_at; + let mut s = handle.shared.lock().unwrap(); + s.last_activity_at = last_activity_at; + s.last_meaningful_activity_at = last_activity_at; } /// Simulate the reader thread producing one frame (append + fan-out). @@ -3452,6 +3504,133 @@ mod tests { assert_eq!(reg.inventory().len(), 1); } + #[test] + fn enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise() { + let reg = TerminalRegistry::new(); + reg.insert_headless("T", "S"); + reg.set_auto_kill_idle_minutes(1); + // Warm-up: the FIRST paint of a status line is genuinely new content + // and legitimately counts as activity. + reg.feed("T", frame(1, "\r\x1b[2Kβ ‹ (1s β€’ esc to interrupt)", "S")); + reg.backdate_last_activity("T", now_ms() - 10 * 60_000); + // Codex-style repaint noise after the backdate: same status line, + // only the braille glyph and the digits tick. Each frame still bumps + // the wire-visible last_activity_at (unchanged legacy semantics) but + // must NOT refresh the reap clock. + for (i, paint) in [ + "\r\x1b[2Kβ ™ (2s β€’ esc to interrupt)", + "\r\x1b[2Kβ Ή (3s β€’ esc to interrupt)", + "\r\x1b[2Kβ Έ (14s β€’ esc to interrupt)", + "\r\x1b[2Kβ Ό (65s β€’ esc to interrupt)", + ] + .iter() + .enumerate() + { + reg.feed("T", frame(i as i64 + 2, paint, "S")); + } + + let killed = reg.enforce_idle_kills(); + + assert_eq!(killed, vec!["T".to_string()]); + assert!(reg.inventory().is_empty()); + } + + #[test] + fn enforce_idle_kills_spares_detached_terminal_streaming_genuine_output() { + let reg = TerminalRegistry::new(); + reg.insert_headless("T", "S"); + reg.set_auto_kill_idle_minutes(1); + reg.backdate_last_activity("T", now_ms() - 10 * 60_000); + // A long build streaming REAL new log lines: genuine work, must + // survive the sweep even while detached. + reg.feed( + "T", + frame(1, " Compiling freshell-terminal v0.1.0\n", "S"), + ); + reg.feed( + "T", + frame(2, "warning: unused variable `x` in registry.rs\n", "S"), + ); + + let killed = reg.enforce_idle_kills(); + + assert!(killed.is_empty()); + assert_eq!(reg.inventory().len(), 1); + } + + #[test] + fn input_write_resets_the_idle_reap_clock() { + // User keystrokes are always activity (headless => the PTY write is + // skipped but the activity bump still happens, matching input()). + let reg = TerminalRegistry::new(); + reg.insert_headless("T", "S"); + reg.set_auto_kill_idle_minutes(1); + reg.backdate_last_activity("T", now_ms() - 10 * 60_000); + reg.input("T", b"ls\n"); + + let killed = reg.enforce_idle_kills(); + + assert!(killed.is_empty()); + assert_eq!(reg.inventory().len(), 1); + } + + #[test] + fn detach_grants_full_idle_threshold_of_grace() { + // A user may WATCH a spinner-only terminal attached for hours: the + // attached exemption forbids reaping, but nothing refreshes the + // meaningful clock, so it pre-expires underneath them. Detaching + // must therefore grant one full threshold of grace (DEV-0009) β€” + // otherwise the very next 30s sweep kills a terminal the user + // deliberately backgrounded seconds earlier (legacy never reaped it; + // terminal-core.md A13: "terminal stays running"). + let reg = TerminalRegistry::new(); + reg.insert_headless("T", "S"); + let (sink, _seen) = collector(); + let outcome = reg.attach("T", 1, sink, Some("a".into()), 0, false, None); + assert!(outcome.found); + reg.set_auto_kill_idle_minutes(1); + reg.backdate_last_activity("T", now_ms() - 10 * 60_000); + reg.detach("T", 1); + + // Freshly detached: the transition bump spares it a full threshold. + assert!(reg.enforce_idle_kills().is_empty()); + assert_eq!(reg.inventory().len(), 1); + + // Once it goes stale again AFTER the detach, it is reaped normally. + reg.backdate_last_activity("T", now_ms() - 10 * 60_000); + assert_eq!(reg.enforce_idle_kills(), vec!["T".to_string()]); + } + + #[test] + fn disconnect_grants_full_idle_threshold_of_grace() { + // Socket-close cleanup (remove_connection) is the other live + // transition-to-detached path and must grant the same grace as an + // explicit detach. The bump is gated on "this connection actually + // subscribed here AND the set became empty AND status is Running" β€” + // remove_connection iterates EVERY terminal, and an unconditional + // bump would reset unrelated detached terminals' countdowns on + // every socket close. + let reg = TerminalRegistry::new(); + reg.insert_headless("T", "S"); + let (sink, _seen) = collector(); + let outcome = reg.attach("T", 1, sink, Some("a".into()), 0, false, None); + assert!(outcome.found); + // A second, already-detached terminal whose countdown must NOT be + // disturbed by conn 1's disconnect. + reg.insert_headless("U", "S"); + reg.set_auto_kill_idle_minutes(1); + reg.backdate_last_activity("T", now_ms() - 10 * 60_000); + reg.backdate_last_activity("U", now_ms() - 10 * 60_000); + reg.remove_connection(1); + + let killed = reg.enforce_idle_kills(); + + // T was freshly detached by the disconnect => spared one threshold. + // U never had a subscriber => its stale countdown stands => reaped. + assert_eq!(killed, vec!["U".to_string()]); + assert_eq!(reg.inventory().len(), 1); + } + #[test] fn enforce_idle_kills_disabled_when_minutes_zero() { let reg = TerminalRegistry::new(); From 7005105574d0b89a333fd50dfcde2cab86e45342 Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:37:03 -0700 Subject: [PATCH 6/7] docs(oracle): record DEV-0009 idle-reap repaint-noise deviation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ledger entry (status: proposed) for keying enforce_idle_kills on the meaningful-activity clock instead of the every-frame lastActivityAt. Objective defect bar: resource leak (detached animated TUIs were never reapable, 2026-07-25 incident). Pinning tests referenced and passing. πŸ€– Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- port/oracle/DEVIATIONS.md | 48 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/port/oracle/DEVIATIONS.md b/port/oracle/DEVIATIONS.md index 974654a1e..5ec68b452 100644 --- a/port/oracle/DEVIATIONS.md +++ b/port/oracle/DEVIATIONS.md @@ -652,6 +652,54 @@ path itself is intact). - status: accepted (terminals.changed parity CLOSED; terminal.meta.updated open gap, tracked for closure with DEV-0006) +### DEV-0009 β€” idle auto-kill reap clock ignores self-generated repaint noise (original never reaps an animated detached TUI) + +- objective_defect: *resource leak* β€” `server/terminal-registry.ts:1684` bumps `lastActivityAt` + on **every** PTY output frame, and `enforceIdleKills` (`terminal-registry.ts:1406-1425`) keys + idleness on that stamp. Any detached terminal whose program merely repaints (codex's braille + spinner + ticking `(Ns β€’ esc to interrupt)` counter, claude's ticking `⏻ Crunched for Ns` line, + any status-bar clock) refreshes the stamp continuously, so `settings.safety.autoKillIdleMinutes` + can never reap it: the PTY, its child process tree, and its replay buffer are retained + indefinitely β€” precisely the leak the setting exists to prevent. Observed in production + 2026-07-25: 10 detached CLIs alive 10-22h against a 3h threshold (the client-side half of that + incident was PR #534; this entry is the server-side half). +- original_behavior: idleness = `now - lastActivityAt`, where `lastActivityAt` is refreshed by + every PTY output frame regardless of content; a detached animated TUI is exempt from the idle + sweep forever. +- port_behavior: the port keeps `lastActivityAt`'s wire semantics identical (still bumped on + every output frame and every input write β€” terminal-core.md Β§1.3 holds for `inventory`, the + directory projection, and sorting) but gives `enforce_idle_kills` its own reap clock, + `last_meaningful_activity_at` (`crates/freshell-terminal/src/registry.rs`), refreshed by input + writes, by transition-to-detached (a freshly detached or socket-orphaned terminal gets one full + threshold of grace β€” its clock may have gone stale while a watcher was attached, since attached + terminals are reaper-exempt), and by output frames carrying genuinely new content per the + stateful per-terminal `NoiseScanner` (`crates/freshell-terminal/src/idle_noise.rs`): a frame + whose escape-stripped text β€” minus whitespace, ASCII digits, Braille spinner glyphs + (U+2800-U+28FF), and a small spinner-glyph set (`⏻`-family incl. `⏢`, `|/-\`, bullets + `Β·`/`β€’`/`β—¦`, geometric spinners) β€” is empty or fingerprint-identical to one of the 32 most + recent distinct frames counts as repaint noise and does not refresh the reap clock (ring sized + for codex's shimmer animation, which cycles ~13-16 letter-subset fingerprints; measured on + codex 0.145.0). Detection fails open (anything not provably a repeat counts as activity); + attached terminals stay exempt and `autoKillIdleMinutes <= 0` stays disabled, both unchanged. + Known accepted limitation (deliberate): a detached workload whose ONLY output novelty is + numeric (curl/dd-style single-transfer meters, bare numeric step logs) fingerprints identically + and is reaped after the threshold despite being genuine work β€” at fingerprint level such output + is indistinguishable from the ticking counters this deviation exists to defeat; bar-style and + prose-emitting workloads are unaffected. +- fingerprint: behavior/timing-only β€” no wire message, field, or schema change; the only + observable divergence is that the port's idle sweep reaps a detached repaint-only terminal after + the threshold where the original never would (surfaces as a `terminal.killed by=idle` / + `terminal.exit` for such a terminal, and its absence from subsequent inventories). +- pinning_test: `crates/freshell-terminal/src/registry.rs` tests + `enforce_idle_kills_reaps_detached_terminal_with_only_repaint_noise`, + `enforce_idle_kills_spares_detached_terminal_streaming_genuine_output`, + `detach_grants_full_idle_threshold_of_grace`, and + `disconnect_grants_full_idle_threshold_of_grace`, plus the `NoiseScanner` + unit suite in `crates/freshell-terminal/src/idle_noise.rs` (split-escape statefulness, ring + membership, digits-only ticks, codex shimmer letter-subset cycle, first-paint-counts semantics). +- adjudicated_by: pending antagonist review. +- status: proposed. + ## E2E-discovered intentional divergences (EDEV-xx) **Scope β€” READ THIS FIRST.** This section is DELIBERATELY SEPARATE from the DEV-NNNN From f46f19eec62f36554e11094199d57468e9f74a8d Mon Sep 17 00:00:00 2001 From: Dan Shapiro <3732858+danshapiro@users.noreply.github.com> Date: Mon, 27 Jul 2026 01:44:28 -0700 Subject: [PATCH 7/7] docs(oracle): fix DEV-0009 spinner glyphs and stale input() doc comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix two issues in the DEV-0009 entry of port/oracle/DEVIATIONS.md: - Replace wrong glyphs (U+23FB '⏻' and U+23F6 '⏢') with correct ones (U+273B '✻' and U+2736 '✢') in the objective_defect and port_behavior fields. These now match the glyphs used in idle_noise.rs spinner-glyph set. - Update the doc comment on registry.rs:input() to mention both the lastActivityAt bump and the DEV-0009 meaningful-activity reap clock bump that the function performs. Verified with hexdump: the bytes are now e2 9c bb (✻) and e2 9c b6 (✢). πŸ€– Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-authored-by: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- crates/freshell-terminal/src/registry.rs | 2 +- port/oracle/DEVIATIONS.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/freshell-terminal/src/registry.rs b/crates/freshell-terminal/src/registry.rs index 9de7cd5f4..db55bed0b 100644 --- a/crates/freshell-terminal/src/registry.rs +++ b/crates/freshell-terminal/src/registry.rs @@ -1119,7 +1119,7 @@ impl TerminalRegistry { } /// `terminal.input` write path (`terminal-registry.ts:3867-3894`): write bytes to - /// the PTY; bump `lastActivityAt`. No wire reply. + /// the PTY; bump `lastActivityAt` and the DEV-0009 meaningful-activity reap clock. No wire reply. pub fn input(&self, terminal_id: &str, data: &[u8]) { let tapped_mode = { let mut inner = self.inner.lock().expect("registry lock"); diff --git a/port/oracle/DEVIATIONS.md b/port/oracle/DEVIATIONS.md index 5ec68b452..23269214a 100644 --- a/port/oracle/DEVIATIONS.md +++ b/port/oracle/DEVIATIONS.md @@ -657,7 +657,7 @@ path itself is intact). - objective_defect: *resource leak* β€” `server/terminal-registry.ts:1684` bumps `lastActivityAt` on **every** PTY output frame, and `enforceIdleKills` (`terminal-registry.ts:1406-1425`) keys idleness on that stamp. Any detached terminal whose program merely repaints (codex's braille - spinner + ticking `(Ns β€’ esc to interrupt)` counter, claude's ticking `⏻ Crunched for Ns` line, + spinner + ticking `(Ns β€’ esc to interrupt)` counter, claude's ticking `✻ Crunched for Ns` line, any status-bar clock) refreshes the stamp continuously, so `settings.safety.autoKillIdleMinutes` can never reap it: the PTY, its child process tree, and its replay buffer are retained indefinitely β€” precisely the leak the setting exists to prevent. Observed in production @@ -675,7 +675,7 @@ path itself is intact). terminals are reaper-exempt), and by output frames carrying genuinely new content per the stateful per-terminal `NoiseScanner` (`crates/freshell-terminal/src/idle_noise.rs`): a frame whose escape-stripped text β€” minus whitespace, ASCII digits, Braille spinner glyphs - (U+2800-U+28FF), and a small spinner-glyph set (`⏻`-family incl. `⏢`, `|/-\`, bullets + (U+2800-U+28FF), and a small spinner-glyph set (`✻`-family incl. `✢`, `|/-\`, bullets `Β·`/`β€’`/`β—¦`, geometric spinners) β€” is empty or fingerprint-identical to one of the 32 most recent distinct frames counts as repaint noise and does not refresh the reap clock (ring sized for codex's shimmer animation, which cycles ~13-16 letter-subset fingerprints; measured on