From 020d2b690f8f8e3640048ba065f59e666dadee42 Mon Sep 17 00:00:00 2001 From: Pavel Fadeev Date: Thu, 19 Mar 2026 22:14:26 +0100 Subject: [PATCH 1/3] fix: prefer foreground process during session resurrection When a parent process has multiple children (e.g. `claude --resume` spawning MCP servers), `get_all_cmds_by_ppid` would store only the last child from `ps` output due to HashMap::insert overwriting. This caused session resurrection to capture a subprocess command instead of the user's actual foreground command. Fix by: - Adding the STAT column to ps output (`ps -ao ppid,stat,args`) - Collecting all children per parent PID - Preferring the foreground process (STAT contains '+') - Falling back to the first child when no foreground process exists The non-unix (Windows) path receives the same fix for the overwrite bug, using first-child as the heuristic since sysinfo does not expose foreground process group information. The return type and trait signature are unchanged, so all consumers (populate_session_layout_metadata, get_pane_running_command) work without modification. Relates to #2925, #4129 --- zellij-server/src/os_input_output.rs | 54 ++++++++++--- zellij-server/src/session_layout_metadata.rs | 4 +- .../src/unit/os_input_output_tests.rs | 79 +++++++++++++++++++ 3 files changed, 126 insertions(+), 11 deletions(-) diff --git a/zellij-server/src/os_input_output.rs b/zellij-server/src/os_input_output.rs index dd7225ad8b..7cd248b7ad 100644 --- a/zellij-server/src/os_input_output.rs +++ b/zellij-server/src/os_input_output.rs @@ -548,9 +548,12 @@ impl ServerOsApi for ServerOsInputOutput { #[cfg(unix)] fn get_all_cmds_by_ppid(&self, post_hook: &Option) -> HashMap> { // the key is the stringified ppid - let mut cmds = HashMap::new(); + // We collect all children per ppid and prefer the foreground process + // (STAT contains '+') to avoid the last child in ps output silently + // overwriting earlier ones via HashMap::insert. + let mut candidates: HashMap)>> = HashMap::new(); if let Some(output) = Command::new("ps") - .args(vec!["-ao", "ppid,args"]) + .args(vec!["-ao", "ppid,stat,args"]) .output() .ok() { @@ -564,7 +567,9 @@ impl ServerOsApi for ServerOsInputOutput { .collect(); let mut line_parts = line_parts.into_iter(); let ppid = line_parts.next(); - if let Some(ppid) = ppid { + let stat = line_parts.next(); + if let (Some(ppid), Some(stat)) = (ppid, stat) { + let is_foreground = stat.contains('+'); match &post_hook { Some(post_hook) => { let command: Vec = line_parts.clone().collect(); @@ -581,16 +586,21 @@ impl ServerOsApi for ServerOsInputOutput { .split_ascii_whitespace() .map(|p| p.to_owned()) .collect(); - cmds.insert(ppid.into(), line_parts); + if !line_parts.is_empty() { + candidates.entry(ppid).or_default().push((is_foreground, line_parts)); + } }, None => { - cmds.insert(ppid.into(), line_parts.collect()); + let parts: Vec = line_parts.collect(); + if !parts.is_empty() { + candidates.entry(ppid).or_default().push((is_foreground, parts)); + } }, } } } } - cmds + select_best_candidates(candidates) } #[cfg(not(unix))] @@ -598,7 +608,10 @@ impl ServerOsApi for ServerOsInputOutput { let mut system_info = System::new(); let refresh_kind = ProcessRefreshKind::nothing().with_cmd(UpdateKind::Always); system_info.refresh_processes_specifics(ProcessesToUpdate::All, true, refresh_kind); - let mut cmds = HashMap::new(); + // Use Vec to collect all children per ppid, same as the unix path, + // to avoid HashMap::insert silently overwriting earlier entries. + // STAT/foreground info is unavailable via sysinfo, so we use false. + let mut candidates: HashMap)>> = HashMap::new(); for (_pid, process) in system_info.processes() { if let Some(parent_pid) = process.parent() { let ppid_str = format!("{}", parent_pid); @@ -625,15 +638,17 @@ impl ServerOsApi for ServerOsInputOutput { .split_ascii_whitespace() .map(|p| p.to_owned()) .collect(); - cmds.insert(ppid_str, line_parts); + if !line_parts.is_empty() { + candidates.entry(ppid_str).or_default().push((false, line_parts)); + } }, None => { - cmds.insert(ppid_str, command); + candidates.entry(ppid_str).or_default().push((false, command)); }, } } } - cmds + select_best_candidates(candidates) } fn write_to_file(&mut self, buf: String, name: Option) -> Result<()> { @@ -759,6 +774,25 @@ fn run_command_hook( Ok(String::from_utf8(output.stdout)?.trim().to_string()) } +/// Given a map of ppid -> [(is_foreground, command_args)], select the best +/// candidate per ppid: prefer a foreground process, fall back to first child. +fn select_best_candidates( + candidates: HashMap)>>, +) -> HashMap> { + let mut cmds = HashMap::new(); + for (ppid, children) in candidates { + let chosen = children + .iter() + .find(|(fg, _)| *fg) + .or(children.first()) + .map(|(_, cmd)| cmd.clone()); + if let Some(cmd) = chosen { + cmds.insert(ppid, cmd); + } + } + cmds +} + #[cfg(test)] #[path = "./unit/os_input_output_tests.rs"] mod os_input_output_tests; diff --git a/zellij-server/src/session_layout_metadata.rs b/zellij-server/src/session_layout_metadata.rs index 62efea9861..d1426041e3 100644 --- a/zellij-server/src/session_layout_metadata.rs +++ b/zellij-server/src/session_layout_metadata.rs @@ -284,7 +284,9 @@ impl SessionLayoutMetadata { let args: Vec = command_line.map(|c| c.to_owned()).collect(); if Self::is_default_shell(self.default_shell.as_ref(), &command_name, &args) { - pane_layout_metadata.run = None; + // Don't clear a previously stored non-shell command; + // a transient return to the shell (e.g. between command + // restarts) shouldn't erase the pane's saved command. } else { let mut run_command = RunCommand::new(PathBuf::from(command_name)); run_command.args = args; diff --git a/zellij-server/src/unit/os_input_output_tests.rs b/zellij-server/src/unit/os_input_output_tests.rs index 994ed98421..d73153fc88 100644 --- a/zellij-server/src/unit/os_input_output_tests.rs +++ b/zellij-server/src/unit/os_input_output_tests.rs @@ -1,4 +1,5 @@ use super::*; +use super::select_best_candidates; use zellij_utils::input::command::RunCommand; fn make_server() -> ServerOsInputOutput { @@ -201,3 +202,81 @@ fn spawn_and_read_output() { output_str ); } + +// --- select_best_candidates tests --- + +fn candidates_from(entries: Vec<(&str, Vec<(bool, Vec<&str>)>)>) -> HashMap)>> { + entries + .into_iter() + .map(|(ppid, children)| { + let children = children + .into_iter() + .map(|(fg, args)| (fg, args.into_iter().map(String::from).collect())) + .collect(); + (ppid.to_string(), children) + }) + .collect() +} + +#[test] +fn foreground_process_preferred_over_background_children() { + let candidates = candidates_from(vec![ + ("1234", vec![ + (true, vec!["claude", "--resume", "my-session"]), + (false, vec!["node", "/path/to/mcp-server"]), + ]), + ]); + let cmds = select_best_candidates(candidates); + assert_eq!(cmds.get("1234").unwrap(), &["claude", "--resume", "my-session"]); +} + +#[test] +fn background_process_listed_first_does_not_win() { + let candidates = candidates_from(vec![ + ("1234", vec![ + (false, vec!["node", "/path/to/mcp-server-1"]), + (false, vec!["node", "/path/to/mcp-server-2"]), + (true, vec!["claude", "--resume", "my-session"]), + ]), + ]); + let cmds = select_best_candidates(candidates); + assert_eq!(cmds.get("1234").unwrap(), &["claude", "--resume", "my-session"]); +} + +#[test] +fn single_child_returned_regardless_of_foreground() { + let candidates = candidates_from(vec![ + ("5678", vec![(false, vec!["nvim", "main.rs"])]), + ]); + let cmds = select_best_candidates(candidates); + assert_eq!(cmds.get("5678").unwrap(), &["nvim", "main.rs"]); +} + +#[test] +fn multiple_ppids_handled_independently() { + let candidates = candidates_from(vec![ + ("100", vec![ + (false, vec!["node", "mcp-server"]), + (true, vec!["claude", "--resume", "foo"]), + ]), + ("200", vec![ + (true, vec!["nvim", "bar.rs"]), + (false, vec!["node", "lsp-server"]), + ]), + ]); + let cmds = select_best_candidates(candidates); + assert_eq!(cmds.get("100").unwrap(), &["claude", "--resume", "foo"]); + assert_eq!(cmds.get("200").unwrap(), &["nvim", "bar.rs"]); +} + +#[test] +fn no_foreground_falls_back_to_first_child() { + let candidates = candidates_from(vec![ + ("300", vec![ + (false, vec!["node", "server-a"]), + (false, vec!["node", "server-b"]), + ]), + ]); + let cmds = select_best_candidates(candidates); + assert_eq!(cmds.get("300").unwrap(), &["node", "server-a"]); +} From 5b34c90e64dacc9b18d2c822038dae68c3a4de2b Mon Sep 17 00:00:00 2001 From: Pavel Fadeev Date: Thu, 19 Mar 2026 23:57:56 +0100 Subject: [PATCH 2/3] refactor: clean up review feedback - Negate empty if-branch in update_terminal_commands (match existing is_dirty pattern at line 124) - Avoid unnecessary .clone() in select_best_candidates by using position() + into_iter().nth() to move instead of copy --- zellij-server/src/os_input_output.rs | 12 +++++------- zellij-server/src/session_layout_metadata.rs | 9 ++++----- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/zellij-server/src/os_input_output.rs b/zellij-server/src/os_input_output.rs index 7cd248b7ad..5e11ab524e 100644 --- a/zellij-server/src/os_input_output.rs +++ b/zellij-server/src/os_input_output.rs @@ -781,14 +781,12 @@ fn select_best_candidates( ) -> HashMap> { let mut cmds = HashMap::new(); for (ppid, children) in candidates { - let chosen = children - .iter() - .find(|(fg, _)| *fg) - .or(children.first()) - .map(|(_, cmd)| cmd.clone()); - if let Some(cmd) = chosen { - cmds.insert(ppid, cmd); + if children.is_empty() { + continue; } + let idx = children.iter().position(|(fg, _)| *fg).unwrap_or(0); + let (_, cmd) = children.into_iter().nth(idx).unwrap(); + cmds.insert(ppid, cmd); } cmds } diff --git a/zellij-server/src/session_layout_metadata.rs b/zellij-server/src/session_layout_metadata.rs index d1426041e3..0d7fa18473 100644 --- a/zellij-server/src/session_layout_metadata.rs +++ b/zellij-server/src/session_layout_metadata.rs @@ -282,16 +282,15 @@ impl SessionLayoutMetadata { let mut command_line = command.iter(); if let Some(command_name) = command_line.next() { let args: Vec = command_line.map(|c| c.to_owned()).collect(); - if Self::is_default_shell(self.default_shell.as_ref(), &command_name, &args) + if !Self::is_default_shell(self.default_shell.as_ref(), &command_name, &args) { - // Don't clear a previously stored non-shell command; - // a transient return to the shell (e.g. between command - // restarts) shouldn't erase the pane's saved command. - } else { let mut run_command = RunCommand::new(PathBuf::from(command_name)); run_command.args = args; pane_layout_metadata.run = Some(Run::Command(run_command)); } + // else: don't clear a previously stored non-shell command; + // a transient return to the shell shouldn't erase the + // pane's saved command during periodic serialization. } } } From 34fcb62b126ab387445506bafafd8bbe081729cb Mon Sep 17 00:00:00 2001 From: Pavel Fadeev Date: Fri, 20 Mar 2026 00:44:31 +0100 Subject: [PATCH 3/3] revert: restore original shell-detection clearing behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sticky command change (not clearing `run` when the default shell is detected) caused a regression. The os_input_output fix alone addresses the root cause — this clearing behavior is needed for panes that legitimately return to the shell. --- zellij-server/src/session_layout_metadata.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/zellij-server/src/session_layout_metadata.rs b/zellij-server/src/session_layout_metadata.rs index 0d7fa18473..62efea9861 100644 --- a/zellij-server/src/session_layout_metadata.rs +++ b/zellij-server/src/session_layout_metadata.rs @@ -282,15 +282,14 @@ impl SessionLayoutMetadata { let mut command_line = command.iter(); if let Some(command_name) = command_line.next() { let args: Vec = command_line.map(|c| c.to_owned()).collect(); - if !Self::is_default_shell(self.default_shell.as_ref(), &command_name, &args) + if Self::is_default_shell(self.default_shell.as_ref(), &command_name, &args) { + pane_layout_metadata.run = None; + } else { let mut run_command = RunCommand::new(PathBuf::from(command_name)); run_command.args = args; pane_layout_metadata.run = Some(Run::Command(run_command)); } - // else: don't clear a previously stored non-shell command; - // a transient return to the shell shouldn't erase the - // pane's saved command during periodic serialization. } } }