From d133776fc65272fe97866958e81a92052b4b3e40 Mon Sep 17 00:00:00 2001 From: demrom <47463752+demrom@users.noreply.github.com> Date: Fri, 16 Oct 2026 20:31:46 +0300 Subject: [PATCH] fix: stop hiding codex sessions whose -c args mention grep or exec Codex discovery skipped any process whose command line contained "grep", and marked it as a one-shot exec when it contained " exec". Long -c values such as developer instructions are part of that command line, so an interactive TUI could be dropped entirely, or shown as exec and then hidden as Done after its first task_complete. - grep: exclude only when the executable itself is a grep (new process::cmd_is_grep), in codex, codex mcp-server, and opencode discovery. - exec: take it from the rollout's session_meta originator (codex_exec vs codex-tui); argv is only a fallback for rollouts without one, and matches the whole token. --- src/collector/codex.rs | 80 +++++++++++++++++++++++++++++++++++++-- src/collector/mcp.rs | 2 +- src/collector/opencode.rs | 3 +- src/collector/process.rs | 27 +++++++++++++ 4 files changed, 106 insertions(+), 6 deletions(-) diff --git a/src/collector/codex.rs b/src/collector/codex.rs index 68d9cbe0..1758af94 100644 --- a/src/collector/codex.rs +++ b/src/collector/codex.rs @@ -553,12 +553,18 @@ impl CodexCollector { // so task_complete alone does NOT mean the session is finished when PID is alive. // However, for exec (one-shot) sessions, task_complete means truly done. let pid_alive = proc.is_some(); + // The rollout's originator names the client ("codex_exec" vs + // "codex-tui"); argv is only a guess for rollouts without one. + let is_exec = match result.originator.as_str() { + "" => process_ctx.is_exec, + originator => originator == "codex_exec", + }; // Mirrors Claude: trust the trailing-event-is-user signal alone. // Codex tool outputs flow through response_item, not user_message, // so model_generating only flips on real prompts. let status = if process_ctx.unknown_process_owner { SessionStatus::Unknown - } else if !pid_alive || (process_ctx.is_exec && result.task_complete) { + } else if !pid_alive || (is_exec && result.task_complete) { SessionStatus::Done } else { let has_active_child = process_ctx.owns_process_tree @@ -692,9 +698,12 @@ impl CodexCollector { continue; } let cmd = &info.command; - let is_exec = cmd.contains(" exec"); + // Whole tokens only: `-c` values and prompts can contain " exec" + // or "executable", and an interactive TUI marked as exec is hidden + // as Done after its first turn. + let is_exec = cmd.split_whitespace().skip(1).any(|tok| tok == "exec"); let is_codex = process::cmd_has_binary(cmd, "codex"); - if is_codex && !cmd.contains(" app-server") && !cmd.contains("grep") { + if is_codex && !cmd.contains(" app-server") && !process::cmd_is_grep(cmd) { pids.push((*pid, is_exec)); } } @@ -734,7 +743,7 @@ impl CodexCollector { let cmd = &info.command; if process::cmd_has_binary(cmd, "codex") && cmd.contains(" app-server") - && !cmd.contains("grep") + && !process::cmd_is_grep(cmd) { pids.push(*pid); } @@ -1768,6 +1777,31 @@ mod tests { assert_eq!(pids, vec![(10, false)]); } + #[test] + fn find_codex_pids_ignores_grep_and_exec_inside_args() { + let mut process_info = HashMap::new(); + // ps joins argv with spaces, so a long `-c` value reads as words. + process_info.insert( + 10, + proc_info( + 10, + 1, + "codex -c developer_instructions=Use grep first, then run executables --resume abc", + ), + ); + process_info.insert(20, proc_info(20, 1, "grep codex")); + process_info.insert(30, proc_info(30, 1, "/usr/bin/pgrep codex")); + process_info.insert( + 40, + proc_info(40, 1, "codex --profile ci exec fix the build"), + ); + + let mut pids = CodexCollector::find_codex_pids_from_shared(&process_info, &HashSet::new()); + pids.sort_unstable(); + + assert_eq!(pids, vec![(10, false), (40, true)]); + } + #[test] fn find_codex_desktop_pids_detects_app_servers() { let mut process_info = HashMap::new(); @@ -2620,6 +2654,44 @@ mod tests { assert_eq!(session.current_tasks, vec!["finished".to_string()]); } + #[test] + fn test_codex_tui_originator_overrides_exec_guess_from_argv() { + let mut file = tempfile::NamedTempFile::new().unwrap(); + write_lines( + &mut file, + &[ + r#"{"type":"session_meta","timestamp":"2026-03-28T15:00:00Z","payload":{"id":"sess-123","cwd":"/home/user/project","originator":"codex-tui","cli_version":"0.163.0","timestamp":"2026-03-28T15:00:00Z"}}"#, + r#"{"type":"event_msg","timestamp":"2026-03-28T15:01:00Z","payload":{"type":"task_started"}}"#, + r#"{"type":"event_msg","timestamp":"2026-03-28T15:02:01Z","payload":{"type":"task_complete"}}"#, + ], + ); + + let collector = CodexCollector::new(); + let mut process_info = HashMap::new(); + process_info.insert( + 42, + proc_info(42, 1, "codex -c developer_instructions=never exec blindly"), + ); + let process_ctx = CodexProcessContext { + pid: Some(42), + is_exec: true, + owns_process_tree: true, + unknown_process_owner: false, + }; + + let (session, _) = collector + .load_session_with_rate_limit( + process_ctx, + file.path(), + &process_info, + &HashMap::new(), + &HashMap::new(), + ) + .unwrap(); + + assert_eq!(session.status, SessionStatus::Waiting); + } + #[test] fn test_codex_exec_command_end_closes_task_and_records_duration() { let mut file = tempfile::NamedTempFile::new().unwrap(); diff --git a/src/collector/mcp.rs b/src/collector/mcp.rs index 42126981..bf4c77fe 100644 --- a/src/collector/mcp.rs +++ b/src/collector/mcp.rs @@ -153,7 +153,7 @@ pub fn detect(process_info: &HashMap) -> McpDetection { fn is_codex_mcp_server(cmd: &str) -> bool { process::cmd_has_binary(cmd, "codex") && cmd.contains("mcp-server") - && !cmd.contains("grep") + && !process::cmd_is_grep(cmd) && !cmd.contains("app-server") } diff --git a/src/collector/opencode.rs b/src/collector/opencode.rs index 7acb9114..4192a00a 100644 --- a/src/collector/opencode.rs +++ b/src/collector/opencode.rs @@ -245,7 +245,8 @@ impl OpenCodeCollector { process_info .iter() .filter(|(_, info)| { - process::cmd_has_binary(&info.command, "opencode") && !info.command.contains("grep") + process::cmd_has_binary(&info.command, "opencode") + && !process::cmd_is_grep(&info.command) }) .map(|(pid, _)| *pid) .collect() diff --git a/src/collector/process.rs b/src/collector/process.rs index a700f8f7..35cfe34f 100644 --- a/src/collector/process.rs +++ b/src/collector/process.rs @@ -416,6 +416,22 @@ pub fn cmd_first_token_has_binary(cmd: &str, name: &str) -> bool { .is_some_and(|tok| unix_token_has_binary(tok, name)) } +/// True when the process itself is a grep (`grep`, `pgrep`, `egrep`, ...), +/// e.g. a `ps aux | grep codex` that `cmd_has_binary` would match on its +/// second token. Only the executable counts: an agent whose arguments +/// mention grep (prompts, `-c` instructions) is not a grep. +pub fn cmd_is_grep(cmd: &str) -> bool { + #[cfg(windows)] + let first = windows_command_tokens(cmd).into_iter().next(); + #[cfg(not(windows))] + let first = cmd.split_whitespace().next().map(str::to_owned); + first.is_some_and(|tok| { + tok.rsplit(['/', '\\']) + .next() + .is_some_and(|base| base.contains("grep")) + }) +} + #[cfg(not(windows))] fn unix_token_has_binary(tok: &str, name: &str) -> bool { let mut iter = tok.rsplit('/'); @@ -577,6 +593,17 @@ mod tests { assert!(!cmd_has_binary("/some/versions/2.1.121", "claude")); } + #[test] + fn cmd_is_grep_checks_only_the_executable() { + assert!(cmd_is_grep("grep codex")); + assert!(cmd_is_grep("/usr/bin/pgrep -f codex mcp-server")); + assert!(!cmd_is_grep( + "codex -c instructions=use-grep-first mcp-server" + )); + assert!(!cmd_is_grep("opencode run 'grep the logs'")); + assert!(!cmd_is_grep("")); + } + #[cfg(windows)] #[test] fn cmd_has_binary_windows_detects_node_wrapped_codex() {