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
This commit is contained in:
parent
cfa112e741
commit
38b7b540e1
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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=<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.
|
||||
|
|
|
|||
|
|
@ -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. |
|
||||
|
|
|
|||
|
|
@ -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`. |
|
||||
|
||||
|
|
|
|||
76
src/pane.rs
76
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<u32>,
|
||||
probed_process_group_id: Option<u32>,
|
||||
) -> Option<u32> {
|
||||
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(
|
||||
|
|
|
|||
|
|
@ -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<ProcessDetectionMode, &str> {
|
||||
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<ProcessDetectionMode> = 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<std::ffi::OsString> {
|
||||
vec!["/bin/sh".into(), flag.into(), command.into()]
|
||||
}
|
||||
|
|
@ -96,8 +128,19 @@ pub(crate) fn available_pane_shell(child_pid: u32) -> Option<String> {
|
|||
}
|
||||
|
||||
pub fn foreground_job(child_pid: u32) -> Option<ForegroundJob> {
|
||||
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<ForegroundJob> {
|
||||
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<ForegroundJob> {
|
|||
}
|
||||
|
||||
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<u32> {
|
||||
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<u32>,
|
||||
mut task_children: impl FnMut(u32, u32) -> Vec<u32>,
|
||||
mut process_group_id: impl FnMut(u32) -> Option<i32>,
|
||||
) -> Option<u32> {
|
||||
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<u32> = (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([
|
||||
|
|
|
|||
Loading…
Reference in New Issue