From 38b7b540e1d1e5cdcd8fcc31bc8bcf1cde44b603 Mon Sep 17 00:00:00 2001 From: Can Celik Date: Thu, 30 Jul 2026 15:14:27 +0300 Subject: [PATCH] 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 --- docs/next/CHANGELOG.md | 1 + docs/next/website/src/content/docs/agents.mdx | 2 + .../src/content/docs/cli-reference.mdx | 1 + .../src/content/docs/configuration.mdx | 1 + src/pane.rs | 76 +++++-- src/platform/linux.rs | 190 +++++++++++++++++- 6 files changed, 250 insertions(+), 21 deletions(-) diff --git a/docs/next/CHANGELOG.md b/docs/next/CHANGELOG.md index 1fc8b6f0..d3f9f8e9 100644 --- a/docs/next/CHANGELOG.md +++ b/docs/next/CHANGELOG.md @@ -8,6 +8,7 @@ ### Fixed - Vibe and other Kitty-keyboard pane applications now receive shifted letters and punctuation when they request associated text. (#2020) +- Linux runtimes without terminal foreground process groups can opt into child-group agent detection with `HERDR_PROCESS_DETECTION=child-groups`. (#1982) - Installing the Herdr agent skill with the `skills` CLI no longer copies the entire repository. (#2022) - Agent prompts now wait briefly after sending text before pressing Enter, preventing prompts from remaining in agent composers without starting a turn. (#1878) - Empty clipboard writes from pane applications no longer erase existing clipboard contents or show a copied confirmation. (#1893) diff --git a/docs/next/website/src/content/docs/agents.mdx b/docs/next/website/src/content/docs/agents.mdx index f5a4ac48..e7764d84 100644 --- a/docs/next/website/src/content/docs/agents.mdx +++ b/docs/next/website/src/content/docs/agents.mdx @@ -51,6 +51,8 @@ Claude Code, Codex, GitHub Copilot CLI, Droid, Qoder CLI, Cursor Agent CLI, and On Linux and macOS, a host-visible wrapper can hide the real agent process from Herdr. Set `HERDR_AGENT=` on the wrapper command to tell Herdr which existing agent screen manifest to use. For example, run `HERDR_AGENT=claude fence -- claude` on Linux or `HERDR_AGENT=claude nono run --profile claude-code -- claude` on macOS. The hint is scoped to that foreground process; setting it only inside a VM or container is not visible to Herdr, and you should avoid exporting it globally unless every inherited foreground process should be treated as that agent. +Some restricted Linux runtimes do not expose a terminal foreground process group. Start the Herdr server with `HERDR_PROCESS_DETECTION=child-groups` to opt into direct child-process-group inference when native detection is unavailable. Native detection remains preferred, and the default `native` mode never performs this inference. The opt-in mode is best effort: a newer background job can be mistaken for the foreground job. The variable is read by the server and requires a restart; set it in the remote server environment rather than on an attaching client. + ## Blocked state Blocked detection is deliberately strict for screen-manifest agents. Herdr only marks `blocked` when the live bottom-buffer snapshot matches known visible approval, question, or permission UI. If no manifest rule matches for a known agent, Herdr falls back to `idle` and labels that fallback as `default_known_agent_idle_fallback` in explain output. diff --git a/docs/next/website/src/content/docs/cli-reference.mdx b/docs/next/website/src/content/docs/cli-reference.mdx index a49a4212..2aa008f5 100644 --- a/docs/next/website/src/content/docs/cli-reference.mdx +++ b/docs/next/website/src/content/docs/cli-reference.mdx @@ -479,6 +479,7 @@ These meanings apply to reads. For `pane wait-output` only, both `recent` and `r | `HERDR_CONFIG_PATH` | Override the config file path. | | `HERDR_SESSION` | Select a named session for CLI commands. | | `HERDR_SOCKET_PATH` | Low-level socket path override. | +| `HERDR_PROCESS_DETECTION` | Linux process detection strategy: `native` (default) or opt-in `child-groups`. | | `HERDR_ENV` | Set to `1` inside Herdr-managed pane processes. | | `HERDR_PANE_ID` | Public pane id for the running pane process. | | `HERDR_TAB_ID` | Public tab id for the running pane process. | diff --git a/docs/next/website/src/content/docs/configuration.mdx b/docs/next/website/src/content/docs/configuration.mdx index df6dc8d3..bd908765 100644 --- a/docs/next/website/src/content/docs/configuration.mdx +++ b/docs/next/website/src/content/docs/configuration.mdx @@ -452,6 +452,7 @@ On Windows, support is currently limited to the Korean IME. With an IME for any | `HERDR_CONFIG_PATH` | Override the config file path. | | `HERDR_SESSION` | Select a named session for CLI commands. | | `HERDR_SOCKET_PATH` | Low-level socket path override. | +| `HERDR_PROCESS_DETECTION` | Linux process detection strategy: `native` (default) or opt-in `child-groups`. | | `HERDR_LOG` | Set log filtering, for example `HERDR_LOG=herdr=debug`. | | `HERDR_DISABLE_SOUND` | Disable sound playback even when `[ui.sound] enabled = true`. | 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([