From 38b7b540e1d1e5cdcd8fcc31bc8bcf1cde44b603 Mon Sep 17 00:00:00 2001 From: Can Celik Date: Thu, 30 Jul 2026 15:14:27 +0300 Subject: fix: add opt-in child-group process detection (#2052) * fix: add opt-in child-group process detection refs #1982 * fix: preserve lifecycle probes without foreground groups refs #1982 --- src/pane.rs | 76 +++++++++++++++----- src/platform/linux.rs | 190 +++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 245 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/pane.rs b/src/pane.rs index 3bf94ec0..be1a23da 100644 --- a/src/pane.rs +++ b/src/pane.rs @@ -381,11 +381,22 @@ fn foreground_group_changed( && (foreground_pgid.is_some() || last_foreground_pgid.is_some()) } +// Only kernel-observed foreground groups drive change detection. Remembering an +// inferred group would look like a change on every tick while the kernel stays silent. +fn process_group_for_change_tracking( + observed_foreground_pgid: Option, + probed_process_group_id: Option, +) -> Option { + observed_foreground_pgid?; + probed_process_group_id.or(observed_foreground_pgid) +} + fn should_skip_process_probe_for_lifecycle_authority( full_lifecycle_authority_active: bool, input: ProcessProbeInput, ) -> bool { full_lifecycle_authority_active + && input.foreground_pgid.is_some() && !input.pending_foreground_shell_clear && input.suppressed_agent.is_none() && input.has_process_probe @@ -713,6 +724,8 @@ fn spawn_basic_detection_task( has_process_probe = true; let probe = probe_foreground_process(pid, foreground_pgid); let process_group_id = probe.process_group_id; + let tracked_process_group_id = + process_group_for_change_tracking(foreground_pgid, process_group_id); let foreground_is_pane_shell = probe.foreground_is_pane_shell; let mut new_agent = probe.agent; if let Some(suppressed_agent) = suppressed_agent { @@ -737,17 +750,15 @@ fn spawn_basic_detection_task( &mut pending_foreground_shell_clear, &mut foreground_shell_exit_reported, ); + last_foreground_pgid = tracked_process_group_id; if new_agent.is_some() { - last_foreground_pgid = process_group_id.or(foreground_pgid); acquisition_started_at = None; last_content_change_at = None; - } else if agent_presence.current_agent().is_none() { - last_foreground_pgid = process_group_id.or(foreground_pgid); - if had_process_probe && process_group_changed { - acquisition_started_at = Some(now); - } - } else { - last_foreground_pgid = process_group_id.or(foreground_pgid); + } else if agent_presence.current_agent().is_none() + && had_process_probe + && process_group_changed + { + acquisition_started_at = Some(now); } if changed { agent = agent_presence.current_agent(); @@ -2151,6 +2162,10 @@ impl PaneRuntime { let probe = probe_foreground_process(pid, foreground_pgid); let process_name = probe.process_name; let process_group_id = probe.process_group_id; + let tracked_process_group_id = process_group_for_change_tracking( + foreground_pgid, + process_group_id, + ); let foreground_is_pane_shell = probe.foreground_is_pane_shell; let mut new_agent = probe.agent; @@ -2179,20 +2194,17 @@ impl PaneRuntime { &mut pending_foreground_shell_clear, &mut foreground_shell_exit_reported, ); + last_foreground_pgid = tracked_process_group_id; if new_agent.is_some() { - last_foreground_pgid = process_group_id; acquisition_started_at = None; last_content_change_at = None; - pending_restore_probe = false; - } else if agent_presence.current_agent().is_none() { - last_foreground_pgid = process_group_id.or(foreground_pgid); - if had_process_probe && process_group_changed { - acquisition_started_at = Some(now); - } - pending_restore_probe = false; - } else { - last_foreground_pgid = process_group_id.or(foreground_pgid); + } else if agent_presence.current_agent().is_none() + && had_process_probe + && process_group_changed + { + acquisition_started_at = Some(now); } + pending_restore_probe = false; if changed { agent = agent_presence.current_agent(); if agent != previous_agent @@ -3671,6 +3683,19 @@ mod tests { })); } + #[test] + fn inferred_group_does_not_trigger_a_probe_on_every_tick() { + let tracked = process_group_for_change_tracking(None, Some(300)); + assert_eq!(tracked, None); + assert!(!should_probe_foreground_job(ProcessProbeInput { + current_agent: Some(Agent::Claude), + foreground_pgid: None, + last_foreground_pgid: tracked, + elapsed_since_process_check: std::time::Duration::from_millis(300), + ..process_probe_input() + })); + } + #[test] fn pending_shell_clear_and_restore_force_process_probes() { assert!(should_probe_foreground_job(ProcessProbeInput { @@ -3705,6 +3730,21 @@ mod tests { )); } + #[test] + fn lifecycle_authority_keeps_periodic_probes_without_an_observed_group() { + let input = ProcessProbeInput { + current_agent: Some(Agent::Pi), + foreground_pgid: None, + last_foreground_pgid: None, + elapsed_since_process_check: PROCESS_RECHECK_IDENTIFIED, + ..process_probe_input() + }; + assert!(!should_skip_process_probe_for_lifecycle_authority( + true, input + )); + assert!(should_probe_foreground_job(input)); + } + #[test] fn lifecycle_authority_preserves_process_exit_and_release_probes() { assert!(!should_skip_process_probe_for_lifecycle_authority( diff --git a/src/platform/linux.rs b/src/platform/linux.rs index 6557b41b..76568ad0 100644 --- a/src/platform/linux.rs +++ b/src/platform/linux.rs @@ -4,6 +4,7 @@ use std::{ os::fd::RawFd, path::PathBuf, process::{Command, Stdio}, + sync::OnceLock, }; use super::{ @@ -12,6 +13,14 @@ use super::{ }; const WSL_MARKER_ENV_VARS: &[&str] = &["WSL_DISTRO_NAME", "WSL_INTEROP"]; +const PROCESS_DETECTION_ENV_VAR: &str = "HERDR_PROCESS_DETECTION"; +const CHILD_GROUPS_SCAN_LIMIT: usize = 64; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ProcessDetectionMode { + Native, + ChildGroups, +} #[derive(Debug, Clone, PartialEq, Eq)] struct ProcGroupMember { @@ -45,6 +54,29 @@ fn text_indicates_wsl(text: &str) -> bool { text.contains("microsoft") || text.contains("wsl") } +fn parse_process_detection_mode(value: Option<&str>) -> Result { + match value { + None | Some("") | Some("native") => Ok(ProcessDetectionMode::Native), + Some("child-groups") => Ok(ProcessDetectionMode::ChildGroups), + Some(value) => Err(value), + } +} + +fn process_detection_mode() -> ProcessDetectionMode { + static MODE: OnceLock = OnceLock::new(); + *MODE.get_or_init(|| { + let value = std::env::var(PROCESS_DETECTION_ENV_VAR).ok(); + parse_process_detection_mode(value.as_deref()).unwrap_or_else(|value| { + tracing::warn!( + variable = PROCESS_DETECTION_ENV_VAR, + %value, + "unknown process detection mode; using native detection" + ); + ProcessDetectionMode::Native + }) + }) +} + fn raw_command_argv(command: &str, flag: &str) -> Vec { vec!["/bin/sh".into(), flag.into(), command.into()] } @@ -96,8 +128,19 @@ pub(crate) fn available_pane_shell(child_pid: u32) -> Option { } pub fn foreground_job(child_pid: u32) -> Option { - let tpgid = foreground_process_group_id(child_pid)?; - let members = foreground_process_group_members(child_pid, tpgid)?; + if let Some(tpgid) = foreground_process_group_id(child_pid) { + return foreground_job_for_group(child_pid, tpgid); + } + + if process_detection_mode() != ProcessDetectionMode::ChildGroups { + return None; + } + + foreground_job_for_group(child_pid, child_groups_foreground_process_group(child_pid)?) +} + +fn foreground_job_for_group(child_pid: u32, process_group_id: u32) -> Option { + let members = foreground_process_group_members(child_pid, process_group_id)?; let processes = members .into_iter() .map(|member| { @@ -117,11 +160,60 @@ pub fn foreground_job(child_pid: u32) -> Option { } Some(ForegroundJob { - process_group_id: tpgid, + process_group_id, processes, }) } +/// Best-effort foreground group for environments that do not expose terminal +/// foreground groups. This mode is explicit because background jobs cannot be +/// distinguished from foreground jobs without the native terminal signal. +fn child_groups_foreground_process_group(child_pid: u32) -> Option { + let shell_group_id = process_pgrp_and_comm(child_pid) + .map(|(pgrp, _)| pgrp) + .filter(|pgrp| *pgrp > 0)? as u32; + + child_groups_foreground_process_group_with( + child_pid, + shell_group_id, + process_task_ids, + process_task_children, + |pid| process_pgrp_and_comm(pid).map(|(pgrp, _)| pgrp), + ) +} + +fn child_groups_foreground_process_group_with( + child_pid: u32, + shell_group_id: u32, + mut task_ids: impl FnMut(u32) -> Vec, + mut task_children: impl FnMut(u32, u32) -> Vec, + mut process_group_id: impl FnMut(u32) -> Option, +) -> Option { + let mut newest = None; + let mut scanned = 0usize; + for tid in task_ids(child_pid) { + for child in task_children(child_pid, tid) { + if scanned >= CHILD_GROUPS_SCAN_LIMIT { + return None; + } + scanned += 1; + + let Some(pgrp) = process_group_id(child) else { + continue; + }; + if pgrp <= 0 { + continue; + } + let pgrp = pgrp as u32; + if pgrp == shell_group_id { + continue; + } + newest = Some(newest.map_or(pgrp, |current: u32| current.max(pgrp))); + } + } + newest.or(Some(shell_group_id)) +} + fn foreground_process_group_members( child_pid: u32, process_group_id: u32, @@ -665,6 +757,98 @@ mod tests { assert!(!text_indicates_wsl("")); } + #[test] + fn process_detection_mode_requires_explicit_child_groups_value() { + assert_eq!( + parse_process_detection_mode(None), + Ok(ProcessDetectionMode::Native) + ); + assert_eq!( + parse_process_detection_mode(Some("")), + Ok(ProcessDetectionMode::Native) + ); + assert_eq!( + parse_process_detection_mode(Some("native")), + Ok(ProcessDetectionMode::Native) + ); + assert_eq!( + parse_process_detection_mode(Some("child-groups")), + Ok(ProcessDetectionMode::ChildGroups) + ); + assert_eq!(parse_process_detection_mode(Some("gvisor")), Err("gvisor")); + } + + #[test] + fn child_groups_foreground_group_picks_the_newest_job() { + let tasks = HashMap::from([(100, vec![100])]); + let children = HashMap::from([((100, 100), vec![200, 300])]); + let groups = HashMap::from([(200, 200), (300, 300)]); + + let group = child_groups_foreground_process_group_with( + 100, + 100, + |pid| tasks.get(&pid).cloned().unwrap_or_default(), + |pid, tid| children.get(&(pid, tid)).cloned().unwrap_or_default(), + |pid| groups.get(&pid).copied(), + ); + + assert_eq!(group, Some(300)); + } + + #[test] + fn child_groups_foreground_group_returns_to_the_shell_group() { + let tasks = HashMap::from([(100, vec![100])]); + let children = HashMap::from([((100, 100), vec![150, 160])]); + let groups = HashMap::from([(150, 90), (160, 90)]); + + let group = child_groups_foreground_process_group_with( + 100, + 90, + |pid| tasks.get(&pid).cloned().unwrap_or_default(), + |pid, tid| children.get(&(pid, tid)).cloned().unwrap_or_default(), + |pid| groups.get(&pid).copied(), + ); + + assert_eq!(group, Some(90)); + } + + #[test] + fn child_groups_foreground_group_skips_the_shell_group() { + let tasks = HashMap::from([(100, vec![100])]); + let children = HashMap::from([((100, 100), vec![150, 160, 300])]); + let groups = HashMap::from([(150, 90), (160, 90), (300, 300)]); + + let group = child_groups_foreground_process_group_with( + 100, + 90, + |pid| tasks.get(&pid).cloned().unwrap_or_default(), + |pid, tid| children.get(&(pid, tid)).cloned().unwrap_or_default(), + |pid| groups.get(&pid).copied(), + ); + + assert_eq!(group, Some(300)); + } + + #[test] + fn child_groups_foreground_group_fails_closed_at_the_scan_limit() { + let children: Vec = (1..=(CHILD_GROUPS_SCAN_LIMIT as u32 + 10)).collect(); + let mut inspected = 0usize; + + let group = child_groups_foreground_process_group_with( + 100, + 100, + |_| vec![100], + |_, _| children.clone(), + |pid| { + inspected += 1; + Some(pid as i32) + }, + ); + + assert_eq!(inspected, CHILD_GROUPS_SCAN_LIMIT); + assert_eq!(group, None); + } + #[test] fn foreground_members_follow_the_pane_tree_and_filter_by_process_group() { let tasks = HashMap::from([ -- cgit v1.2.3