From 3f6c6267a4d83669d71e6d4f8228c3f1b33784bb Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Sun, 12 Jul 2026 11:03:07 -0700 Subject: [PATCH 1/3] refactor: move `Lifecycle` into protocol; tighten visibility and naming --- src/app.rs | 15 ++++++++------- src/daemon.rs | 2 +- src/format.rs | 6 +----- src/protocol.rs | 11 ++++++++++- src/supervisor.rs | 12 ++++++------ src/task.rs | 12 +----------- src/ui.rs | 9 ++++----- 7 files changed, 31 insertions(+), 36 deletions(-) diff --git a/src/app.rs b/src/app.rs index aa4c4a5..23fb4b1 100644 --- a/src/app.rs +++ b/src/app.rs @@ -16,10 +16,11 @@ use crossterm::event::{ use crossterm::{execute, style::Print}; use crate::path; -use crate::protocol::{Command, Event, MouseBtn, MouseKind, ScreenView, ScrollAction, TaskView}; +use crate::protocol::{ + Command, Event, Lifecycle, MouseBtn, MouseKind, ScreenView, ScrollAction, TaskView, +}; use crate::session; use crate::supervisor::Supervisor; -use crate::task::Lifecycle; use crate::transport::{ExitIntent, SocketTransport, ThreadTransport, Transport}; use crate::ui; @@ -177,7 +178,7 @@ fn desired_input_modes(attached: Option<&ScreenView>, view_scroll: bool) -> (boo } /// Dashboard grouping bucket: tagged tasks first, then live, then completed. -pub fn bucket(v: &TaskView) -> u8 { +fn bucket(v: &TaskView) -> u8 { if v.tagged { 0 } else if matches!(v.lifecycle, Lifecycle::Ok | Lifecycle::Failed) { @@ -389,7 +390,7 @@ impl App { /// tasks: the unit the scroll window slides over. Windowing rows (not tasks) /// is what keeps headers and their tasks aligned when the list is taller /// than the screen. - pub fn rows(&self) -> Vec { + pub fn list_rows(&self) -> Vec { let mut out = Vec::new(); for (label, idxs) in self.sections() { out.push(Row::Section(label)); @@ -1443,7 +1444,7 @@ mod tests { app.pump(); app.group_mode = GroupMode::Dir; - let rows = app.rows(); + let rows = app.list_rows(); assert_eq!(rows.len(), 4, "two sections, one task each"); assert_eq!(rows[0], Row::Section(app.invocation_label.clone())); assert!(matches!(rows[1], Row::Task(i) if app.views[i].id == 1)); @@ -1469,7 +1470,7 @@ mod tests { let height = 4; for step in 0..10 { app.select_down(); - let rows = app.rows(); + let rows = app.list_rows(); let sel = app.selected_row(&rows).expect("selection always resolves"); let (start, count) = scroll_window(sel, rows.len(), height); assert!( @@ -1479,7 +1480,7 @@ mod tests { } for step in 0..10 { app.select_up(); - let rows = app.rows(); + let rows = app.list_rows(); let sel = app.selected_row(&rows).expect("selection always resolves"); let (start, count) = scroll_window(sel, rows.len(), height); assert!( diff --git a/src/daemon.rs b/src/daemon.rs index c5ac65f..4719fb3 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -235,7 +235,7 @@ pub fn connect_ready_bounded() -> io::Result { } /// Connect to the daemon or start one, then wait up to one second for its socket. -pub fn connect_or_autostart() -> io::Result { +fn connect_or_autostart() -> io::Result { let path = socket_path(); if let Ok(s) = UnixStream::connect(&path) { return Ok(s); diff --git a/src/format.rs b/src/format.rs index 99cbb6f..20908fe 100644 --- a/src/format.rs +++ b/src/format.rs @@ -27,11 +27,7 @@ pub fn truncate(s: &str, max: usize) -> String { return String::new(); } let clean = s.chars().map(|c| if c.is_control() { ' ' } else { c }); - let count = s - .chars() - .filter(|c| !c.is_control()) - .count() - .max(s.chars().count()); + let count = s.chars().count(); if count <= max { return clean.collect(); } diff --git a/src/protocol.rs b/src/protocol.rs index 24d35ff..ef5cdb7 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -9,7 +9,6 @@ use base64::Engine as _; use base64::engine::general_purpose::STANDARD as B64; use crate::frame::{KIND_CONTROL, KIND_HELLO, KIND_SCREEN}; -use crate::task::Lifecycle; /// Wire-protocol version; the handshake rejects mismatched peers. pub const PROTOCOL_VERSION: u32 = 3; @@ -125,6 +124,16 @@ pub enum Event { Status(String), } +/// Process-derived lifecycle state, independent of the user's `tagged` intent. +/// `Idle` means no recent output, not that the process is waiting for input. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Lifecycle { + Active, + Idle, + Ok, + Failed, +} + /// A read-only snapshot of one task: everything a dashboard row needs, with no /// handle into the live process. Time is pre-reduced to `started_ago` and /// `lifecycle` is pre-computed by the core (it owns the clock and the idle diff --git a/src/supervisor.rs b/src/supervisor.rs index 2019e2c..ac3dd1f 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -618,7 +618,7 @@ mod tests { fn wait_for_lifecycle( s: &mut Supervisor, id: u64, - pred: impl Fn(crate::task::Lifecycle) -> bool, + pred: impl Fn(crate::protocol::Lifecycle) -> bool, ) { for _ in 0..200 { s.tick(); @@ -640,7 +640,7 @@ mod tests { /// plus the `Ok` lifecycle is proof of TERM-before-KILL. #[test] fn kill_delivers_term_before_kill() { - use crate::task::Lifecycle; + use crate::protocol::Lifecycle; let dir = scratch("term_first"); let (ready, trapped) = (dir.join("ready"), dir.join("trapped")); let mut s = sup(24, 80); @@ -664,7 +664,7 @@ mod tests { /// reap-driven escalation. `Kill` must never leave an immortal task. #[test] fn term_ignoring_task_escalates_to_kill() { - use crate::task::Lifecycle; + use crate::protocol::Lifecycle; let dir = scratch("escalate"); let ready = dir.join("ready"); let mut s = sup(24, 80); @@ -785,7 +785,7 @@ mod tests { /// gains one line per run). #[test] fn restart_reruns_finished_task_in_place() { - use crate::task::Lifecycle; + use crate::protocol::Lifecycle; let dir = scratch("restart"); let marker = dir.join("marker"); let mut s = sup(24, 80); @@ -819,7 +819,7 @@ mod tests { /// and keeps running. An unknown id gets a notice too, not a panic. #[test] fn restart_refuses_running_task_and_unknown_id() { - use crate::task::Lifecycle; + use crate::protocol::Lifecycle; let mut s = sup(24, 80); s.apply(Command::Spawn { command: "sleep 30".into(), @@ -860,7 +860,7 @@ mod tests { /// fresh screen would be skipped as "unchanged". #[test] fn restart_watched_task_resends_screen() { - use crate::task::Lifecycle; + use crate::protocol::Lifecycle; let mut s = sup(24, 80); s.apply(Command::Spawn { command: "true".into(), diff --git a/src/task.rs b/src/task.rs index 998ce1c..887c863 100644 --- a/src/task.rs +++ b/src/task.rs @@ -13,7 +13,7 @@ use portable_pty::{CommandBuilder, MasterPty, PtySize, native_pty_system}; use rustix::process::{WaitId, WaitIdOptions, waitid}; use crate::core::{Wake, Waker}; -use crate::protocol::{MouseKind, ScrollAction}; +use crate::protocol::{Lifecycle, MouseKind, ScrollAction}; /// Number of history rows retained by each task's terminal grid. const SCROLLBACK: usize = 2000; @@ -143,16 +143,6 @@ pub fn mouse_bytes(screen: &vt100::Screen, kind: MouseKind, col: u16, row: u16) None } -/// Process-derived lifecycle state, independent of the user's `tagged` intent. -/// `Idle` means no recent output, not that the process is waiting for input. -#[derive(Clone, Copy, PartialEq, Eq, Debug)] -pub enum Lifecycle { - Active, - Idle, - Ok, - Failed, -} - pub struct Task { pub id: u64, pub command: String, diff --git a/src/ui.rs b/src/ui.rs index 11be1b9..240bdba 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -11,8 +11,7 @@ use crossterm::{ use crate::app::{App, DirKind, Mode, Row, scroll_window}; use crate::format::{pad, rel_time, truncate}; -use crate::protocol::TaskView; -use crate::task::Lifecycle; +use crate::protocol::{Lifecycle, TaskView}; pub fn render(out: &mut Stdout, app: &mut App) -> io::Result<()> { let mut buf: Vec = Vec::with_capacity(app.cols as usize * app.rows as usize * 3 + 128); @@ -31,7 +30,7 @@ pub fn render(out: &mut Stdout, app: &mut App) -> io::Result<()> { render_session_picker(&mut buf, app)?; } Mode::Disconnected => render_disconnected(&mut buf, app)?, - _ => render_dashboard(&mut buf, app)?, + Mode::Dashboard | Mode::Spawn | Mode::SaveSession => render_dashboard(&mut buf, app)?, } // Repaint only on change: a stable frame (idle tasks, no input) is a no-op, // so there is nothing to flicker and nothing to burn CPU on. @@ -73,13 +72,13 @@ fn render_dashboard(out: &mut impl Write, app: &App) -> io::Result<()> { } } // List region: rows 2..=list_bottom. Command line and footer sit below. - // The section/task rows come pre-flattened from `app.rows()`; the scroll + // The section/task rows come pre-flattened from `app.list_rows()`; the scroll // window slides over them, so the selected row is always drawn however many // tasks the fleet holds. let list_top = 2u16; let list_bottom = rows.saturating_sub(3); let height = (usize::from(list_bottom) + 1).saturating_sub(usize::from(list_top)); - let list = app.rows(); + let list = app.list_rows(); let sel_row = app.selected_row(&list); let (start, count) = scroll_window(sel_row.unwrap_or(0), list.len(), height); From 4db69a3503df83d928f99f71ef088885022444bd Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Sun, 12 Jul 2026 11:03:07 -0700 Subject: [PATCH 2/3] fix: require the hello ack in --kill's socket path; single connect-error report --- src/app.rs | 2 +- src/daemon.rs | 8 ++++++-- src/main.rs | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/app.rs b/src/app.rs index 23fb4b1..fe44be6 100644 --- a/src/app.rs +++ b/src/app.rs @@ -226,7 +226,7 @@ impl App { self.watched = None; self.selected_id = None; self.mode = Mode::Dashboard; - self.status = Some("reconnected to fresh daemon".to_string()); + self.status = Some("reconnected".to_string()); } Err(e) => self.status = Some(format!("reconnect failed: {e}")), } diff --git a/src/daemon.rs b/src/daemon.rs index 4719fb3..52588fe 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -336,14 +336,18 @@ pub fn run_kill() -> io::Result<()> { /// Send a `Shutdown` frame when the lock file contains no usable pid, blocking /// until the daemon closes the socket after stopping its jobs. Hellos first: -/// the daemon refuses commands before the handshake, and this path is same-binary so -/// the versions always match. +/// the daemon refuses commands before the handshake — and the ack must be +/// checked, because the socket can belong to a stale older-version daemon that +/// refuses the hello and drops the `Shutdown` unread. Returning `Ok` there +/// would report a kill that never happened. fn kill_via_socket() -> io::Result<()> { let path = socket_path(); match UnixStream::connect(&path) { Ok(mut s) => { let (kind, payload) = encode_hello(&LaunchContext::here()); write_frame(&mut s, kind, &payload)?; + let (kind, payload) = read_frame(&mut s)?; + check_hello_ack(kind, &payload)?; let (kind, payload) = encode_command(&Command::Shutdown); write_frame(&mut s, kind, &payload)?; let mut buf = [0u8; 256]; diff --git a/src/main.rs b/src/main.rs index 78139c0..39d214c 100644 --- a/src/main.rs +++ b/src/main.rs @@ -148,7 +148,7 @@ fn main() -> io::Result<()> { Ok(a) => a, Err(e) => { eprintln!("fleetcom: could not reach the daemon: {e}"); - return Err(e); + std::process::exit(1); } } }; From e74973e1f086623ad2f18a57b6a784faa42fcea3 Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Sun, 12 Jul 2026 11:21:50 -0700 Subject: [PATCH 3/3] refactor: improve documentation for clarity and conciseness --- src/app.rs | 20 ++++++++++++-------- src/daemon.rs | 8 ++------ src/protocol.rs | 2 +- src/ui.rs | 4 +--- 4 files changed, 16 insertions(+), 18 deletions(-) diff --git a/src/app.rs b/src/app.rs index fe44be6..fa0d9b3 100644 --- a/src/app.rs +++ b/src/app.rs @@ -386,10 +386,8 @@ impl App { self.sections().into_iter().flat_map(|(_, v)| v).collect() } - /// The dashboard list as flat rows, section headers interleaved with their - /// tasks: the unit the scroll window slides over. Windowing rows (not tasks) - /// is what keeps headers and their tasks aligned when the list is taller - /// than the screen. + /// Dashboard rows in render order, with section headers interleaved. + /// Scrolling over rows keeps each header aligned with its tasks. pub fn list_rows(&self) -> Vec { let mut out = Vec::new(); for (label, idxs) in self.sections() { @@ -1676,14 +1674,20 @@ mod tests { let mut out = io::stdout(); let key = |code| KeyEvent::new(code, KeyModifiers::NONE); - app.on_key_attached(&mut out, KeyEvent::new(KeyCode::PageUp, KeyModifiers::SHIFT)) - .unwrap(); + app.on_key_attached( + &mut out, + KeyEvent::new(KeyCode::PageUp, KeyModifiers::SHIFT), + ) + .unwrap(); assert!(app.view_scroll, "Shift+PageUp must enter the scroll view"); app.on_key_attached(&mut out, key(KeyCode::Esc)).unwrap(); assert!(!app.view_scroll, "Esc must return to live"); - app.on_key_attached(&mut out, KeyEvent::new(KeyCode::PageUp, KeyModifiers::CONTROL)) - .unwrap(); + app.on_key_attached( + &mut out, + KeyEvent::new(KeyCode::PageUp, KeyModifiers::CONTROL), + ) + .unwrap(); assert!(app.view_scroll, "Ctrl+PageUp is an entry fallback"); app.on_key_attached(&mut out, key(KeyCode::Char('x'))) .unwrap(); diff --git a/src/daemon.rs b/src/daemon.rs index 52588fe..0601df3 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -334,12 +334,8 @@ pub fn run_kill() -> io::Result<()> { )) } -/// Send a `Shutdown` frame when the lock file contains no usable pid, blocking -/// until the daemon closes the socket after stopping its jobs. Hellos first: -/// the daemon refuses commands before the handshake — and the ack must be -/// checked, because the socket can belong to a stale older-version daemon that -/// refuses the hello and drops the `Shutdown` unread. Returning `Ok` there -/// would report a kill that never happened. +/// Send `Shutdown` when the lock file has no usable pid. Complete the handshake +/// first, then wait for the daemon to close the socket after stopping its jobs. fn kill_via_socket() -> io::Result<()> { let path = socket_path(); match UnixStream::connect(&path) { diff --git a/src/protocol.rs b/src/protocol.rs index ef5cdb7..1d5964a 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -1,4 +1,4 @@ -//! Client-to-core commands and core-to-client snapshots for the Unix socket. +//! Messages shared by the client and core, including their Unix-socket encoding. use std::ffi::{OsStr, OsString}; use std::os::unix::ffi::{OsStrExt, OsStringExt}; diff --git a/src/ui.rs b/src/ui.rs index 240bdba..448e0f1 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -72,9 +72,7 @@ fn render_dashboard(out: &mut impl Write, app: &App) -> io::Result<()> { } } // List region: rows 2..=list_bottom. Command line and footer sit below. - // The section/task rows come pre-flattened from `app.list_rows()`; the scroll - // window slides over them, so the selected row is always drawn however many - // tasks the fleet holds. + // Scroll over section and task rows together to keep the selection visible. let list_top = 2u16; let list_bottom = rows.saturating_sub(3); let height = (usize::from(list_bottom) + 1).saturating_sub(usize::from(list_top));