diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b035eb..b532fc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,13 @@ Everything since `v1.50.0`. Applies to all three crates (`socktop`, `socktop_age - `socktop` consumes `socktop_connector` via a path+version dep — connector changes are testable in-repo before publishing. - wasm examples build against the in-repo connector; note `zellij_socktop_plugin` has pre-existing compile errors and needs its own rework. +### Process kill (PR #40) + +- **Kill a local process from the TUI** (`t` on a selected process, or inside Process Details): btop-style Terminate/Force-kill confirmation. Local agents only — the signal is sent by socktop itself with its own privileges, never over the wire; remote agents never show the option. PID-reuse guarded (the confirmed name must still own the PID at signal time). +- **Agent no longer reports dead processes**: a long-lived sysinfo `System` accumulated every process ever seen (21k+ entries on a 289-process host), inflating memory, per-poll work, and the process count — and keeping killed processes on screen forever. Update agent and client together on machines where the kill feature will be used. +- Killed rows leave the list when the process actually exits and cannot be resurrected by cached agent snapshots; details views for dead processes close themselves, including through parent-navigation chains. +- Selection hint no longer vanishes for long process names; confirmation/info dialogs size to their content. + ### Upgrade notes - **Release/publish order**: `socktop_connector` → `socktop` → agent packages. diff --git a/Cargo.lock b/Cargo.lock index 7bfa0b8..7cb3fbd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2423,6 +2423,7 @@ dependencies = [ "serde", "serde_json", "socktop_connector", + "sysinfo", "tempfile", "tokio", "unicode-width", diff --git a/socktop/Cargo.toml b/socktop/Cargo.toml index 9f7ce20..9ba09d1 100644 --- a/socktop/Cargo.toml +++ b/socktop/Cargo.toml @@ -22,6 +22,9 @@ ratatui = { workspace = true } crossterm = { workspace = true } unicode-width = { workspace = true } anyhow = { workspace = true } +# Local process signalling only (src/proc_kill.rs). The TUI never gathers its +# own metrics — everything on screen comes from the agent over the connector. +sysinfo = { workspace = true } dirs-next = { workspace = true } [dev-dependencies] diff --git a/socktop/src/app.rs b/socktop/src/app.rs index 47e09b9..24a8501 100644 --- a/socktop/src/app.rs +++ b/socktop/src/app.rs @@ -20,6 +20,7 @@ use ratatui::{ use tokio::time::{sleep, timeout}; use crate::history::{PerCoreHistory, push_capped}; +use crate::proc_kill::{KillSignal, kill_local_process}; use crate::retry::{RetryTiming, compute_retry_timing}; use crate::types::Metrics; use crate::ui::cpu::{ @@ -51,6 +52,21 @@ use socktop_connector::{ const MIN_METRICS_INTERVAL_MS: u64 = 100; const MIN_PROCESSES_INTERVAL_MS: u64 = 200; +/// Floor for the post-kill forced refresh delay: just past the agent's +/// DEFAULT `Processes` cache TTL of 1500ms, so the answer reflects the kill +/// instead of the cached snapshot taken before it. The effective delay scales +/// with the user's processes interval — see [`App::proc_refresh_settle`]. +const PROC_CACHE_SETTLE_FLOOR: Duration = Duration::from_millis(1_600); + +/// Margin a tombstone outlives the settle window by. With default intervals +/// this reproduces the original fixed 5s tombstone (1.6s + 3.4s). +const TOMBSTONE_MARGIN: Duration = Duration::from_millis(3_400); + +/// How long to keep re-checking a signalled process for its exit. Long enough +/// to cover a slow shutdown, short enough that a process which plainly ignored +/// the signal keeps its row. +const KILL_WATCH_FOR: Duration = Duration::from_secs(5); + /// Budget for one request/response round trip. Replies are matched to /// requests by order, so a request that never answers would otherwise hang /// `ws.next()` forever and freeze the TUI (raw mode even eats Ctrl+C). @@ -136,6 +152,15 @@ pub struct App { procs_row_peak_cpu: f32, last_procs_poll: Instant, + /// When set, the next metrics tick polls processes regardless of the + /// regular cadence. Used after a kill — see refresh_after_kill. + procs_refresh_due_at: Option, + /// PIDs we have signalled, with the instant we stop watching for their + /// exit. Re-checked each metrics tick — see poll_kill_watch. + kill_watch: Vec<(u32, Instant)>, + /// PIDs confirmed gone after a signal, kept briefly so the agent's cached + /// process list cannot put them back on screen. + killed_gone: Vec<(u32, Instant)>, last_disks_poll: Instant, procs_interval: Duration, disks_interval: Duration, @@ -153,6 +178,10 @@ pub struct App { last_io_write_bytes: Option, // Previous write bytes for delta calculation pub max_process_mem_bytes: u64, // Maximum memory usage observed for current process pub process_details_unsupported: bool, // Track if agent doesn't support process details + /// The agent has successfully answered at least one details request this + /// session. Distinguishes "this agent is too old" from "that process is + /// gone", which arrive over the wire as the same error. + process_details_answered: bool, last_process_details_poll: Instant, last_journal_poll: Instant, process_details_interval: Duration, @@ -165,6 +194,13 @@ pub struct App { // Security / status flags pub is_tls: bool, pub has_token: bool, + // Whether the connected agent is on this machine. Gates the local + // process-kill feature (t = SIGTERM, k = SIGKILL). + pub is_local: bool, + // Pending kill awaiting confirmation: (pid, process name). Which signal is + // sent depends on the button chosen in the confirmation modal, so it isn't + // decided until then. + pending_kill: Option<(u32, String)>, // --compact: pin the compact layout regardless of window size. Without it the // layout switches on its own once the window is too short for the Disks pane. @@ -222,6 +258,9 @@ impl App { procs_filter_dirty: true, procs_row_cache: Vec::new(), procs_row_peak_cpu: 0.0, + procs_refresh_due_at: None, + kill_watch: Vec::new(), + killed_gone: Vec::new(), last_procs_poll: Instant::now() .checked_sub(Duration::from_secs(2)) .unwrap_or_else(Instant::now), // trigger immediately on first loop @@ -242,6 +281,7 @@ impl App { last_io_write_bytes: None, max_process_mem_bytes: 0, process_details_unsupported: false, + process_details_answered: false, last_process_details_poll: Instant::now() .checked_sub(Duration::from_secs(10)) .unwrap_or_else(Instant::now), @@ -255,6 +295,8 @@ impl App { verify_hostname: false, is_tls: false, has_token: false, + is_local: false, + pending_kill: None, force_compact: false, header_title: String::new(), header_intervals_text: String::new(), @@ -306,6 +348,232 @@ impl App { self } + /// Enable the local process-kill feature. Only set true when the agent has + /// been verified to be on this machine (see [`crate::local`]). + pub fn with_local(mut self, is_local: bool) -> Self { + self.is_local = is_local; + self + } + + /// Look up the display name of a process by PID. Prefers the details + /// payload, which is the only source that has a name for a process not in + /// the top-N list — e.g. after walking up to a parent from the details + /// modal. + fn process_name_for_pid(&self, pid: u32) -> Option { + if let Some(details) = self + .process_details + .as_ref() + .filter(|d| d.process.pid == pid) + { + return Some(details.process.name.clone()); + } + self.last_metrics + .as_ref()? + .top_processes + .iter() + .find(|p| p.pid == pid) + .map(|p| p.name.clone()) + } + + /// Raise the kill confirmation for `pid`. No-op unless the agent is on this + /// machine — the same gate the keybinding uses, repeated here because this + /// is also reachable from the details modal. + fn prompt_kill(&mut self, pid: u32) { + if !self.is_local { + return; + } + let name = self + .process_name_for_pid(pid) + .unwrap_or_else(|| "process".to_string()); + self.modal_manager.push_modal(ModalType::Confirmation { + title: "Confirm signal".to_string(), + message: format!("Send a signal to {name} (PID {pid})?"), + confirm_text: "Terminate".to_string(), + cancel_text: "Cancel".to_string(), + }); + self.pending_kill = Some((pid, name)); + } + + /// Signal the process the confirmation was raised for, then report the + /// outcome. Pops the confirmation first so the result lands on top of + /// whatever was underneath it (the process list, or the details modal). + fn run_pending_kill(&mut self, signal: KillSignal) { + let Some((pid, name)) = self.pending_kill.take() else { + return; + }; + self.modal_manager.pop_modal(); + // The name shown in the confirmation doubles as the reuse guard: if + // the PID has been recycled since, the kill is refused. The "process" + // fallback from prompt_kill means "name unknown" — no guard possible. + let expected = (name != "process").then_some(name.as_str()); + let (title, message) = match kill_local_process(pid, expected, signal) { + Ok(()) => { + self.refresh_after_kill(pid); + ( + "Signal sent".to_string(), + format!("Sent {} to {name} (PID {pid}).", signal.label()), + ) + } + Err(e) => ("Signal failed".to_string(), e), + }; + self.modal_manager + .push_modal(ModalType::Info { title, message }); + } + + /// Bring the process list back in step with reality after a signal. + /// + /// A single check at signal time is not enough, which is what the first + /// version got wrong: SIGTERM is a *request*, so the process is usually + /// still alive for the few hundred milliseconds it takes to wind down. The + /// row therefore stayed put, and the list looked like the kill had done + /// nothing. + /// + /// So the PID goes on a watch list, re-checked every metrics tick until it + /// exits (or the watch expires). Confirmed-gone PIDs are also remembered + /// briefly — see `killed_gone` — because the agent serves `Processes` from + /// a 1500ms cache and would otherwise hand back a snapshot taken before + /// the kill and put the row straight back. + fn refresh_after_kill(&mut self, pid: u32) { + self.kill_watch.retain(|(p, _)| *p != pid); + self.kill_watch.push((pid, Instant::now() + KILL_WATCH_FOR)); + // Check once right now: SIGKILL, and anything already exiting, is gone + // by the time the confirmation is dismissed. + self.poll_kill_watch(); + self.procs_refresh_due_at = Some(Instant::now() + self.proc_refresh_settle()); + } + + /// How long the post-kill forced refresh waits, and the base of the + /// tombstone lifetime. Scales with the user's processes interval: someone + /// who raised the agent's Processes TTL will have raised their client + /// interval to match (there is no point polling faster than the cache), + /// so the interval is the best client-side signal for how stale an agent + /// snapshot can be. Never below the default-TTL floor. + fn proc_refresh_settle(&self) -> Duration { + PROC_CACHE_SETTLE_FLOOR.max(self.procs_interval) + } + + /// How long a confirmed-dead PID is remembered, so a cached agent snapshot + /// taken before the kill cannot resurrect its row. Must outlive the settle + /// window plus one round trip, hence settle + margin. + fn kill_tombstone_for(&self) -> Duration { + self.proc_refresh_settle() + TOMBSTONE_MARGIN + } + + /// Re-check the processes we have signalled and retire the rows of any that + /// have since exited. Cheap: one `/proc` lookup per watched PID, and the + /// list is almost always empty. + fn poll_kill_watch(&mut self) { + if self.kill_watch.is_empty() { + return; + } + let now = Instant::now(); + let mut gone = Vec::new(); + self.kill_watch.retain(|(pid, deadline)| { + if !crate::proc_kill::process_exists(*pid) { + gone.push(*pid); + return false; + } + // Still alive. Keep watching until the deadline — a process that + // ignores the signal outright should keep its row. + now < *deadline + }); + for pid in gone { + self.forget_process_row(pid); + // Nothing left to show details for. Also matters mechanically: the + // details poll keys off the selection, which forget_process_row + // just cleared, so leaving the modal open would freeze it on the + // dead process's last sample. + self.close_details_for_gone_process(pid); + self.killed_gone.push((pid, now)); + } + let tombstone_for = self.kill_tombstone_for(); + self.killed_gone + .retain(|(_, at)| now.duration_since(*at) < tombstone_for); + } + + /// Drop rows for processes we have confirmed dead. Applied to every process + /// list the agent sends, because its cached snapshot can predate the kill. + fn drop_tombstoned_rows(&mut self) { + if self.killed_gone.is_empty() { + return; + } + let now = Instant::now(); + let tombstone_for = self.kill_tombstone_for(); + self.killed_gone + .retain(|(_, at)| now.duration_since(*at) < tombstone_for); + let pids: Vec = self.killed_gone.iter().map(|(p, _)| *p).collect(); + for pid in pids { + self.forget_process_row(pid); + } + } + + /// Give up a selection whose process is no longer in the list. `Processes` + /// carries every process, not a top-N window, so a PID that is absent has + /// genuinely gone — and a selection pointing at it means the hint offers to + /// kill a corpse and `t` reports "no longer exists". + fn drop_vanished_selection(&mut self) { + let Some(pid) = self.selected_process_pid else { + return; + }; + let present = self + .last_metrics + .as_ref() + .is_some_and(|m| m.top_processes.iter().any(|p| p.pid == pid)); + if !present { + self.selected_process_pid = None; + self.selected_process_index = None; + } + } + + /// Close the details view for a process that no longer exists, and drop the + /// data collected for it. + /// + /// A parent-navigation chain can leave another details view underneath + /// (child → P → parent killed): the resurfacing view must resume polling, + /// so retarget the selection to it — the same thing SwitchToParentProcess + /// does on the way down. Without this the child view came back with no + /// selection (forget_process_row had just cleared it) and wiped data, and + /// the selection-gated details poll never refilled it: a frozen, orphaned + /// window. + fn close_details_for_gone_process(&mut self, pid: u32) { + if self.modal_manager.close_process_details(pid) { + self.clear_process_details(); + if let Some(next_pid) = self.modal_manager.topmost_process_details() { + self.selected_process_pid = Some(next_pid); + // Fire the details poll on the next tick rather than waiting + // out the interval. + self.last_process_details_poll = Instant::now() + .checked_sub(self.process_details_interval) + .unwrap_or_else(Instant::now); + } + } + } + + /// Drop a process from the cached view without waiting for the agent, and + /// give up any selection pointing at it — a hint offering to kill a process + /// that no longer exists is worse than no hint. + fn forget_process_row(&mut self, pid: u32) { + let Some(m) = self.last_metrics.as_mut() else { + return; + }; + let before = m.top_processes.len(); + m.top_processes.retain(|p| p.pid != pid); + if m.top_processes.len() == before { + return; // wasn't on screen; nothing to reconcile + } + // Keep the header's "(N total)" honest until the next real poll. + m.process_count = m.process_count.map(|c| c.saturating_sub(1)); + if self.selected_process_pid == Some(pid) { + self.selected_process_pid = None; + self.selected_process_index = None; + } + self.invalidate_procs_filter(); + if let Some(mm) = self.last_metrics.as_ref() { + self.procs_row_peak_cpu = + crate::ui::processes::rebuild_row_cache(mm, &mut self.procs_row_cache); + } + } + /// Show a connection error modal pub fn show_connection_error(&mut self, message: String) { if !self.modal_manager.is_active() { @@ -757,18 +1025,47 @@ impl App { continue; // Skip normal key processing } ModalAction::Cancel | ModalAction::Dismiss => { - // If ProcessDetails modal was dismissed, clear the data to save resources - if let Some(crate::ui::modal::ModalType::ProcessDetails { - .. - }) = self.modal_manager.current_modal() + // If a ProcessDetails view is what we landed on, + // clear the stale data AND point the poll at it — + // Esc-ing back from a parent view otherwise left + // the selection on the parent, refilling the + // child-titled view with the parent's data. + if let Some(crate::ui::modal::ModalType::ProcessDetails { pid }) = + self.modal_manager.current_modal() { + let pid = *pid; self.clear_process_details(); + self.selected_process_pid = Some(pid); + self.last_process_details_poll = Instant::now() + .checked_sub(self.process_details_interval) + .unwrap_or_else(Instant::now); } + // Abandon any pending kill the user backed out of. + self.pending_kill = None; // Modal was dismissed, skip normal key processing continue; } ModalAction::Confirm => { - // Handle confirmation action here if needed in the future + // The only confirmation in the app is the + // process-kill prompt; Confirm is the polite + // signal, ConfirmForce the forceful one. + if self.pending_kill.is_some() { + self.run_pending_kill(KillSignal::Term); + continue; + } + } + ModalAction::ConfirmForce => { + if self.pending_kill.is_some() { + self.run_pending_kill(KillSignal::Kill); + continue; + } + } + ModalAction::KillSelected(pid) => { + // `t` from inside the details modal. The + // confirmation stacks on top of it, so + // cancelling returns to the details view. + self.prompt_kill(pid); + continue; } ModalAction::SwitchToParentProcess(_current_pid) => { // Get parent PID from current process details @@ -880,6 +1177,20 @@ impl App { self.modal_manager.push_modal(ModalType::Help); } + // Kill the selected process — local agents only. `t` is the + // one kill key everywhere: `k` scrolls the thread table in + // the details modal so it could not be reused there, and one + // key for both entry points is one thing to remember. + // SIGTERM vs SIGKILL is chosen in the confirmation modal. + if self.is_local + && !self.modal_manager.is_active() + && matches!(k.code, KeyCode::Char('t') | KeyCode::Char('T')) + && let Some(pid) = self.selected_process_pid + { + self.prompt_kill(pid); + continue; + } + // Per-core scroll via keys (Up/Down/PageUp/PageDown/Home/End) let sz = terminal.size()?; let area = Rect::new(0, 0, sz.width, sz.height); @@ -1117,8 +1428,21 @@ impl App { self.consecutive_request_timeouts = 0; self.update_with_metrics(m); - // Only poll processes every 2s - if self.last_procs_poll.elapsed() >= self.procs_interval { + // A process signalled a moment ago may have exited + // since. Checked here, on every tick, so its row goes + // as soon as it is actually gone rather than at the + // next full process poll. + self.poll_kill_watch(); + + // Only poll processes every 2s — unless a kill asked for + // a refresh, which jumps the queue. + let forced = self + .procs_refresh_due_at + .is_some_and(|due| Instant::now() >= due); + if forced || self.last_procs_poll.elapsed() >= self.procs_interval { + if forced { + self.procs_refresh_due_at = None; + } let mut updated = false; match timeout(REQUEST_TIMEOUT, ws.request(AgentRequest::Processes)) .await @@ -1148,6 +1472,14 @@ impl App { &mut self.procs_row_cache, ); } + // The agent's snapshot can predate a kill by up + // to its cache TTL, so strip anything we already + // know is gone before it reaches the screen. + self.drop_tombstoned_rows(); + // And a selection whose process is no longer in + // the list would otherwise still be the target + // of `t`. + self.drop_vanished_selection(); } self.last_procs_poll = Instant::now(); } @@ -1252,11 +1584,44 @@ impl App { self.process_details = Some(details); self.process_details_unsupported = false; + // This agent demonstrably answers + // details requests, which is what + // lets the error arm below read a + // later failure as "that process is + // gone" rather than "old agent". + self.process_details_answered = true; } Ok(Err(_)) => { - // Agent responded with an error: endpoint - // not supported. - self.process_details_unsupported = true; + // An error reply means one of two very + // different things, and the wire cannot + // tell them apart: the agent lacks the + // endpoint, or this PID is gone (the + // agent sends {"error":"Process N not + // found"}, which fails to deserialize + // and arrives here identically). + // + // If the agent has already answered a + // details request this session, the + // endpoint plainly works, so the PID is + // the problem — close the view instead + // of claiming the agent needs updating. + // + // Unless the process is still in the + // agent's own list: then this error is a + // transient (socket blip, torn frame), + // not a death — keep the view and let + // the next poll retry. + if self.process_details_answered { + let still_listed = + self.last_metrics.as_ref().is_some_and(|m| { + m.top_processes.iter().any(|p| p.pid == pid) + }); + if !still_listed { + self.close_details_for_gone_process(pid); + } + } else { + self.process_details_unsupported = true; + } } Err(_) => { // No reply at all: old agents IGNORE @@ -1583,6 +1948,7 @@ impl App { filtered_indices: &self.procs_filtered, cached_rows: &self.procs_row_cache, peak_cpu: self.procs_row_peak_cpu, + is_local: self.is_local, }, ); @@ -1603,6 +1969,7 @@ impl App { }, max_mem_bytes: self.max_process_mem_bytes, unsupported: self.process_details_unsupported, + is_local: self.is_local, }, ); } @@ -1614,3 +1981,418 @@ impl Default for App { Self::new() } } + +#[cfg(test)] +mod kill_refresh_tests { + use super::*; + use socktop_connector::{Metrics, ProcessInfo}; + + fn proc(pid: u32, name: &str) -> ProcessInfo { + ProcessInfo { + pid, + name: name.into(), + cpu_usage: 1.0, + mem_bytes: 1_000, + } + } + + fn app_with(pids: &[u32]) -> App { + let mut app = App::new(); + app.last_metrics = Some(Metrics { + sampled_at_ms: None, + cpu_total: 0.0, + cpu_per_core: vec![], + mem_total: 1_000_000, + mem_used: 0, + swap_total: 0, + swap_used: 0, + hostname: "t".into(), + cpu_temp_c: None, + disks: vec![], + networks: vec![], + top_processes: pids.iter().map(|p| proc(*p, "victim")).collect(), + gpus: None, + process_count: Some(pids.len()), + }); + app + } + + #[test] + fn dropping_a_row_updates_the_list_count_and_selection() { + let mut app = app_with(&[1, 2, 3]); + app.selected_process_pid = Some(2); + app.selected_process_index = Some(1); + + app.forget_process_row(2); + + let m = app.last_metrics.as_ref().unwrap(); + assert_eq!( + m.top_processes.iter().map(|p| p.pid).collect::>(), + vec![1, 3] + ); + assert_eq!(m.process_count, Some(2), "header count went stale"); + assert_eq!( + app.selected_process_pid, None, + "selection still points at a dead process" + ); + assert_eq!(app.selected_process_index, None); + } + + /// A process that was never on screen (outside the top-N) must not decrement + /// the total or disturb the selection. + #[test] + fn dropping_an_offscreen_row_changes_nothing() { + let mut app = app_with(&[1, 2, 3]); + app.selected_process_pid = Some(1); + app.forget_process_row(999); + let m = app.last_metrics.as_ref().unwrap(); + assert_eq!(m.top_processes.len(), 3); + assert_eq!(m.process_count, Some(3)); + assert_eq!(app.selected_process_pid, Some(1)); + } + + /// A dead process disappears immediately, and a refresh is still scheduled + /// so the agent's own view catches up past its cache TTL. + #[test] + fn a_confirmed_dead_process_leaves_at_once() { + let mut child = std::process::Command::new("true").spawn().expect("spawn"); + let pid = child.id(); + child.wait().expect("reap"); + + let mut app = app_with(&[pid, 4242]); + app.refresh_after_kill(pid); + + let m = app.last_metrics.as_ref().unwrap(); + assert_eq!( + m.top_processes.iter().map(|p| p.pid).collect::>(), + vec![4242], + "a process known to be gone should not still be listed" + ); + assert!(app.procs_refresh_due_at.is_some(), "no refresh scheduled"); + } + + /// A process that survived the signal keeps its row — better a row that is + /// still true than one that vanishes and comes back. + #[test] + fn a_surviving_process_keeps_its_row_until_the_refresh() { + let mut child = std::process::Command::new("sleep") + .arg("30") + .spawn() + .expect("spawn"); + let pid = child.id(); + + let mut app = app_with(&[pid]); + app.refresh_after_kill(pid); + let listed = app + .last_metrics + .as_ref() + .unwrap() + .top_processes + .iter() + .any(|p| p.pid == pid); + + let _ = child.kill(); + let _ = child.wait(); + + assert!(listed, "row for a live process was removed optimistically"); + assert!(app.procs_refresh_due_at.is_some()); + } + + /// The scheduled refresh must land after the agent's process cache TTL, + /// or it just re-reads the pre-kill snapshot. + #[test] + fn the_forced_refresh_waits_out_the_agent_cache() { + assert!( + PROC_CACHE_SETTLE_FLOOR >= Duration::from_millis(1_500), + "agent serves Processes from a 1500ms cache by default" + ); + } + + /// Users who raise the agent's Processes TTL raise the client interval to + /// match, so the settle window (and the tombstone that must outlive it) + /// scales with the interval instead of assuming the default TTL. + #[test] + fn settle_and_tombstone_scale_with_the_processes_interval() { + // The default processes interval is 2s, which already exceeds the + // 1.6s floor — so the default settle is the interval itself. + let mut app = App::new(); + assert_eq!(app.proc_refresh_settle(), Duration::from_secs(2)); + + app = app.with_intervals(None, Some(10_000)); + assert_eq!(app.proc_refresh_settle(), Duration::from_secs(10)); + assert!(app.kill_tombstone_for() > app.proc_refresh_settle()); + + // A tiny interval never drops the settle below the default-TTL floor. + app = app.with_intervals(None, Some(200)); + assert_eq!(app.proc_refresh_settle(), PROC_CACHE_SETTLE_FLOOR); + } +} + +#[cfg(test)] +mod parent_chain_tests { + use super::*; + use crate::ui::modal::ModalType; + + /// Kill a parent reached via P-navigation: the child's view resurfaces and + /// must resume polling. Reported as: "I can still see the orphaned window + /// if I open a process, hit P, then terminate that process with t". + #[test] + fn killing_a_navigated_to_parent_retargets_the_child_view() { + let mut app = App::new(); + let (child, parent) = (200u32, 100u32); + app.modal_manager + .push_modal(ModalType::ProcessDetails { pid: child }); + app.modal_manager + .push_modal(ModalType::ProcessDetails { pid: parent }); + // The kill flow stacks the "Signal sent" Info on top, and the watch + // usually confirms the death while it is still up. + app.modal_manager.push_modal(ModalType::Info { + title: "Signal sent".into(), + message: "Sent SIGTERM".into(), + }); + // forget_process_row has already cleared the selection by this point. + app.selected_process_pid = None; + + app.close_details_for_gone_process(parent); + + assert_eq!( + app.selected_process_pid, + Some(child), + "resurfaced child view has no selection: its poll never runs and \ + the window sits frozen" + ); + assert_eq!(app.modal_manager.topmost_process_details(), Some(child)); + assert!( + app.last_process_details_poll.elapsed() >= app.process_details_interval, + "poll should be due immediately" + ); + } +} + +#[cfg(test)] +mod details_close_tests { + use super::*; + use crate::ui::modal::ModalType; + use socktop_connector::{Metrics, ProcessInfo}; + + fn app_viewing(pid: u32) -> App { + let mut app = App::new(); + app.last_metrics = Some(Metrics { + sampled_at_ms: None, + cpu_total: 0.0, + cpu_per_core: vec![], + mem_total: 1_000_000, + mem_used: 0, + swap_total: 0, + swap_used: 0, + hostname: "t".into(), + cpu_temp_c: None, + disks: vec![], + networks: vec![], + top_processes: vec![ProcessInfo { + pid, + name: "victim".into(), + cpu_usage: 1.0, + mem_bytes: 1_000, + }], + gpus: None, + process_count: Some(1), + }); + app.selected_process_pid = Some(pid); + app.selected_process_index = Some(0); + app.modal_manager + .push_modal(ModalType::ProcessDetails { pid }); + app.max_process_mem_bytes = 12_345; // stand-in for collected history + app + } + + /// Killing the process you are looking at should not leave you staring at + /// its details — especially since the details poll keys off the selection, + /// which is cleared at the same time. + #[test] + fn killing_the_viewed_process_closes_its_details() { + let mut child = std::process::Command::new("true").spawn().expect("spawn"); + let pid = child.id(); + child.wait().expect("reap"); + + let mut app = app_viewing(pid); + app.refresh_after_kill(pid); + + assert!( + !app.modal_manager.is_active(), + "details modal stayed open for a dead process" + ); + assert_eq!( + app.max_process_mem_bytes, 0, + "details state was not cleared" + ); + assert!(app.last_metrics.as_ref().unwrap().top_processes.is_empty()); + } + + /// A process that survived the signal keeps both its row and its details. + #[test] + fn a_surviving_process_keeps_its_details_open() { + let mut child = std::process::Command::new("sleep") + .arg("30") + .spawn() + .expect("spawn"); + let pid = child.id(); + + let mut app = app_viewing(pid); + app.refresh_after_kill(pid); + let still_open = app.modal_manager.is_active(); + + let _ = child.kill(); + let _ = child.wait(); + + assert!(still_open, "closed the details of a process still running"); + } +} + +#[cfg(test)] +mod kill_watch_tests { + use super::*; + use socktop_connector::{Metrics, ProcessInfo}; + + fn metrics_with(pids: &[(u32, &str)]) -> Metrics { + Metrics { + sampled_at_ms: None, + cpu_total: 0.0, + cpu_per_core: vec![], + mem_total: 1_000_000, + mem_used: 0, + swap_total: 0, + swap_used: 0, + hostname: "t".into(), + cpu_temp_c: None, + disks: vec![], + networks: vec![], + top_processes: pids + .iter() + .map(|(pid, name)| ProcessInfo { + pid: *pid, + name: (*name).into(), + cpu_usage: 1.0, + mem_bytes: 1_000, + }) + .collect(), + gpus: None, + process_count: Some(pids.len()), + } + } + + fn listed(app: &App, pid: u32) -> bool { + app.last_metrics + .as_ref() + .is_some_and(|m| m.top_processes.iter().any(|p| p.pid == pid)) + } + + /// The reported bug: SIGTERM is a request, so the process is normally still + /// alive at signal time. The row must go when it actually exits, not stay + /// until the next full poll. + #[test] + fn a_row_goes_as_soon_as_the_process_actually_exits() { + let mut child = std::process::Command::new("sleep") + .arg("30") + .spawn() + .expect("spawn"); + let pid = child.id(); + + let mut app = App::new(); + app.last_metrics = Some(metrics_with(&[(pid, "sleep"), (1, "init")])); + app.selected_process_pid = Some(pid); + + // Signalled, but it has not exited yet: the row stays. + app.refresh_after_kill(pid); + assert!( + listed(&app, pid), + "row vanished while the process was alive" + ); + + // It exits (as a SIGTERM'd process does, a moment later). + let _ = child.kill(); + let _ = child.wait(); + + // The next tick notices. + app.poll_kill_watch(); + assert!(!listed(&app, pid), "row survived the process exiting"); + assert_eq!(app.selected_process_pid, None, "selection left on a corpse"); + } + + /// Also reported: after terminating, the row came back. The agent serves + /// `Processes` from a 1500ms cache, so its next answer can predate the kill. + #[test] + fn a_stale_agent_snapshot_cannot_resurrect_a_killed_process() { + let mut child = std::process::Command::new("true").spawn().expect("spawn"); + let pid = child.id(); + child.wait().expect("reap"); + + let mut app = App::new(); + app.last_metrics = Some(metrics_with(&[(pid, "true"), (1, "init")])); + app.refresh_after_kill(pid); + assert!(!listed(&app, pid), "confirmed-dead row should be gone"); + + // The agent answers with a snapshot taken before the kill. + app.last_metrics = Some(metrics_with(&[(pid, "true"), (1, "init")])); + app.drop_tombstoned_rows(); + assert!(!listed(&app, pid), "stale snapshot put the row back"); + assert!(listed(&app, 1), "unrelated processes must survive"); + } + + /// His exact path: find the process with `/`, then kill it. The filtered + /// view is derived from the same list, so it must lose the row too. + #[test] + fn a_search_filtered_view_loses_the_row_as_well() { + let mut child = std::process::Command::new("true").spawn().expect("spawn"); + let pid = child.id(); + child.wait().expect("reap"); + + let mut app = App::new(); + app.last_metrics = Some(metrics_with(&[(pid, "victim"), (1, "init")])); + app.process_search_query = "victim".into(); + assert_eq!( + app.procs_filter().len(), + 1, + "search should match the victim" + ); + + app.refresh_after_kill(pid); + + assert!( + app.procs_filter().is_empty(), + "killed process still present in the filtered list" + ); + } + + #[test] + fn a_selection_that_leaves_the_list_is_dropped() { + let mut app = App::new(); + app.last_metrics = Some(metrics_with(&[(1, "init")])); + app.selected_process_pid = Some(4242); + app.selected_process_index = Some(7); + app.drop_vanished_selection(); + assert_eq!(app.selected_process_pid, None); + assert_eq!(app.selected_process_index, None); + } + + #[test] + fn a_selection_still_in_the_list_is_kept() { + let mut app = App::new(); + app.last_metrics = Some(metrics_with(&[(1, "init")])); + app.selected_process_pid = Some(1); + app.drop_vanished_selection(); + assert_eq!(app.selected_process_pid, Some(1)); + } + + /// A process that ignores the signal must not be watched forever. + #[test] + fn the_watch_expires() { + let mut app = App::new(); + app.last_metrics = Some(metrics_with(&[(1, "init")])); + // Already-expired deadline, for a PID that certainly exists (ourselves). + let me = std::process::id(); + app.kill_watch.push((me, Instant::now())); + app.poll_kill_watch(); + assert!(app.kill_watch.is_empty(), "expired watch was not dropped"); + } +} diff --git a/socktop/src/local.rs b/socktop/src/local.rs new file mode 100644 index 0000000..857c465 --- /dev/null +++ b/socktop/src/local.rs @@ -0,0 +1,85 @@ +//! Detection of whether the connected agent is running on this same machine. +//! +//! Process-kill is only offered for *local* agents. The reasoning is a +//! security one: the PIDs shown in the UI are reported by the agent, and when +//! the user asks to kill one, socktop sends the signal with its OWN local OS +//! privileges (a direct syscall — never over the network; see [`crate::proc_kill`]). +//! A PID is therefore only meaningful — and only safe to act on — when the +//! agent lives on this machine. If we acted on a remote agent's PIDs we would +//! be signalling whatever unrelated *local* process happened to share that +//! number. +//! +//! An address is considered local when it is loopback, or when we can bind an +//! ephemeral socket to it: a bind only succeeds for an address assigned to one +//! of this host's own network interfaces, so it also covers the case of an +//! agent reached over this machine's LAN IP. Detection fails closed — any +//! parse/resolution failure, or any resolved address that is not local, +//! disables the feature. + +use std::net::{IpAddr, ToSocketAddrs, UdpSocket}; + +/// Returns true only if the agent reached at `ws_url` is on this machine. +pub fn agent_is_local(ws_url: &str) -> bool { + let Ok(parsed) = url::Url::parse(ws_url) else { + return false; + }; + match parsed.host() { + // IP literals can be checked directly without any name resolution. + Some(url::Host::Ipv4(ip)) => ip_is_local(IpAddr::V4(ip)), + Some(url::Host::Ipv6(ip)) => ip_is_local(IpAddr::V6(ip)), + // A hostname (e.g. "localhost", or a LAN name) must resolve, and every + // address it resolves to must be local. ws=80, wss=443 are the known + // default ports; an explicit port in the URL is honored. + Some(url::Host::Domain(domain)) => { + let port = parsed.port_or_known_default().unwrap_or(0); + match (domain, port).to_socket_addrs() { + Ok(addrs) => { + let mut saw_any = false; + for addr in addrs { + saw_any = true; + if !ip_is_local(addr.ip()) { + return false; + } + } + saw_any + } + Err(_) => false, + } + } + None => false, + } +} + +/// An address is local if it is loopback, or if we can bind an ephemeral +/// socket to it (only possible for an address on one of our own interfaces). +/// Port 0 requests an ephemeral port and sends no traffic. +fn ip_is_local(ip: IpAddr) -> bool { + ip.is_loopback() || UdpSocket::bind((ip, 0)).is_ok() +} + +#[cfg(test)] +mod tests { + use super::agent_is_local; + + #[test] + fn loopback_hosts_are_local() { + assert!(agent_is_local("ws://127.0.0.1:3000/ws")); + assert!(agent_is_local("ws://localhost:3000/ws")); + assert!(agent_is_local("ws://[::1]:3000/ws")); + assert!(agent_is_local("wss://127.0.0.1/ws")); + } + + #[test] + fn public_addresses_are_not_local() { + // 8.8.8.8 is not assigned to any local interface. + assert!(!agent_is_local("ws://8.8.8.8:3000/ws")); + // Documentation-range address, guaranteed not bound locally. + assert!(!agent_is_local("ws://203.0.113.1:3000/ws")); + } + + #[test] + fn garbage_fails_closed() { + assert!(!agent_is_local("not a url")); + assert!(!agent_is_local("")); + } +} diff --git a/socktop/src/main.rs b/socktop/src/main.rs index 78e62ef..a7ffca4 100644 --- a/socktop/src/main.rs +++ b/socktop/src/main.rs @@ -2,6 +2,8 @@ mod app; mod history; +mod local; +mod proc_kill; mod profiles; mod retry; mod types; @@ -321,10 +323,15 @@ async fn main() -> Result<(), Box> { let is_tls = url.starts_with("wss://"); let has_token = url.contains("token="); + // Only enable local process-kill when the agent is verified to be on this + // machine; otherwise on-screen PIDs refer to a remote host and acting on + // them locally would signal the wrong process. See local::agent_is_local. + let is_local = local::agent_is_local(&url); let mut app = App::new() .with_intervals(metrics_interval_ms, processes_interval_ms) .with_status(is_tls, has_token) - .with_compact(parsed.compact); + .with_compact(parsed.compact) + .with_local(is_local); if parsed.dry_run { return Ok(()); } @@ -404,7 +411,12 @@ async fn run_demo_mode( } Err(e) => return Err(e.into()), }; - let mut app = App::new().with_compact(compact); + // Demo mode runs the real agent on loopback, so its PIDs are real local + // processes — enable the local process-kill feature, gated the same way as + // the normal connect path (loopback resolves local). + let mut app = App::new() + .with_compact(compact) + .with_local(local::agent_is_local(&url)); // Demo mode connects to localhost, so disable hostname verification tokio::select! { res=app.run(&url,None,false)=>{ drop(child); res } _=tokio::signal::ctrl_c()=>{ drop(child); Ok(()) } } } diff --git a/socktop/src/proc_kill.rs b/socktop/src/proc_kill.rs new file mode 100644 index 0000000..9cf84a9 --- /dev/null +++ b/socktop/src/proc_kill.rs @@ -0,0 +1,181 @@ +//! Local process termination. +//! +//! Signals are sent by socktop itself, using this process's own OS privileges, +//! via a direct `sysinfo` call. Nothing is transmitted to the agent — the +//! agent and connector have no kill capability at all. This code path is only +//! reachable once the agent has been verified to be local (see +//! [`crate::local`]), which guarantees the PID refers to a process on this +//! machine. + +use sysinfo::{ProcessRefreshKind, ProcessesToUpdate, Signal, System}; + +/// The signals socktop can send. Deliberately limited to the two btop-style +/// primaries; no arbitrary-signal chooser. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum KillSignal { + /// SIGTERM — polite request to terminate. + Term, + /// SIGKILL — forceful, cannot be caught. + Kill, +} + +impl KillSignal { + fn as_sysinfo(self) -> Signal { + match self { + KillSignal::Term => Signal::Term, + KillSignal::Kill => Signal::Kill, + } + } + + /// Human-facing label for confirmation/result messages. + pub fn label(self) -> &'static str { + match self { + KillSignal::Term => "SIGTERM", + KillSignal::Kill => "SIGKILL", + } + } +} + +/// Is `pid` still a live local process? +/// +/// A zombie counts as gone: after a kill the entry can linger until the parent +/// reaps it, and showing a row for a process that no longer runs is exactly the +/// staleness this check exists to avoid. +pub fn process_exists(pid: u32) -> bool { + let spid = sysinfo::Pid::from_u32(pid); + let mut sys = System::new(); + sys.refresh_processes_specifics( + ProcessesToUpdate::Some(&[spid]), + false, + ProcessRefreshKind::nothing(), + ); + match sys.process(spid) { + Some(p) => p.status() != sysinfo::ProcessStatus::Zombie, + None => false, + } +} + +/// Send `signal` to local process `pid`. Returns `Ok(())` on success, or an +/// `Err` with a human-readable reason (process gone, PID reused, permission +/// denied, signal unsupported on this platform). +/// +/// `expected_name`, when given, is compared against the process that owns the +/// PID **right now**: the PID came from an agent snapshot and the confirmation +/// dialog can sit open indefinitely, so by signal time the kernel may have +/// recycled the number for an unrelated process. Both names come from the +/// same sysinfo source, so a live, unchanged target compares equal. +pub fn kill_local_process( + pid: u32, + expected_name: Option<&str>, + signal: KillSignal, +) -> Result<(), String> { + let spid = sysinfo::Pid::from_u32(pid); + + // Refresh just this one PID — we don't need a full process scan to signal it. + let mut sys = System::new(); + sys.refresh_processes_specifics( + ProcessesToUpdate::Some(&[spid]), + false, + ProcessRefreshKind::nothing(), + ); + + let Some(proc_) = sys.process(spid) else { + return Err(format!("Process {pid} no longer exists")); + }; + + if let Some(expected) = expected_name { + let current = proc_.name().to_string_lossy(); + if current != expected { + return Err(format!( + "PID {pid} now belongs to \"{current}\", not \"{expected}\" — \ + not signalling. Reselect the process and try again." + )); + } + } + + match proc_.kill_with(signal.as_sysinfo()) { + Some(true) => Ok(()), + Some(false) => Err(format!( + "Could not send {} to PID {pid} (permission denied?)", + signal.label() + )), + None => Err(format!( + "{} is not supported on this platform", + signal.label() + )), + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::process::Command; + use std::time::{Duration, Instant}; + + /// The path that matters: a real, live, local process must actually receive + /// the signal. Exercises the `refresh_processes_specifics` lookup as well — + /// if that call does not populate the process map, `sys.process()` returns + /// None and a live PID is reported as "no longer exists". + #[test] + fn signals_a_real_child_process() { + let mut child = Command::new("sleep") + .arg("30") + .spawn() + .expect("spawn sleep for the test"); + let pid = child.id(); + + let result = kill_local_process(pid, Some("sleep"), KillSignal::Term); + + // Reap on every path before asserting, so a failing assert cannot leak a + // 30s sleep and cannot trip clippy's zombie_processes lint. + let deadline = Instant::now() + Duration::from_secs(5); + let mut exited = false; + while Instant::now() < deadline { + if matches!(child.try_wait(), Ok(Some(_))) { + exited = true; + break; + } + std::thread::sleep(Duration::from_millis(20)); + } + if !exited { + let _ = child.kill(); + } + let _ = child.wait(); + + assert!(result.is_ok(), "kill_local_process returned {result:?}"); + assert!( + exited, + "SIGTERM was reported sent but the child never exited" + ); + } + + /// The reuse guard: a live PID whose owner does not match the name the + /// user confirmed must NOT be signalled. This also proves the name is + /// populated under ProcessRefreshKind::nothing() — if it weren't, the + /// matching-name test above would fail instead. + #[test] + fn refuses_a_pid_owned_by_a_different_process() { + let mut child = Command::new("sleep") + .arg("30") + .spawn() + .expect("spawn sleep"); + let pid = child.id(); + + let result = kill_local_process(pid, Some("firefox"), KillSignal::Term); + + let _ = child.kill(); + let _ = child.wait(); + + let err = result.expect_err("signalled a process under the wrong name"); + assert!(err.contains("firefox") && err.contains("sleep"), "{err}"); + } + + #[test] + fn reports_a_pid_that_is_gone() { + let mut child = Command::new("true").spawn().expect("spawn true"); + let pid = child.id(); + child.wait().expect("reap"); + // The PID is now free; signalling it must fail cleanly, not panic. + assert!(kill_local_process(pid, None, KillSignal::Term).is_err()); + } +} diff --git a/socktop/src/ui/modal.rs b/socktop/src/ui/modal.rs index ae39e46..b35dc25 100644 --- a/socktop/src/ui/modal.rs +++ b/socktop/src/ui/modal.rs @@ -1,6 +1,10 @@ //! Modal window system for socktop TUI application -use super::theme::MODAL_DIM_BG; +use super::fit; +use super::theme::{ + BTN_EXIT_BG_ACTIVE, BTN_RETRY_BG_ACTIVE, MODAL_BG, MODAL_BORDER_FG, MODAL_DIM_BG, MODAL_FG, + MODAL_TITLE_FG, +}; use crossterm::event::KeyCode; use ratatui::{ Frame, @@ -26,6 +30,10 @@ pub struct ModalManager { pub help_scroll_offset: usize, } +/// Key hints shown under the confirmation buttons. Also sets the minimum +/// width of that dialog — sizing from the question alone clipped this line. +const CONFIRM_HINT: &str = "Tab ← → choose · Enter run · Esc cancel"; + impl ModalManager { pub fn new() -> Self { Self { @@ -83,6 +91,52 @@ impl ModalManager { } m } + /// Close the details view for `pid` WHEREVER it sits in the stack. + /// Returns whether anything was closed. + /// + /// Not just the top: killing from inside the details view stacks the + /// "Signal sent" Info modal on top of it, and a SIGKILL victim is usually + /// confirmed dead on the very next tick — while that Info is still up. A + /// top-only check missed the close, and since a gone PID is processed + /// once, the details view stayed open (frozen on the dead process's last + /// sample) with nothing left to ever close it. + /// + /// Per-PID matching keeps the parent-navigation property: only the dead + /// process's view goes; parent views underneath are other processes that + /// may still be alive and close themselves the same way. + pub fn close_process_details(&mut self, pid: u32) -> bool { + let was_top = + matches!(self.stack.last(), Some(ModalType::ProcessDetails { pid: p }) if *p == pid); + let before = self.stack.len(); + self.stack + .retain(|m| !matches!(m, ModalType::ProcessDetails { pid: p } if *p == pid)); + if self.stack.len() == before { + return false; + } + // Mirror pop_modal's focus bookkeeping when the top changed. + if was_top && let Some(next) = self.stack.last() { + self.active_button = match next { + ModalType::ConnectionError { .. } => ModalButton::Retry, + ModalType::ProcessDetails { .. } => ModalButton::Ok, + ModalType::About => ModalButton::Ok, + ModalType::Help => ModalButton::Ok, + ModalType::Confirmation { .. } => ModalButton::Confirm, + ModalType::Info { .. } => ModalButton::Ok, + }; + } + true + } + + /// PID of the uppermost ProcessDetails view, looking through any + /// Info/Confirmation stacked above it. What the user will land on when + /// transient modals are dismissed. + pub fn topmost_process_details(&self) -> Option { + self.stack.iter().rev().find_map(|m| match m { + ModalType::ProcessDetails { pid } => Some(*pid), + _ => None, + }) + } + pub fn update_connection_error_countdown(&mut self, new_countdown: Option) { if let Some(ModalType::ConnectionError { auto_retry_countdown, @@ -110,6 +164,16 @@ impl ModalManager { self.prev_button(); ModalAction::None } + // Kill the process being viewed. `t` rather than `k` because `k` + // scrolls the thread table in this modal — and using the same key + // here as on the processes pane means one thing to remember. + KeyCode::Char('t') | KeyCode::Char('T') => { + if let Some(ModalType::ProcessDetails { pid }) = self.stack.last() { + ModalAction::KillSelected(*pid) + } else { + ModalAction::None + } + } KeyCode::Char('r') | KeyCode::Char('R') => { if matches!(self.stack.last(), Some(ModalType::ConnectionError { .. })) { ModalAction::RetryConnection @@ -241,7 +305,16 @@ impl ModalManager { ModalAction::Dismiss } (Some(ModalType::Confirmation { .. }), ModalButton::Confirm) => ModalAction::Confirm, - (Some(ModalType::Confirmation { .. }), ModalButton::Cancel) => ModalAction::Cancel, + (Some(ModalType::Confirmation { .. }), ModalButton::ConfirmForce) => { + ModalAction::ConfirmForce + } + (Some(ModalType::Confirmation { .. }), ModalButton::Cancel) => { + // Pop here so Enter-on-Cancel behaves like Esc (which pops in + // handle_key); the app's Cancel handler can then assume the + // modal is already gone. + self.pop_modal(); + ModalAction::Cancel + } (Some(ModalType::Info { .. }), ModalButton::Ok) => { self.pop_modal(); ModalAction::Dismiss @@ -253,12 +326,29 @@ impl ModalManager { self.active_button = match (&self.stack.last(), &self.active_button) { (Some(ModalType::ConnectionError { .. }), ModalButton::Retry) => ModalButton::Exit, (Some(ModalType::ConnectionError { .. }), ModalButton::Exit) => ModalButton::Retry, - (Some(ModalType::Confirmation { .. }), ModalButton::Confirm) => ModalButton::Cancel, + // Confirmation cycles through three: the safe affirmative, the + // escalated one, then cancel. + (Some(ModalType::Confirmation { .. }), ModalButton::Confirm) => { + ModalButton::ConfirmForce + } + (Some(ModalType::Confirmation { .. }), ModalButton::ConfirmForce) => { + ModalButton::Cancel + } (Some(ModalType::Confirmation { .. }), ModalButton::Cancel) => ModalButton::Confirm, _ => self.active_button.clone(), }; } fn prev_button(&mut self) { + // Confirmation has three buttons, so stepping back is not the same as + // stepping forward; everything else is a two-way toggle. + if let Some(ModalType::Confirmation { .. }) = self.stack.last() { + self.active_button = match self.active_button { + ModalButton::Confirm => ModalButton::Cancel, + ModalButton::ConfirmForce => ModalButton::Confirm, + _ => ModalButton::ConfirmForce, + }; + return; + } self.next_button(); } @@ -280,6 +370,150 @@ impl ModalManager { ); } + /// Wrap `text` to at most `width` columns on word boundaries, so a dialog + /// can be sized from its content instead of guessing. + fn wrap_cols(text: &str, width: u16) -> Vec { + let mut lines = Vec::new(); + let mut current = String::new(); + for word in text.split_whitespace() { + let candidate = if current.is_empty() { + word.to_string() + } else { + format!("{current} {word}") + }; + if fit::cols(&candidate) <= width || current.is_empty() { + current = candidate; + } else { + lines.push(std::mem::take(&mut current)); + current = word.to_string(); + } + } + if !current.is_empty() { + lines.push(current); + } + if lines.is_empty() { + lines.push(String::new()); + } + lines + } + + /// A centered box just big enough for `message` plus `footer_rows` of + /// buttons/hints. Never exceeds the screen, and never gets so narrow that + /// the title is clipped. + /// + /// `min_content_w` is the width the footer needs. Sizing from the message + /// alone clipped the key-hint line, which is longer than most questions. + fn dialog_rect(area: Rect, message: &str, footer_rows: u16, min_content_w: u16) -> Rect { + // 2 border columns + 2 columns of breathing room on each side. + const CHROME_W: u16 = 6; + const MAX_TEXT_W: u16 = 64; + const MIN_TEXT_W: u16 = 24; + + let avail_text = area.width.saturating_sub(CHROME_W).max(1); + let text_w = fit::cols(message) + .min(MAX_TEXT_W) + .min(avail_text) + .max(MIN_TEXT_W.min(avail_text)); + let lines = Self::wrap_cols(message, text_w); + let widest = lines + .iter() + .map(|l| fit::cols(l)) + .max() + .unwrap_or(text_w) + .max(min_content_w.min(avail_text)); + + let width = (widest + CHROME_W).min(area.width); + // borders + blank + message + blank + footer + let height = (lines.len() as u16 + footer_rows + 4).min(area.height); + Rect { + x: area.x + (area.width.saturating_sub(width)) / 2, + y: area.y + (area.height.saturating_sub(height)) / 2, + width, + height, + } + } + + /// Shared chrome for the small dialogs: themed border, centered message + /// with real padding, and the footer row(s) returned for the caller to + /// fill with buttons. + /// + /// The old versions laid their content out over `area` rather than the + /// block's inner rect, which put the first line of text on top of the + /// border and pushed the buttons against the frame. + fn render_dialog_frame( + f: &mut Frame, + area: Rect, + title: &str, + message: &str, + footer_rows: u16, + ) -> Rect { + let block = Block::default() + .title( + Line::from(format!(" {title} ")).style( + Style::default() + .fg(MODAL_TITLE_FG) + .add_modifier(Modifier::BOLD), + ), + ) + .borders(Borders::ALL) + .border_style(Style::default().fg(MODAL_BORDER_FG)) + .style(Style::default().bg(MODAL_BG)); + let inner = block.inner(area); + f.render_widget(block, area); + + // Pad one column each side so text never touches the border. + let padded = Rect { + x: inner.x + 1, + y: inner.y, + width: inner.width.saturating_sub(2), + height: inner.height, + }; + let rows = Layout::default() + .direction(Direction::Vertical) + .constraints([ + Constraint::Length(1), // breathing room under the title + Constraint::Min(1), // message + Constraint::Length(1), // gap above the footer + Constraint::Length(footer_rows), // buttons / hints + ]) + .split(padded); + + f.render_widget( + Paragraph::new(message) + .style(Style::default().fg(MODAL_FG)) + .alignment(Alignment::Center) + .wrap(Wrap { trim: true }), + rows[1], + ); + rows[3] + } + + /// One button, sized to its label and centered in `area`. + fn render_button(f: &mut Frame, area: Rect, label: &str, active: bool, accent: Color) { + let style = if active { + Style::default() + .bg(accent) + .fg(MODAL_BG) + .add_modifier(Modifier::BOLD) + } else { + Style::default().fg(accent) + }; + let text = format!(" {label} "); + let w = fit::cols(&text).min(area.width); + let btn = Rect { + x: area.x + (area.width.saturating_sub(w)) / 2, + y: area.y, + width: w, + height: 1, + }; + f.render_widget( + Paragraph::new(text) + .style(style) + .alignment(Alignment::Center), + btn, + ); + } + fn render_modal_content(&mut self, f: &mut Frame, modal: &ModalType, data: ProcessModalData) { let area = f.area(); // Different sizes for different modal types @@ -296,6 +530,13 @@ impl ModalManager { // Help modal uses medium size self.centered_rect(70, 80, area) } + // Confirmation and Info are one-question dialogs. A fixed 70%x50% + // box left a short question floating in a mostly-empty pane, so + // these size themselves to their content instead. + ModalType::Confirmation { message, .. } => { + Self::dialog_rect(area, message, 3, fit::cols(CONFIRM_HINT)) + } + ModalType::Info { message, .. } => Self::dialog_rect(area, message, 1, 16), _ => { // Other modals use smaller size self.centered_rect(70, 50, area) @@ -340,86 +581,64 @@ impl ModalManager { confirm_text: &str, cancel_text: &str, ) { - let chunks = Layout::default() + // Three buttons + a key hint line. + let footer = Self::render_dialog_frame(f, area, title, message, 3); + let rows = Layout::default() .direction(Direction::Vertical) - .constraints([Constraint::Min(1), Constraint::Length(3)]) - .split(area); - let block = Block::default() - .title(format!(" {title} ")) - .borders(Borders::ALL) - .style(Style::default().bg(Color::Black)); - f.render_widget(block, area); - f.render_widget( - Paragraph::new(message) - .style(Style::default().fg(Color::White)) - .alignment(Alignment::Center) - .wrap(Wrap { trim: true }), - chunks[0], - ); - let buttons = Layout::default() + .constraints([ + Constraint::Length(1), // buttons + Constraint::Length(1), // spacer + Constraint::Length(1), // key hints + ]) + .split(footer); + + let cols = Layout::default() .direction(Direction::Horizontal) - .constraints([Constraint::Percentage(50), Constraint::Percentage(50)]) - .split(chunks[1]); - let confirm_style = if self.active_button == ModalButton::Confirm { - Style::default() - .bg(Color::Green) - .fg(Color::Black) - .add_modifier(Modifier::BOLD) - } else { - Style::default().fg(Color::Green) - }; - let cancel_style = if self.active_button == ModalButton::Cancel { - Style::default() - .bg(Color::Red) - .fg(Color::Black) - .add_modifier(Modifier::BOLD) - } else { - Style::default().fg(Color::Red) - }; - f.render_widget( - Paragraph::new(confirm_text) - .style(confirm_style) - .alignment(Alignment::Center), - buttons[0], + .constraints([ + Constraint::Ratio(1, 3), + Constraint::Ratio(1, 3), + Constraint::Ratio(1, 3), + ]) + .split(rows[0]); + + Self::render_button( + f, + cols[0], + confirm_text, + self.active_button == ModalButton::Confirm, + BTN_RETRY_BG_ACTIVE, ); + Self::render_button( + f, + cols[1], + "Force kill", + self.active_button == ModalButton::ConfirmForce, + MODAL_TITLE_FG, + ); + Self::render_button( + f, + cols[2], + cancel_text, + self.active_button == ModalButton::Cancel, + BTN_EXIT_BG_ACTIVE, + ); + f.render_widget( - Paragraph::new(cancel_text) - .style(cancel_style) + Paragraph::new(CONFIRM_HINT) + .style(Style::default().fg(MODAL_FG).add_modifier(Modifier::DIM)) .alignment(Alignment::Center), - buttons[1], + rows[2], ); } fn render_info(&self, f: &mut Frame, area: Rect, title: &str, message: &str) { - let chunks = Layout::default() - .direction(Direction::Vertical) - .constraints([Constraint::Min(1), Constraint::Length(3)]) - .split(area); - let block = Block::default() - .title(format!(" {title} ")) - .borders(Borders::ALL) - .style(Style::default().bg(Color::Black)); - f.render_widget(block, area); - f.render_widget( - Paragraph::new(message) - .style(Style::default().fg(Color::White)) - .alignment(Alignment::Center) - .wrap(Wrap { trim: true }), - chunks[0], - ); - let ok_style = if self.active_button == ModalButton::Ok { - Style::default() - .bg(Color::Blue) - .fg(Color::White) - .add_modifier(Modifier::BOLD) - } else { - Style::default().fg(Color::Blue) - }; - f.render_widget( - Paragraph::new("[ Enter ] OK") - .style(ok_style) - .alignment(Alignment::Center), - chunks[1], + let footer = Self::render_dialog_frame(f, area, title, message, 1); + Self::render_button( + f, + footer, + "Enter — OK", + self.active_button == ModalButton::Ok, + BTN_RETRY_BG_ACTIVE, ); } @@ -505,6 +724,9 @@ impl ModalManager { " ↑/↓ ............ Select/navigate processes", " Enter .......... Open Process Details", " x/X ............ Clear selection", + " t .............. Signal selected process — local agent only", + " (also works inside Process Details; the prompt", + " offers Terminate/SIGTERM or Force kill/SIGKILL)", " Click header ... Sort by column (CPU/Mem)", " Click row ...... Select process", "", @@ -632,3 +854,254 @@ impl ModalManager { .split(vert[1])[1] } } + +#[cfg(test)] +mod confirm_tests { + use super::*; + + fn confirm_modal() -> ModalManager { + let mut m = ModalManager::new(); + m.push_modal(ModalType::Confirmation { + title: "Confirm signal".into(), + message: "Send a signal to bash (PID 42)?".into(), + confirm_text: "Terminate".into(), + cancel_text: "Cancel".into(), + }); + m + } + + /// The safe option is focused first, so a reflexive Enter terminates rather + /// than force-kills. + #[test] + fn opens_on_the_safe_option() { + let mut m = confirm_modal(); + assert_eq!(m.active_button, ModalButton::Confirm); + assert_eq!(m.handle_key(KeyCode::Enter), ModalAction::Confirm); + } + + #[test] + fn tab_cycles_all_three_buttons_forward() { + let mut m = confirm_modal(); + m.handle_key(KeyCode::Tab); + assert_eq!(m.active_button, ModalButton::ConfirmForce); + m.handle_key(KeyCode::Tab); + assert_eq!(m.active_button, ModalButton::Cancel); + m.handle_key(KeyCode::Tab); + assert_eq!(m.active_button, ModalButton::Confirm); + } + + /// With three buttons, back is not the same as forward — the old + /// prev_button just called next_button, which only worked for two. + #[test] + fn shift_tab_cycles_backward() { + let mut m = confirm_modal(); + m.handle_key(KeyCode::BackTab); + assert_eq!(m.active_button, ModalButton::Cancel); + m.handle_key(KeyCode::BackTab); + assert_eq!(m.active_button, ModalButton::ConfirmForce); + m.handle_key(KeyCode::BackTab); + assert_eq!(m.active_button, ModalButton::Confirm); + } + + #[test] + fn force_kill_reports_its_own_action() { + let mut m = confirm_modal(); + m.handle_key(KeyCode::Tab); + assert_eq!(m.handle_key(KeyCode::Enter), ModalAction::ConfirmForce); + } + + #[test] + fn escape_cancels_and_closes() { + let mut m = confirm_modal(); + assert_eq!(m.handle_key(KeyCode::Esc), ModalAction::Cancel); + assert!(!m.is_active()); + } + + /// Enter on Cancel must behave like Esc, including closing the modal. + #[test] + fn enter_on_cancel_closes_too() { + let mut m = confirm_modal(); + m.handle_key(KeyCode::Tab); + m.handle_key(KeyCode::Tab); + assert_eq!(m.handle_key(KeyCode::Enter), ModalAction::Cancel); + assert!(!m.is_active()); + } + + /// `t` inside process details asks the app to raise the kill prompt for the + /// process being viewed — not for whatever is selected in the list behind it. + #[test] + fn t_in_process_details_targets_that_pid() { + let mut m = ModalManager::new(); + m.push_modal(ModalType::ProcessDetails { pid: 4242 }); + assert_eq!( + m.handle_key(KeyCode::Char('t')), + ModalAction::KillSelected(4242) + ); + } + + /// `k` still scrolls the thread table, which is why `t` is the kill key. + #[test] + fn k_in_process_details_still_scrolls() { + let mut m = ModalManager::new(); + m.push_modal(ModalType::ProcessDetails { pid: 1 }); + m.thread_scroll_max = 5; + m.handle_key(KeyCode::Char('j')); + assert_eq!(m.thread_scroll_offset, 1); + assert_eq!(m.handle_key(KeyCode::Char('k')), ModalAction::Handled); + assert_eq!(m.thread_scroll_offset, 0); + } + + #[test] + fn t_elsewhere_is_not_a_kill() { + let mut m = ModalManager::new(); + m.push_modal(ModalType::Help); + assert_eq!(m.handle_key(KeyCode::Char('t')), ModalAction::None); + } + + /// A one-line question must not be handed a half-screen box. + #[test] + fn dialog_is_sized_to_its_content() { + let screen = Rect::new(0, 0, 120, 40); + let r = ModalManager::dialog_rect( + screen, + "Send a signal to bash (PID 42)?", + 3, + fit::cols(CONFIRM_HINT), + ); + assert!(r.width < screen.width, "dialog took the full width"); + assert!(r.height <= 12, "dialog was {} rows tall", r.height); + assert!(r.height >= 7, "dialog too short to hold its own footer"); + // Centered to within the rounding of integer division. + let center_delta = (r.x + r.width / 2) as i32 - (screen.width / 2) as i32; + assert!(center_delta.abs() <= 1, "off-center by {center_delta}"); + } + + #[test] + fn dialog_never_exceeds_a_small_screen() { + let screen = Rect::new(0, 0, 20, 8); + let long = "a".repeat(400); + let r = ModalManager::dialog_rect(screen, &long, 3, fit::cols(CONFIRM_HINT)); + assert!(r.width <= screen.width && r.height <= screen.height); + } +} + +#[cfg(test)] +mod button_style_tests { + use super::*; + use ratatui::Terminal; + use ratatui::backend::TestBackend; + + /// The focused button must be the highlighted one — the whole point of the + /// three-button layout is that you can see which action Enter will run. + #[test] + fn focus_moves_the_highlight() { + let msg = "Send a signal to bash (PID 42)?"; + let mut m = ModalManager::new(); + m.push_modal(ModalType::Confirmation { + title: "Confirm signal".into(), + message: msg.into(), + confirm_text: "Terminate".into(), + cancel_text: "Cancel".into(), + }); + + // Background colors present on the button row, per focused button. + let bgs = |m: &ModalManager| -> Vec { + let screen = Rect::new(0, 0, 100, 30); + let area = ModalManager::dialog_rect(screen, msg, 3, fit::cols(CONFIRM_HINT)); + let mut t = Terminal::new(TestBackend::new(100, 30)).unwrap(); + t.draw(|f| { + m.render_confirmation(f, area, "Confirm signal", msg, "Terminate", "Cancel") + }) + .unwrap(); + let buf = t.backend().buffer(); + // Buttons sit on the first footer row: title, gap, message, gap. + let row = area.y + 4; + (area.x..area.x + area.width) + .map(|x| buf[(x, row)].bg) + .collect() + }; + + let terminate_focused = bgs(&m); + assert!( + terminate_focused.contains(&BTN_RETRY_BG_ACTIVE), + "Terminate should be highlighted when focused" + ); + assert!( + !terminate_focused.contains(&BTN_EXIT_BG_ACTIVE), + "Cancel must not be highlighted while Terminate has focus" + ); + + m.handle_key(KeyCode::Tab); + m.handle_key(KeyCode::Tab); + let cancel_focused = bgs(&m); + assert!( + cancel_focused.contains(&BTN_EXIT_BG_ACTIVE), + "Cancel should be highlighted after two Tabs" + ); + assert!( + !cancel_focused.contains(&BTN_RETRY_BG_ACTIVE), + "Terminate must not stay highlighted" + ); + } +} + +#[cfg(test)] +mod close_details_tests { + use super::*; + + #[test] + fn closes_the_view_for_that_pid() { + let mut m = ModalManager::new(); + m.push_modal(ModalType::ProcessDetails { pid: 4242 }); + assert!(m.close_process_details(4242)); + assert!(!m.is_active()); + } + + #[test] + fn leaves_a_different_pid_alone() { + let mut m = ModalManager::new(); + m.push_modal(ModalType::ProcessDetails { pid: 4242 }); + assert!(!m.close_process_details(1)); + assert!(m.is_active()); + } + + /// Walking up to a parent stacks details views. Only the dead process's + /// view goes — whichever position it holds — and the survivor stays put. + #[test] + fn closes_only_the_dead_pids_view_in_a_parent_chain() { + let mut m = ModalManager::new(); + m.push_modal(ModalType::ProcessDetails { pid: 100 }); // parent + m.push_modal(ModalType::ProcessDetails { pid: 200 }); // child, on top + + // Parent dies while the child is viewed: its view is removed from + // UNDER the top, so closing the child later lands on the process list + // instead of a frozen corpse view. + assert!(m.close_process_details(100)); + assert!(matches!( + m.current_modal(), + Some(ModalType::ProcessDetails { pid: 200 }) + )); + assert!(m.close_process_details(200)); + assert!(!m.is_active()); + } + + /// The F1 regression: killing from inside the details view stacks the + /// "Signal sent" Info on top, and the death is usually confirmed while + /// that Info is still up. The details view must close anyway — a top-only + /// check left it open forever, frozen on the dead process. + #[test] + fn closes_details_beneath_a_stacked_info_modal() { + let mut m = ModalManager::new(); + m.push_modal(ModalType::ProcessDetails { pid: 7 }); + m.push_modal(ModalType::Info { + title: "Signal sent".into(), + message: "Sent SIGKILL".into(), + }); + + assert!(m.close_process_details(7)); + // The Info survives on top; dismissing it lands on the process list. + assert!(matches!(m.current_modal(), Some(ModalType::Info { .. }))); + m.pop_modal(); + assert!(!m.is_active()); + } +} diff --git a/socktop/src/ui/modal_process.rs b/socktop/src/ui/modal_process.rs index a3f91e5..56aa034 100644 --- a/socktop/src/ui/modal_process.rs +++ b/socktop/src/ui/modal_process.rs @@ -95,7 +95,7 @@ impl ModalManager { } // Help line - let help_text = vec![Line::from(vec![ + let mut help_text = vec![Line::from(vec![ Span::styled( "X ", Style::default() @@ -129,6 +129,23 @@ impl ModalManager { Span::styled("journal", Style::default().add_modifier(Modifier::DIM)), ])]; + // Kill from here too — same key as the processes pane, and only shown + // when the agent is local, since that is the only case where it works. + if data.is_local + && let Some(line) = help_text.first_mut() + { + line.spans.push(Span::styled( + " t ", + Style::default() + .fg(PROCESS_DETAILS_ACCENT) + .add_modifier(Modifier::BOLD), + )); + line.spans.push(Span::styled( + "kill", + Style::default().add_modifier(Modifier::DIM), + )); + } + let help = Paragraph::new(Text::from(help_text)) .alignment(Alignment::Center) .style(Style::default()); diff --git a/socktop/src/ui/modal_types.rs b/socktop/src/ui/modal_types.rs index 6f7a7c0..a45a85d 100644 --- a/socktop/src/ui/modal_types.rs +++ b/socktop/src/ui/modal_types.rs @@ -19,6 +19,9 @@ pub struct ProcessModalData<'a> { pub history: ProcessHistoryData<'a>, pub max_mem_bytes: u64, pub unsupported: bool, + /// Whether the agent is on this machine. Only used to decide whether the + /// `t` kill hint is shown — the kill itself is gated in `App`. + pub is_local: bool, } /// Parameters for rendering scatter plot @@ -64,9 +67,15 @@ pub enum ModalAction { RetryConnection, ExitApp, Confirm, + /// Confirmation modal's second affirmative: the same action, escalated. + /// Used by the kill prompt for SIGKILL, where `Confirm` means SIGTERM. + ConfirmForce, Cancel, Dismiss, SwitchToParentProcess(u32), // Switch to viewing parent process details + /// `t` pressed while viewing a process's details — the app decides whether + /// the agent is local and, if so, raises the kill confirmation. + KillSelected(u32), } #[derive(Debug, Clone, PartialEq)] @@ -74,6 +83,10 @@ pub enum ModalButton { Retry, Exit, Confirm, + /// Escalated affirmative on a Confirmation modal (SIGKILL for the kill + /// prompt). Separate button rather than a separate keybinding so the + /// destructive option has to be selected deliberately. + ConfirmForce, Cancel, Ok, } diff --git a/socktop/src/ui/processes.rs b/socktop/src/ui/processes.rs index 3e78bd9..0931e88 100644 --- a/socktop/src/ui/processes.rs +++ b/socktop/src/ui/processes.rs @@ -5,13 +5,14 @@ use ratatui::style::Modifier; use ratatui::{ layout::{Constraint, Direction, Layout, Rect}, style::{Color, Style}, - text::Span, + text::{Line, Span}, widgets::{Block, Borders, Paragraph, Scrollbar, ScrollbarOrientation, ScrollbarState, Table}, }; use std::cmp::Ordering; use crate::types::Metrics; use crate::ui::cpu::{per_core_clamp, per_core_handle_scrollbar_mouse}; +use crate::ui::fit; use crate::ui::theme::{ PROCESS_SELECTION_BG, PROCESS_SELECTION_FG, PROCESS_TOOLTIP_BG, PROCESS_TOOLTIP_FG, SB_ARROW, SB_THUMB, SB_TRACK, @@ -86,6 +87,9 @@ pub struct ProcessDisplayParams<'a> { /// Peak cpu_usage from the most recent cache build; used to bold the /// busiest process. -1.0 if no cache. pub peak_cpu: f32, + /// Agent is on this machine, so the `t` kill hint applies. Without it the + /// hint would advertise a key that deliberately does nothing. + pub is_local: bool, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] @@ -449,16 +453,60 @@ pub fn draw_top_processes(f: &mut ratatui::Frame<'_>, area: Rect, params: Proces format!("PID {selected_pid}") }; - let tooltip_text = format!("{process_info} | Enter for details • X to unselect"); - let tooltip_width = tooltip_text.len() as u16 + 2; // Add padding - let tooltip_height = 3; + // Key hints, built as spans so the keys read as keys. `t` only appears + // for a local agent, since that is the only case where it does anything. + let key = Style::default() + .fg(PROCESS_TOOLTIP_FG) + .add_modifier(Modifier::BOLD); + let label = Style::default().fg(PROCESS_TOOLTIP_FG); + let mut hints: Vec = vec![ + Span::styled("⏎", key), + Span::styled(" details", label), + Span::styled(" · ", label), + ]; + if params.is_local { + hints.push(Span::styled("t", key)); + hints.push(Span::styled(" kill", label)); + hints.push(Span::styled(" · ", label)); + } + hints.push(Span::styled("x", key)); + hints.push(Span::styled(" unselect", label)); - // Position tooltip at bottom-right of the processes area - if area.width > tooltip_width + 2 && area.height > tooltip_height + 1 { + let hints_w: u16 = hints.iter().map(|s| fit::cols(&s.content)).sum(); + // One row, borders on both sides, a space of padding each side. + let tooltip_height = 3; + let max_w = area.width.saturating_sub(2); + if max_w > hints_w + 6 && area.height > tooltip_height + 1 { + // The process name is the elastic part: truncate it so the hint + // always fits. The old version sized the box from the full string + // and skipped rendering entirely when a long process name made it + // wider than the pane — so the hint silently vanished exactly when + // a long-named process was selected. + let room_for_info = max_w - hints_w - 6; + let info = fit::truncate_cols(&process_info, room_for_info); + let mut spans: Vec = vec![ + Span::styled(" ", label), + Span::styled( + info.clone(), + Style::default() + .fg(PROCESS_TOOLTIP_FG) + .add_modifier(Modifier::BOLD), + ), + Span::styled(" │ ", label), + ]; + spans.extend(hints); + spans.push(Span::styled(" ", label)); + + let width = spans + .iter() + .map(|s| fit::cols(&s.content)) + .sum::() + .saturating_add(2) + .min(area.width); let tooltip_area = Rect { - x: area.x + area.width.saturating_sub(tooltip_width + 1), + x: area.x + area.width.saturating_sub(width + 1), y: area.y + area.height.saturating_sub(tooltip_height + 1), - width: tooltip_width, + width, height: tooltip_height, }; @@ -468,11 +516,10 @@ pub fn draw_top_processes(f: &mut ratatui::Frame<'_>, area: Rect, params: Proces .fg(PROCESS_TOOLTIP_FG), ); - let tooltip_paragraph = Paragraph::new(tooltip_text) - .block(tooltip_block) - .wrap(ratatui::widgets::Wrap { trim: true }); - - f.render_widget(tooltip_paragraph, tooltip_area); + f.render_widget( + Paragraph::new(Line::from(spans)).block(tooltip_block), + tooltip_area, + ); } } @@ -905,6 +952,7 @@ mod click_tests { filtered_indices: &idxs, cached_rows: &cache, peak_cpu: peak, + is_local: false, }, ) }) @@ -984,3 +1032,110 @@ mod click_tests { } } } + +#[cfg(test)] +mod tooltip_tests { + use super::*; + use ratatui::Terminal; + use ratatui::backend::TestBackend; + use ratatui::layout::Rect; + use socktop_connector::{Metrics, ProcessInfo}; + + fn metrics(name: &str) -> Metrics { + Metrics { + sampled_at_ms: None, + cpu_total: 0.0, + cpu_per_core: vec![], + mem_total: 32_000_000_000, + mem_used: 0, + swap_total: 0, + swap_used: 0, + hostname: "t".into(), + cpu_temp_c: None, + disks: vec![], + networks: vec![], + top_processes: vec![ProcessInfo { + pid: 4242, + name: name.into(), + cpu_usage: 1.5, + mem_bytes: 1_000_000, + }], + gpus: None, + process_count: Some(1), + } + } + + /// Render the pane with a selection and return the whole buffer as text. + fn rendered(name: &str, width: u16, is_local: bool) -> String { + let m = metrics(name); + let mut cache = Vec::new(); + let peak = rebuild_row_cache(&m, &mut cache); + let idxs = [0usize]; + let mut terminal = Terminal::new(TestBackend::new(width, 12)).unwrap(); + terminal + .draw(|f| { + draw_top_processes( + f, + Rect::new(0, 0, width, 12), + ProcessDisplayParams { + metrics: Some(&m), + scroll_offset: 0, + sort_by: ProcSortBy::CpuDesc, + selected_process_pid: Some(4242), + selected_process_index: Some(0), + search_query: "", + search_active: false, + filtered_indices: &idxs, + cached_rows: &cache, + peak_cpu: peak, + is_local, + }, + ) + }) + .unwrap(); + let buf = terminal.backend().buffer(); + let mut out = String::new(); + for y in 0..12 { + for x in 0..width { + out.push_str(buf[(x, y)].symbol()); + } + out.push('\n'); + } + out + } + + #[test] + fn hint_offers_kill_for_a_local_agent() { + let out = rendered("some-process", 80, true); + assert!(out.contains("details"), "no hint rendered at all:\n{out}"); + assert!( + out.contains("kill"), + "local agent should offer kill:\n{out}" + ); + assert!(out.contains("unselect")); + } + + /// The key does nothing for a remote agent, so advertising it would be a lie. + #[test] + fn hint_omits_kill_for_a_remote_agent() { + let out = rendered("some-process", 80, false); + assert!(out.contains("details"), "no hint rendered at all:\n{out}"); + assert!( + !out.contains("kill"), + "remote agent must not offer kill:\n{out}" + ); + } + + /// Regression: the hint used to be sized from the full label including the + /// process name, and was skipped entirely when that made it wider than the + /// pane — so it vanished exactly when a long-named process was selected. + #[test] + fn hint_survives_a_very_long_process_name() { + let long = "/usr/lib/firefox-esr/firefox-esr-with-a-really-long-suffix"; + let out = rendered(long, 80, true); + assert!( + out.contains("details") && out.contains("kill"), + "hint disappeared for a long process name:\n{out}" + ); + } +} diff --git a/socktop_agent/src/metrics.rs b/socktop_agent/src/metrics.rs index ea1ad71..ddaff01 100644 --- a/socktop_agent/src/metrics.rs +++ b/socktop_agent/src/metrics.rs @@ -588,9 +588,17 @@ pub async fn collect_processes_all(state: &AppState) -> ProcessesPayload { // filter when downgrading to a minimal refresh spec. let mut sys_guard = state.sys.lock().await; let sys = &mut *sys_guard; + // `true` = remove processes that no longer exist. With `false`, this + // long-lived System kept every process it had ever seen: the list grew + // without bound (21,648 entries on a machine with 289 processes after a + // few hours of build churn), process_count was meaningless, and — the + // reason this was found — a process you killed kept its row forever, + // because the agent went on reporting it. Safe here only because this is + // `ProcessesToUpdate::All`; with `Some(pids)` it would treat every process + // outside that list as dead and drop it. sys.refresh_processes_specifics( ProcessesToUpdate::All, - false, + true, ProcessRefreshKind::nothing().with_memory().without_tasks(), ); @@ -719,8 +727,11 @@ pub async fn collect_processes_all(state: &AppState) -> ProcessesPayload { //JW too complicated. simplify to remove strange behavior - // For active systems, get accurate CPU metrics - sys.refresh_processes_specifics(ProcessesToUpdate::All, false, kind.with_cpu()); + // For active systems, get accurate CPU metrics. + // `true` = drop processes that have exited; see the Linux path above for + // what `false` cost us (an ever-growing list that kept reporting dead + // processes). Correct only because this is `ProcessesToUpdate::All`. + sys.refresh_processes_specifics(ProcessesToUpdate::All, true, kind.with_cpu()); // } else { // // For idle systems, just get basic process info