From 60624a1f7f660266338bce93638e8f0bb8668dc2 Mon Sep 17 00:00:00 2001 From: Sean Date: Tue, 20 Oct 2026 13:42:40 +0800 Subject: [PATCH 1/5] fix(tmux): close panes on their owning server --- src/instance_binding.rs | 8 +++++ src/pidtrack.rs | 1 + src/terminal.rs | 71 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+) diff --git a/src/instance_binding.rs b/src/instance_binding.rs index 617d031b..63e7e9a9 100644 --- a/src/instance_binding.rs +++ b/src/instance_binding.rs @@ -201,6 +201,7 @@ fn capture_context() -> serde_json::Map { "WT_SESSION", "ConEmuHWND", "TMUX_PANE", + "TMUX", "STY", "ZELLIJ_SESSION_NAME", "ZELLIJ_PANE_ID", @@ -1446,6 +1447,7 @@ mod tests { .unwrap(); let _preset = EnvVarGuard::set("HCOM_LAUNCHED_PRESET", ""); let _process_id = EnvVarGuard::set("HCOM_PROCESS_ID", ""); + let _tmux = EnvVarGuard::set("TMUX", "/tmp/tmux-owner.sock,123,0"); capture_and_store_launch_context(&db, "luna"); @@ -1467,6 +1469,12 @@ mod tests { ctx.get("pid_identity").and_then(|v| v.as_str()), Some("spawn-1") ); + assert_eq!( + ctx.get("env") + .and_then(|env| env.get("TMUX")) + .and_then(|v| v.as_str()), + Some("/tmp/tmux-owner.sock,123,0") + ); cleanup(path); } diff --git a/src/pidtrack.rs b/src/pidtrack.rs index 15896a24..677d687e 100644 --- a/src/pidtrack.rs +++ b/src/pidtrack.rs @@ -80,6 +80,7 @@ impl OrphanProcess { zellij_session_name: self.zellij_session_name.clone(), herdr_socket_path: self.herdr_socket_path.clone(), wezterm_unix_socket: self.wezterm_unix_socket.clone(), + ..crate::terminal::TerminalInfo::default() } } } diff --git a/src/terminal.rs b/src/terminal.rs index debce16f..95d98514 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -98,6 +98,21 @@ fn is_wezterm_argv(argv: &[String]) -> bool { .is_some_and(|stem| stem.eq_ignore_ascii_case("wezterm")) } +/// Add the pane owner's tmux socket when the close template does not select one. +fn splice_tmux_socket(argv: &mut Vec, socket_path: &str) { + let is_tmux = argv + .first() + .and_then(|bin| std::path::Path::new(bin).file_stem()) + .and_then(|stem| stem.to_str()) + .is_some_and(|stem| stem.eq_ignore_ascii_case("tmux")); + let has_socket = argv + .iter() + .any(|arg| arg == "-S" || arg.starts_with("-S") || arg == "-L" || arg.starts_with("-L")); + if is_tmux && !socket_path.is_empty() && !has_socket { + argv.splice(1..1, ["-S".to_string(), socket_path.to_string()]); + } +} + /// Terminal info resolved for an instance. #[derive(Debug, Clone, Default)] pub struct TerminalInfo { @@ -111,6 +126,8 @@ pub struct TerminalInfo { pub herdr_socket_path: String, /// The pane's `WEZTERM_UNIX_SOCKET`: which wezterm GUI process owns it. pub wezterm_unix_socket: String, + /// The pane's tmux server socket path; pane IDs are unique only per server. + pub tmux_socket_path: String, } /// Result from launch_terminal. @@ -2790,6 +2807,7 @@ pub fn close_terminal_pane(pid: u32, info: &TerminalInfo) -> PaneCloseResult { zellij_session_name, herdr_socket_path, wezterm_unix_socket, + tmux_socket_path, } = info; let failed_without_command = || PaneCloseResult { closed: false, @@ -2825,6 +2843,8 @@ pub fn close_terminal_pane(pid: u32, info: &TerminalInfo) -> PaneCloseResult { None => return failed_without_command(), }; + splice_tmux_socket(&mut argv, tmux_socket_path); + let is_zellij = is_zellij_merged(&merged); let zellij_before_close = if is_zellij { @@ -3093,6 +3113,18 @@ pub fn resolve_terminal_info( .and_then(|e| e.get("WEZTERM_UNIX_SOCKET").and_then(|v| v.as_str())) .unwrap_or("") .to_string(); + info.tmux_socket_path = lc + .get("tmux_socket_path") + .and_then(|v| v.as_str()) + .filter(|v| !v.is_empty()) + .or_else(|| { + lc_env + .and_then(|e| e.get("TMUX").and_then(|v| v.as_str())) + .and_then(|v| v.split(',').next()) + .filter(|v| !v.is_empty()) + }) + .unwrap_or("") + .to_string(); } // Legacy kitty launches may have pane/socket metadata but no persisted preset. @@ -3286,6 +3318,45 @@ mod tests { assert_eq!(info.terminal_id, "term-1"); } + #[test] + fn test_resolve_terminal_info_reads_tmux_socket_path() { + let info = resolve_terminal_info( + Some("tmux-split"), + Some(r#"{"pane_id":"%0","tmux_socket_path":"/tmp/tmux-owner.sock"}"#), + ); + assert_eq!(info.tmux_socket_path, "/tmp/tmux-owner.sock"); + + let info = resolve_terminal_info( + Some("tmux-split"), + Some(r#"{"pane_id":"%0","env":{"TMUX":"/tmp/tmux-owner.sock,123,0"}}"#), + ); + assert_eq!(info.tmux_socket_path, "/tmp/tmux-owner.sock"); + } + + #[test] + fn test_splice_tmux_socket_targets_the_pane_owner_server() { + let mut command = argv(&["tmux", "kill-pane", "-t", "%0"]); + splice_tmux_socket(&mut command, "/tmp/tmux owner.sock"); + assert_eq!( + command, + argv(&[ + "tmux", + "-S", + "/tmp/tmux owner.sock", + "kill-pane", + "-t", + "%0" + ]) + ); + + let mut explicit = argv(&["tmux", "-L", "custom", "kill-pane", "-t", "%0"]); + splice_tmux_socket(&mut explicit, "/tmp/tmux owner.sock"); + assert_eq!( + explicit, + argv(&["tmux", "-L", "custom", "kill-pane", "-t", "%0"]) + ); + } + #[test] fn test_launcher_env_preserves_zellij_session_but_strips_pane() { let env = get_launcher_env_from(vec![ From 08753e77f22526f20123bace84b662a23fa18ff7 Mon Sep 17 00:00:00 2001 From: Sean Date: Tue, 20 Oct 2026 14:15:31 +0800 Subject: [PATCH 2/5] fix(tmux): preserve owner socket through cleanup --- src/instance_binding.rs | 88 ++++++++++++++++++++---- src/pidtrack.rs | 149 ++++++++++++++++++++++++++++++++++++++-- src/terminal.rs | 75 ++++++++++++++++---- 3 files changed, 279 insertions(+), 33 deletions(-) diff --git a/src/instance_binding.rs b/src/instance_binding.rs index 63e7e9a9..0f606603 100644 --- a/src/instance_binding.rs +++ b/src/instance_binding.rs @@ -112,18 +112,73 @@ pub fn capture_and_store_launch_context(db: &HcomDb, instance_name: &str) { // not be overwritten with a stale pid_identity (or lose its identity). let result = db.with_write_scope(|| { let mut ctx = new_ctx; - if !missing.is_empty() - && let Some(pos) = db.get_instance_full(instance_name)? + if let Some(pos) = db.get_instance_full(instance_name)? && let Some(old_json) = &pos.launch_context && let Ok(old_ctx) = serde_json::from_str::(old_json) { - for k in &missing { - if let Some(val) = old_ctx.get(*k) - && !launch_context_value_missing(Some(val)) - { - ctx.insert(k.to_string(), val.clone()); + if !missing.is_empty() { + for k in &missing { + if let Some(val) = old_ctx.get(*k) + && !launch_context_value_missing(Some(val)) + { + ctx.insert(k.to_string(), val.clone()); + } + } + } + + let old_process_id = old_ctx.get("process_id").and_then(|v| v.as_str()); + let new_process_id = ctx.get("process_id").and_then(|v| v.as_str()); + let old_socket = old_ctx + .get("tmux_socket_path") + .and_then(|v| v.as_str()) + .filter(|v| !v.is_empty()) + .map(str::to_string) + .or_else(|| { + let socket = + crate::terminal::resolve_terminal_info_from_launch_context(old_json) + .tmux_socket_path; + (!socket.is_empty()).then_some(socket) + }); + + // Keep the socket captured for this process when later hooks run + // outside its tmux pane or inherit the caller's TMUX value. + if old_process_id.is_some() && old_process_id == new_process_id { + if let Some(old_socket) = old_socket { + ctx.insert("tmux_socket_path".into(), serde_json::json!(old_socket)); + if let Some(old_tmux) = old_ctx + .get("env") + .and_then(|env| env.get("TMUX")) + .filter(|value| !launch_context_value_missing(Some(value))) + { + let env = ctx.entry("env").or_insert_with(|| serde_json::json!({})); + if !env.is_object() { + *env = serde_json::json!({}); + } + env["TMUX"] = old_tmux.clone(); + } + } else if let Some(env) = ctx.get_mut("env").and_then(|v| v.as_object_mut()) { + // This process has no previously verified socket, so a + // later hook's TMUX may belong to its caller. Refuse to + // infer a pane owner from that value. + env.remove("TMUX"); + } + } else { + let current_json = serde_json::Value::Object(ctx.clone()).to_string(); + let current_socket = + crate::terminal::resolve_terminal_info_from_launch_context(¤t_json) + .tmux_socket_path; + if !current_socket.is_empty() { + ctx.insert("tmux_socket_path".into(), serde_json::json!(current_socket)); } } + } else { + let current_json = serde_json::Value::Object(ctx.clone()).to_string(); + let current_socket = + crate::terminal::resolve_terminal_info_from_launch_context(¤t_json) + .tmux_socket_path; + if !current_socket.is_empty() { + ctx.insert("tmux_socket_path".into(), serde_json::json!(current_socket)); + } } let json = serde_json::to_string(&ctx).unwrap_or_else(|_| "{}".to_string()); @@ -1432,7 +1487,8 @@ mod tests { fn test_capture_and_store_launch_context_preserves_terminal_metadata() { // Preserve only the fields we can't recapture from hook env: // pane_id, terminal_id, kitty_listen_on, process_id, and the resolved - // terminal preset name. + // terminal preset name. The owning tmux socket must also survive a + // later hook that runs with the caller's TMUX environment. let (db, path) = setup_test_db(); db.conn() .execute( @@ -1441,26 +1497,26 @@ mod tests { "luna", "claude", 1.0f64, - r#"{"terminal_preset_effective":"herdr","pane_id":"p_7","process_id":"proc-1","pid_identity":"spawn-1"}"# + r#"{"terminal_preset_effective":"tmux-split","pane_id":"%1","process_id":"proc-1","pid_identity":"spawn-1","env":{"TMUX":"/tmp/tmux-owner.sock,123,0"}}"# ], ) .unwrap(); let _preset = EnvVarGuard::set("HCOM_LAUNCHED_PRESET", ""); - let _process_id = EnvVarGuard::set("HCOM_PROCESS_ID", ""); - let _tmux = EnvVarGuard::set("TMUX", "/tmp/tmux-owner.sock,123,0"); + let _process_id = EnvVarGuard::set("HCOM_PROCESS_ID", "proc-1"); + let _tmux = EnvVarGuard::set("TMUX", "/tmp/tmux-parent.sock,456,0"); capture_and_store_launch_context(&db, "luna"); let row = db.get_instance_full("luna").unwrap().unwrap(); - assert_eq!(row.terminal_preset_effective.as_deref(), Some("herdr")); + assert_eq!(row.terminal_preset_effective.as_deref(), Some("tmux-split")); let ctx: serde_json::Value = serde_json::from_str(row.launch_context.as_deref().unwrap_or("{}")).unwrap(); assert_eq!( ctx.get("terminal_preset_effective") .and_then(|v| v.as_str()), - Some("herdr") + Some("tmux-split") ); - assert_eq!(ctx.get("pane_id").and_then(|v| v.as_str()), Some("p_7")); + assert_eq!(ctx.get("pane_id").and_then(|v| v.as_str()), Some("%1")); assert_eq!( ctx.get("process_id").and_then(|v| v.as_str()), Some("proc-1") @@ -1475,6 +1531,10 @@ mod tests { .and_then(|v| v.as_str()), Some("/tmp/tmux-owner.sock,123,0") ); + assert_eq!( + ctx.get("tmux_socket_path").and_then(|v| v.as_str()), + Some("/tmp/tmux-owner.sock") + ); cleanup(path); } diff --git a/src/pidtrack.rs b/src/pidtrack.rs index 677d687e..bfd9a81d 100644 --- a/src/pidtrack.rs +++ b/src/pidtrack.rs @@ -34,6 +34,8 @@ pub struct PidEntry { #[serde(default, skip_serializing_if = "String::is_empty")] pub wezterm_unix_socket: String, #[serde(default, skip_serializing_if = "String::is_empty")] + pub tmux_socket_path: String, + #[serde(default, skip_serializing_if = "String::is_empty")] pub session_id: String, #[serde(default, skip_serializing_if = "is_zero")] pub notify_port: u16, @@ -80,6 +82,22 @@ impl OrphanProcess { zellij_session_name: self.zellij_session_name.clone(), herdr_socket_path: self.herdr_socket_path.clone(), wezterm_unix_socket: self.wezterm_unix_socket.clone(), + tmux_socket_path: read_raw(&crate::paths::hcom_dir()) + .get(&self.pid.to_string()) + .map(|entry| entry.tmux_socket_path.clone()) + .filter(|socket| !socket.is_empty()) + .or_else(|| { + self.names.iter().find_map(|name| { + let socket = tmux_socket_path_for_instance( + &crate::paths::hcom_dir(), + name, + self.pid, + &self.process_id, + ); + (!socket.is_empty()).then_some(socket) + }) + }) + .unwrap_or_default(), ..crate::terminal::TerminalInfo::default() } } @@ -133,6 +151,43 @@ fn write_raw(hcom_dir: &Path, data: &HashMap) { } } +/// Read the owning pane's socket from the matching instance row. The PID or +/// process identity must match so a reused instance name cannot supply a +/// different process's terminal metadata. +fn tmux_socket_path_for_instance( + hcom_dir: &Path, + name: &str, + pid: u32, + process_id: &str, +) -> String { + let db_path = hcom_dir.join("hcom.db"); + if !db_path.is_file() { + return String::new(); + } + let Ok(db) = crate::db::HcomDb::open_at(&db_path) else { + return String::new(); + }; + let Ok(Some(row)) = db.get_instance_full(name) else { + return String::new(); + }; + let Some(context) = row.launch_context.as_deref() else { + return String::new(); + }; + let context_value: serde_json::Value = + serde_json::from_str(context).unwrap_or(serde_json::Value::Null); + let context_process_id = context_value + .get("process_id") + .and_then(serde_json::Value::as_str) + .unwrap_or(""); + let identity_matches = !process_id.is_empty() + && context_process_id == process_id + && row.pid == Some(i64::from(pid)); + if !identity_matches { + return String::new(); + } + crate::terminal::resolve_terminal_info_from_launch_context(context).tmux_socket_path +} + /// Parameters for recording a launched process. #[derive(Debug)] pub struct PidRecord<'a> { @@ -207,6 +262,11 @@ pub fn record_pid(rec: &PidRecord<'_>) { inject_port, tag, } = rec; + let tmux_socket_path = if terminal_preset.starts_with("tmux") { + tmux_socket_path_for_instance(hcom_dir, name, *pid, process_id) + } else { + String::new() + }; let mut data = read_raw(hcom_dir); let key = pid.to_string(); @@ -240,6 +300,9 @@ pub fn record_pid(rec: &PidRecord<'_>) { if !wezterm_unix_socket.is_empty() && entry.wezterm_unix_socket.is_empty() { entry.wezterm_unix_socket = wezterm_unix_socket.to_string(); } + if !tmux_socket_path.is_empty() && entry.tmux_socket_path.is_empty() { + entry.tmux_socket_path = tmux_socket_path.clone(); + } if !session_id.is_empty() && entry.session_id.is_empty() { entry.session_id = session_id.to_string(); } @@ -268,6 +331,7 @@ pub fn record_pid(rec: &PidRecord<'_>) { zellij_session_name: zellij_session_name.to_string(), herdr_socket_path: herdr_socket_path.to_string(), wezterm_unix_socket: wezterm_unix_socket.to_string(), + tmux_socket_path, session_id: session_id.to_string(), notify_port: *notify_port, inject_port: *inject_port, @@ -300,8 +364,25 @@ pub fn get_orphan_processes( } } - // Write back pruned data if anything was removed - if alive.len() != data.len() { + // Recover a socket recorded in the matching instance row when the early + // PID record was written before the terminal hook captured its context. + let mut changed = alive.len() != data.len(); + for (pid_str, entry) in &mut alive { + if entry.tmux_socket_path.is_empty() + && entry.terminal_preset.starts_with("tmux") + && let Ok(pid) = pid_str.parse::() + { + for name in &entry.names { + let socket = tmux_socket_path_for_instance(hcom_dir, name, pid, &entry.process_id); + if !socket.is_empty() { + entry.tmux_socket_path = socket; + changed = true; + break; + } + } + } + } + if changed { write_raw(hcom_dir, &alive); } @@ -625,6 +706,20 @@ mod tests { #[test] fn test_record_and_read() { let dir = make_temp_dir(); + let db = crate::db::HcomDb::open_at(&dir.path().join("hcom.db")).unwrap(); + db.conn() + .execute( + "INSERT INTO instances (name, tool, created_at, pid, launch_context) VALUES (?1, ?2, ?3, ?4, ?5)", + rusqlite::params![ + "luna", + "claude", + 1.0f64, + 12345i64, + r#"{"process_id":"pid-1","tmux_socket_path":"/tmp/tmux-owner.sock"}"# + ], + ) + .unwrap(); + drop(db); record_pid(&PidRecord { hcom_dir: dir.path(), pid: 12345, @@ -632,7 +727,7 @@ mod tests { name: "luna", directory: "/tmp", process_id: "pid-1", - terminal_preset: "kitty", + terminal_preset: "tmux-split", pane_id: "pane-1", terminal_id: "term-1", kitty_listen_on: "/tmp/kitty.sock", @@ -651,7 +746,7 @@ mod tests { assert_eq!(entry.tool, "claude"); assert_eq!(entry.names, vec!["luna"]); assert_eq!(entry.process_id, "pid-1"); - assert_eq!(entry.terminal_preset, "kitty"); + assert_eq!(entry.terminal_preset, "tmux-split"); assert_eq!(entry.pane_id, "pane-1"); assert_eq!(entry.terminal_id, "term-1"); assert_eq!(entry.kitty_listen_on, "/tmp/kitty.sock"); @@ -663,16 +758,58 @@ mod tests { assert_eq!(entry.session_id, "sess-1"); assert_eq!(entry.notify_port, 8080); assert_eq!(entry.inject_port, 8081); + assert_eq!(entry.tmux_socket_path, "/tmp/tmux-owner.sock"); - // An orphan closes its pane on the herdr server that owns it. + // Orphan conversion preserves terminal metadata for pane cleanup. let orphan = OrphanProcess::from((12345, entry)); let info = orphan.terminal_info(); - assert_eq!(info.preset_name, "kitty"); + assert_eq!(info.preset_name, "tmux-split"); assert_eq!(info.pane_id, "pane-1"); assert_eq!( info.herdr_socket_path, "/run/herdr/sessions/work/herdr.sock" ); + assert_eq!( + tmux_socket_path_for_instance(dir.path(), "luna", 12345, "pid-1"), + "/tmp/tmux-owner.sock" + ); + assert!(tmux_socket_path_for_instance(dir.path(), "luna", 12345, "pid-other").is_empty()); + } + + #[test] + #[serial_test::serial] + fn test_orphan_terminal_info_uses_persisted_tmux_socket() { + let (_dir, hcom_dir, _home, _guard) = crate::hooks::test_helpers::isolated_test_env(); + let db = crate::db::HcomDb::open().unwrap(); + let pid = std::process::id(); + let process_id = "tmux-owner-process"; + db.conn() + .execute( + "INSERT INTO instances (name, tool, created_at, pid, launch_context) VALUES (?1, ?2, ?3, ?4, ?5)", + rusqlite::params![ + "luna", + "claude", + 1.0f64, + i64::from(pid), + r#"{"process_id":"tmux-owner-process","tmux_socket_path":"/tmp/tmux-owner.sock"}"# + ], + ) + .unwrap(); + drop(db); + + record_pid(&PidRecord { + process_id, + terminal_preset: "tmux-split", + pane_id: "%1", + ..rec(&hcom_dir, pid, "claude", "luna") + }); + + let orphans = get_orphan_processes(&hcom_dir, None); + let orphan = orphans.iter().find(|orphan| orphan.pid == pid).unwrap(); + assert_eq!( + orphan.terminal_info().tmux_socket_path, + "/tmp/tmux-owner.sock" + ); } #[test] diff --git a/src/terminal.rs b/src/terminal.rs index 95d98514..0b17e6f1 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -98,21 +98,46 @@ fn is_wezterm_argv(argv: &[String]) -> bool { .is_some_and(|stem| stem.eq_ignore_ascii_case("wezterm")) } -/// Add the pane owner's tmux socket when the close template does not select one. -fn splice_tmux_socket(argv: &mut Vec, socket_path: &str) { - let is_tmux = argv - .first() +fn is_tmux_command(argv: &[String]) -> bool { + argv.first() .and_then(|bin| std::path::Path::new(bin).file_stem()) .and_then(|stem| stem.to_str()) - .is_some_and(|stem| stem.eq_ignore_ascii_case("tmux")); - let has_socket = argv - .iter() - .any(|arg| arg == "-S" || arg.starts_with("-S") || arg == "-L" || arg.starts_with("-L")); - if is_tmux && !socket_path.is_empty() && !has_socket { + .is_some_and(|stem| stem.eq_ignore_ascii_case("tmux")) +} + +fn has_tmux_server_selector(argv: &[String]) -> bool { + argv.iter() + .any(|arg| arg == "-S" || arg.starts_with("-S") || arg == "-L" || arg.starts_with("-L")) +} + +fn tmux_close_has_server_identity(argv: &[String], socket_path: &str) -> bool { + !is_tmux_command(argv) || !socket_path.is_empty() || has_tmux_server_selector(argv) +} + +/// Add the pane owner's tmux socket when the close template does not select one. +fn splice_tmux_socket(argv: &mut Vec, socket_path: &str) { + let has_socket = has_tmux_server_selector(argv); + if is_tmux_command(argv) && !socket_path.is_empty() && !has_socket { argv.splice(1..1, ["-S".to_string(), socket_path.to_string()]); } } +/// Parse the socket path from tmux's `TMUX=socket,pid,pane` environment value. +/// Splitting from the right preserves commas that are part of the socket path. +pub(crate) fn tmux_socket_path_from_env(value: &str) -> Option<&str> { + let mut parts = value.rsplitn(3, ','); + let pane_index = parts.next()?; + let server_pid = parts.next()?; + let socket_path = parts.next()?; + if socket_path.is_empty() + || server_pid.parse::().is_err() + || pane_index.parse::().is_err() + { + return None; + } + Some(socket_path) +} + /// Terminal info resolved for an instance. #[derive(Debug, Clone, Default)] pub struct TerminalInfo { @@ -2843,6 +2868,11 @@ pub fn close_terminal_pane(pid: u32, info: &TerminalInfo) -> PaneCloseResult { None => return failed_without_command(), }; + if !tmux_close_has_server_identity(&argv, tmux_socket_path) { + // Without the launcher's server identity, the caller's default tmux + // server may contain an unrelated pane with the same pane ID. + return failed_without_command(); + } splice_tmux_socket(&mut argv, tmux_socket_path); let is_zellij = is_zellij_merged(&merged); @@ -3117,14 +3147,14 @@ pub fn resolve_terminal_info( .get("tmux_socket_path") .and_then(|v| v.as_str()) .filter(|v| !v.is_empty()) + .map(str::to_string) .or_else(|| { lc_env .and_then(|e| e.get("TMUX").and_then(|v| v.as_str())) - .and_then(|v| v.split(',').next()) - .filter(|v| !v.is_empty()) + .and_then(tmux_socket_path_from_env) + .map(str::to_string) }) - .unwrap_or("") - .to_string(); + .unwrap_or_default(); } // Legacy kitty launches may have pane/socket metadata but no persisted preset. @@ -3331,6 +3361,25 @@ mod tests { Some(r#"{"pane_id":"%0","env":{"TMUX":"/tmp/tmux-owner.sock,123,0"}}"#), ); assert_eq!(info.tmux_socket_path, "/tmp/tmux-owner.sock"); + + let info = resolve_terminal_info( + Some("tmux-split"), + Some(r#"{"pane_id":"%0","env":{"TMUX":"/tmp/tmux,owner.sock,123,0"}}"#), + ); + assert_eq!(info.tmux_socket_path, "/tmp/tmux,owner.sock"); + } + + #[test] + fn test_tmux_close_requires_owner_or_explicit_server() { + let tmux = argv(&["tmux", "kill-pane", "-t", "%0"]); + assert!(!tmux_close_has_server_identity(&tmux, "")); + assert!(tmux_close_has_server_identity( + &tmux, + "/tmp/tmux-owner.sock" + )); + + let explicit = argv(&["tmux", "-L", "custom", "kill-pane", "-t", "%0"]); + assert!(tmux_close_has_server_identity(&explicit, "")); } #[test] From b7a9f4a5f86700e3c50b93d0d6ab20e54214b5c9 Mon Sep 17 00:00:00 2001 From: Sean Date: Tue, 20 Oct 2026 14:23:53 +0800 Subject: [PATCH 3/5] fix(tmux): remove needless terminal defaults --- src/pidtrack.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pidtrack.rs b/src/pidtrack.rs index bfd9a81d..cd29f7bd 100644 --- a/src/pidtrack.rs +++ b/src/pidtrack.rs @@ -98,7 +98,6 @@ impl OrphanProcess { }) }) .unwrap_or_default(), - ..crate::terminal::TerminalInfo::default() } } } From 55ebca17eb2bce7808a4af5d5b3767809cb06f68 Mon Sep 17 00:00:00 2001 From: Sean Date: Tue, 20 Oct 2026 14:48:34 +0800 Subject: [PATCH 4/5] fix(tmux): parse server selectors before subcommand --- src/terminal.rs | 50 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 48 insertions(+), 2 deletions(-) diff --git a/src/terminal.rs b/src/terminal.rs index 0b17e6f1..a316f8b7 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -106,8 +106,21 @@ fn is_tmux_command(argv: &[String]) -> bool { } fn has_tmux_server_selector(argv: &[String]) -> bool { - argv.iter() - .any(|arg| arg == "-S" || arg.starts_with("-S") || arg == "-L" || arg.starts_with("-L")) + let mut index = 1; + while let Some(arg) = argv.get(index) { + // Tmux server selectors are global options, so options after the + // subcommand cannot select the server used to reach that command. + if !arg.starts_with('-') || arg == "-" { + break; + } + match arg.as_str() { + "-S" | "-L" => return argv.get(index + 1).is_some(), + "-c" | "-f" | "-T" => index += 2, + _ if arg.starts_with("-S") || arg.starts_with("-L") => return true, + _ => index += 1, + } + } + false } fn tmux_close_has_server_identity(argv: &[String], socket_path: &str) -> bool { @@ -3380,6 +3393,39 @@ mod tests { let explicit = argv(&["tmux", "-L", "custom", "kill-pane", "-t", "%0"]); assert!(tmux_close_has_server_identity(&explicit, "")); + + let subcommand_selector = argv(&["tmux", "kill-pane", "-L", "custom", "-t", "%0"]); + assert!(!tmux_close_has_server_identity(&subcommand_selector, "")); + + let selector_value = argv(&[ + "tmux", + "-c", + "-Snot-a-selector", + "-f", + "-Lnot-a-server", + "-T", + "features", + "kill-pane", + "-t", + "%0", + ]); + assert!(!tmux_close_has_server_identity(&selector_value, "")); + + let selector_after_values = argv(&[ + "tmux", + "-c", + "shell", + "-f", + "/tmp/tmux.conf", + "-T", + "features", + "-S", + "/tmp/tmux-owner.sock", + "kill-pane", + "-t", + "%0", + ]); + assert!(tmux_close_has_server_identity(&selector_after_values, "")); } #[test] From 324cfc7216e20a15c1bad77be509c2da253a5389 Mon Sep 17 00:00:00 2001 From: aannoo Date: Tue, 20 Oct 2026 09:11:02 +0100 Subject: [PATCH 5/5] Replace server selector with a pid-checked close on the owning server Record the pane's TMUX and close through it, and skip the close unless that server still has the pane under the same server pid (pane ids restart at %0 when a server restarts on the same socket). The check times out so a hung server can't block the kill. Also run the tmux-split script via bash, like the tmux preset. --- src/commands/kill.rs | 1 + src/hooks/common.rs | 2 + src/instance_binding.rs | 95 ++----------- src/pidtrack.rs | 162 +++------------------- src/shared/terminal_presets.rs | 2 +- src/terminal.rs | 245 ++++++++++++--------------------- 6 files changed, 127 insertions(+), 380 deletions(-) diff --git a/src/commands/kill.rs b/src/commands/kill.rs index 412c77e3..d1068211 100644 --- a/src/commands/kill.rs +++ b/src/commands/kill.rs @@ -887,6 +887,7 @@ mod tests { zellij_session_name: String::new(), herdr_socket_path: String::new(), wezterm_unix_socket: String::new(), + tmux: String::new(), session_id: String::new(), notify_port: 0, inject_port: 0, diff --git a/src/hooks/common.rs b/src/hooks/common.rs index 235795a1..e5681729 100644 --- a/src/hooks/common.rs +++ b/src/hooks/common.rs @@ -1289,6 +1289,7 @@ fn stop_instance_inner( let zellij_session_name = ti.zellij_session_name; let herdr_socket_path = ti.herdr_socket_path; let wezterm_unix_socket = ti.wezterm_unix_socket; + let tmux = ti.tmux; // Fallback: process_bindings table if proc_id.is_empty() && let Ok(mut stmt) = db @@ -1332,6 +1333,7 @@ fn stop_instance_inner( zellij_session_name: &zellij_session_name, herdr_socket_path: &herdr_socket_path, wezterm_unix_socket: &wezterm_unix_socket, + tmux: &tmux, session_id: instance_data.session_id.as_deref().unwrap_or(""), notify_port, inject_port, diff --git a/src/instance_binding.rs b/src/instance_binding.rs index 0f606603..6f31edf7 100644 --- a/src/instance_binding.rs +++ b/src/instance_binding.rs @@ -112,72 +112,17 @@ pub fn capture_and_store_launch_context(db: &HcomDb, instance_name: &str) { // not be overwritten with a stale pid_identity (or lose its identity). let result = db.with_write_scope(|| { let mut ctx = new_ctx; - if let Some(pos) = db.get_instance_full(instance_name)? + if !missing.is_empty() + && let Some(pos) = db.get_instance_full(instance_name)? && let Some(old_json) = &pos.launch_context && let Ok(old_ctx) = serde_json::from_str::(old_json) { - if !missing.is_empty() { - for k in &missing { - if let Some(val) = old_ctx.get(*k) - && !launch_context_value_missing(Some(val)) - { - ctx.insert(k.to_string(), val.clone()); - } - } - } - - let old_process_id = old_ctx.get("process_id").and_then(|v| v.as_str()); - let new_process_id = ctx.get("process_id").and_then(|v| v.as_str()); - let old_socket = old_ctx - .get("tmux_socket_path") - .and_then(|v| v.as_str()) - .filter(|v| !v.is_empty()) - .map(str::to_string) - .or_else(|| { - let socket = - crate::terminal::resolve_terminal_info_from_launch_context(old_json) - .tmux_socket_path; - (!socket.is_empty()).then_some(socket) - }); - - // Keep the socket captured for this process when later hooks run - // outside its tmux pane or inherit the caller's TMUX value. - if old_process_id.is_some() && old_process_id == new_process_id { - if let Some(old_socket) = old_socket { - ctx.insert("tmux_socket_path".into(), serde_json::json!(old_socket)); - if let Some(old_tmux) = old_ctx - .get("env") - .and_then(|env| env.get("TMUX")) - .filter(|value| !launch_context_value_missing(Some(value))) - { - let env = ctx.entry("env").or_insert_with(|| serde_json::json!({})); - if !env.is_object() { - *env = serde_json::json!({}); - } - env["TMUX"] = old_tmux.clone(); - } - } else if let Some(env) = ctx.get_mut("env").and_then(|v| v.as_object_mut()) { - // This process has no previously verified socket, so a - // later hook's TMUX may belong to its caller. Refuse to - // infer a pane owner from that value. - env.remove("TMUX"); + for k in &missing { + if let Some(val) = old_ctx.get(*k) + && !launch_context_value_missing(Some(val)) + { + ctx.insert(k.to_string(), val.clone()); } - } else { - let current_json = serde_json::Value::Object(ctx.clone()).to_string(); - let current_socket = - crate::terminal::resolve_terminal_info_from_launch_context(¤t_json) - .tmux_socket_path; - if !current_socket.is_empty() { - ctx.insert("tmux_socket_path".into(), serde_json::json!(current_socket)); - } - } - } else { - let current_json = serde_json::Value::Object(ctx.clone()).to_string(); - let current_socket = - crate::terminal::resolve_terminal_info_from_launch_context(¤t_json) - .tmux_socket_path; - if !current_socket.is_empty() { - ctx.insert("tmux_socket_path".into(), serde_json::json!(current_socket)); } } @@ -255,8 +200,8 @@ fn capture_context() -> serde_json::Map { "GUAKE_TAB_UUID", "WT_SESSION", "ConEmuHWND", - "TMUX_PANE", "TMUX", + "TMUX_PANE", "STY", "ZELLIJ_SESSION_NAME", "ZELLIJ_PANE_ID", @@ -1487,8 +1432,7 @@ mod tests { fn test_capture_and_store_launch_context_preserves_terminal_metadata() { // Preserve only the fields we can't recapture from hook env: // pane_id, terminal_id, kitty_listen_on, process_id, and the resolved - // terminal preset name. The owning tmux socket must also survive a - // later hook that runs with the caller's TMUX environment. + // terminal preset name. let (db, path) = setup_test_db(); db.conn() .execute( @@ -1497,26 +1441,25 @@ mod tests { "luna", "claude", 1.0f64, - r#"{"terminal_preset_effective":"tmux-split","pane_id":"%1","process_id":"proc-1","pid_identity":"spawn-1","env":{"TMUX":"/tmp/tmux-owner.sock,123,0"}}"# + r#"{"terminal_preset_effective":"herdr","pane_id":"p_7","process_id":"proc-1","pid_identity":"spawn-1"}"# ], ) .unwrap(); let _preset = EnvVarGuard::set("HCOM_LAUNCHED_PRESET", ""); - let _process_id = EnvVarGuard::set("HCOM_PROCESS_ID", "proc-1"); - let _tmux = EnvVarGuard::set("TMUX", "/tmp/tmux-parent.sock,456,0"); + let _process_id = EnvVarGuard::set("HCOM_PROCESS_ID", ""); capture_and_store_launch_context(&db, "luna"); let row = db.get_instance_full("luna").unwrap().unwrap(); - assert_eq!(row.terminal_preset_effective.as_deref(), Some("tmux-split")); + assert_eq!(row.terminal_preset_effective.as_deref(), Some("herdr")); let ctx: serde_json::Value = serde_json::from_str(row.launch_context.as_deref().unwrap_or("{}")).unwrap(); assert_eq!( ctx.get("terminal_preset_effective") .and_then(|v| v.as_str()), - Some("tmux-split") + Some("herdr") ); - assert_eq!(ctx.get("pane_id").and_then(|v| v.as_str()), Some("%1")); + assert_eq!(ctx.get("pane_id").and_then(|v| v.as_str()), Some("p_7")); assert_eq!( ctx.get("process_id").and_then(|v| v.as_str()), Some("proc-1") @@ -1525,16 +1468,6 @@ mod tests { ctx.get("pid_identity").and_then(|v| v.as_str()), Some("spawn-1") ); - assert_eq!( - ctx.get("env") - .and_then(|env| env.get("TMUX")) - .and_then(|v| v.as_str()), - Some("/tmp/tmux-owner.sock,123,0") - ); - assert_eq!( - ctx.get("tmux_socket_path").and_then(|v| v.as_str()), - Some("/tmp/tmux-owner.sock") - ); cleanup(path); } diff --git a/src/pidtrack.rs b/src/pidtrack.rs index cd29f7bd..0784af66 100644 --- a/src/pidtrack.rs +++ b/src/pidtrack.rs @@ -34,7 +34,7 @@ pub struct PidEntry { #[serde(default, skip_serializing_if = "String::is_empty")] pub wezterm_unix_socket: String, #[serde(default, skip_serializing_if = "String::is_empty")] - pub tmux_socket_path: String, + pub tmux: String, #[serde(default, skip_serializing_if = "String::is_empty")] pub session_id: String, #[serde(default, skip_serializing_if = "is_zero")] @@ -64,6 +64,7 @@ pub struct OrphanProcess { pub zellij_session_name: String, pub herdr_socket_path: String, pub wezterm_unix_socket: String, + pub tmux: String, pub session_id: String, pub notify_port: u16, pub inject_port: u16, @@ -82,22 +83,7 @@ impl OrphanProcess { zellij_session_name: self.zellij_session_name.clone(), herdr_socket_path: self.herdr_socket_path.clone(), wezterm_unix_socket: self.wezterm_unix_socket.clone(), - tmux_socket_path: read_raw(&crate::paths::hcom_dir()) - .get(&self.pid.to_string()) - .map(|entry| entry.tmux_socket_path.clone()) - .filter(|socket| !socket.is_empty()) - .or_else(|| { - self.names.iter().find_map(|name| { - let socket = tmux_socket_path_for_instance( - &crate::paths::hcom_dir(), - name, - self.pid, - &self.process_id, - ); - (!socket.is_empty()).then_some(socket) - }) - }) - .unwrap_or_default(), + tmux: self.tmux.clone(), } } } @@ -117,6 +103,7 @@ impl From<(u32, &PidEntry)> for OrphanProcess { zellij_session_name: entry.zellij_session_name.clone(), herdr_socket_path: entry.herdr_socket_path.clone(), wezterm_unix_socket: entry.wezterm_unix_socket.clone(), + tmux: entry.tmux.clone(), session_id: entry.session_id.clone(), notify_port: entry.notify_port, inject_port: entry.inject_port, @@ -150,43 +137,6 @@ fn write_raw(hcom_dir: &Path, data: &HashMap) { } } -/// Read the owning pane's socket from the matching instance row. The PID or -/// process identity must match so a reused instance name cannot supply a -/// different process's terminal metadata. -fn tmux_socket_path_for_instance( - hcom_dir: &Path, - name: &str, - pid: u32, - process_id: &str, -) -> String { - let db_path = hcom_dir.join("hcom.db"); - if !db_path.is_file() { - return String::new(); - } - let Ok(db) = crate::db::HcomDb::open_at(&db_path) else { - return String::new(); - }; - let Ok(Some(row)) = db.get_instance_full(name) else { - return String::new(); - }; - let Some(context) = row.launch_context.as_deref() else { - return String::new(); - }; - let context_value: serde_json::Value = - serde_json::from_str(context).unwrap_or(serde_json::Value::Null); - let context_process_id = context_value - .get("process_id") - .and_then(serde_json::Value::as_str) - .unwrap_or(""); - let identity_matches = !process_id.is_empty() - && context_process_id == process_id - && row.pid == Some(i64::from(pid)); - if !identity_matches { - return String::new(); - } - crate::terminal::resolve_terminal_info_from_launch_context(context).tmux_socket_path -} - /// Parameters for recording a launched process. #[derive(Debug)] pub struct PidRecord<'a> { @@ -203,6 +153,7 @@ pub struct PidRecord<'a> { pub zellij_session_name: &'a str, pub herdr_socket_path: &'a str, pub wezterm_unix_socket: &'a str, + pub tmux: &'a str, pub session_id: &'a str, pub notify_port: u16, pub inject_port: u16, @@ -232,6 +183,7 @@ impl<'a> PidRecord<'a> { zellij_session_name: "", herdr_socket_path: "", wezterm_unix_socket: "", + tmux: "", session_id: "", notify_port: 0, inject_port: 0, @@ -256,16 +208,12 @@ pub fn record_pid(rec: &PidRecord<'_>) { zellij_session_name, herdr_socket_path, wezterm_unix_socket, + tmux, session_id, notify_port, inject_port, tag, } = rec; - let tmux_socket_path = if terminal_preset.starts_with("tmux") { - tmux_socket_path_for_instance(hcom_dir, name, *pid, process_id) - } else { - String::new() - }; let mut data = read_raw(hcom_dir); let key = pid.to_string(); @@ -299,8 +247,8 @@ pub fn record_pid(rec: &PidRecord<'_>) { if !wezterm_unix_socket.is_empty() && entry.wezterm_unix_socket.is_empty() { entry.wezterm_unix_socket = wezterm_unix_socket.to_string(); } - if !tmux_socket_path.is_empty() && entry.tmux_socket_path.is_empty() { - entry.tmux_socket_path = tmux_socket_path.clone(); + if !tmux.is_empty() && entry.tmux.is_empty() { + entry.tmux = tmux.to_string(); } if !session_id.is_empty() && entry.session_id.is_empty() { entry.session_id = session_id.to_string(); @@ -330,7 +278,7 @@ pub fn record_pid(rec: &PidRecord<'_>) { zellij_session_name: zellij_session_name.to_string(), herdr_socket_path: herdr_socket_path.to_string(), wezterm_unix_socket: wezterm_unix_socket.to_string(), - tmux_socket_path, + tmux: tmux.to_string(), session_id: session_id.to_string(), notify_port: *notify_port, inject_port: *inject_port, @@ -363,25 +311,8 @@ pub fn get_orphan_processes( } } - // Recover a socket recorded in the matching instance row when the early - // PID record was written before the terminal hook captured its context. - let mut changed = alive.len() != data.len(); - for (pid_str, entry) in &mut alive { - if entry.tmux_socket_path.is_empty() - && entry.terminal_preset.starts_with("tmux") - && let Ok(pid) = pid_str.parse::() - { - for name in &entry.names { - let socket = tmux_socket_path_for_instance(hcom_dir, name, pid, &entry.process_id); - if !socket.is_empty() { - entry.tmux_socket_path = socket; - changed = true; - break; - } - } - } - } - if changed { + // Write back pruned data if anything was removed + if alive.len() != data.len() { write_raw(hcom_dir, &alive); } @@ -585,6 +516,7 @@ fn attach_runtime_state( ("ZELLIJ_SESSION_NAME", &orphan.zellij_session_name), ("HERDR_SOCKET_PATH", &orphan.herdr_socket_path), ("WEZTERM_UNIX_SOCKET", &orphan.wezterm_unix_socket), + ("TMUX", &orphan.tmux), ] { if value.is_empty() { continue; @@ -705,20 +637,6 @@ mod tests { #[test] fn test_record_and_read() { let dir = make_temp_dir(); - let db = crate::db::HcomDb::open_at(&dir.path().join("hcom.db")).unwrap(); - db.conn() - .execute( - "INSERT INTO instances (name, tool, created_at, pid, launch_context) VALUES (?1, ?2, ?3, ?4, ?5)", - rusqlite::params![ - "luna", - "claude", - 1.0f64, - 12345i64, - r#"{"process_id":"pid-1","tmux_socket_path":"/tmp/tmux-owner.sock"}"# - ], - ) - .unwrap(); - drop(db); record_pid(&PidRecord { hcom_dir: dir.path(), pid: 12345, @@ -726,13 +644,14 @@ mod tests { name: "luna", directory: "/tmp", process_id: "pid-1", - terminal_preset: "tmux-split", + terminal_preset: "kitty", pane_id: "pane-1", terminal_id: "term-1", kitty_listen_on: "/tmp/kitty.sock", zellij_session_name: "wise-kangaroo", herdr_socket_path: "/run/herdr/sessions/work/herdr.sock", wezterm_unix_socket: "", + tmux: "/tmp/tmux-1000/default,4242,0", session_id: "sess-1", notify_port: 8080, inject_port: 8081, @@ -745,7 +664,7 @@ mod tests { assert_eq!(entry.tool, "claude"); assert_eq!(entry.names, vec!["luna"]); assert_eq!(entry.process_id, "pid-1"); - assert_eq!(entry.terminal_preset, "tmux-split"); + assert_eq!(entry.terminal_preset, "kitty"); assert_eq!(entry.pane_id, "pane-1"); assert_eq!(entry.terminal_id, "term-1"); assert_eq!(entry.kitty_listen_on, "/tmp/kitty.sock"); @@ -757,58 +676,17 @@ mod tests { assert_eq!(entry.session_id, "sess-1"); assert_eq!(entry.notify_port, 8080); assert_eq!(entry.inject_port, 8081); - assert_eq!(entry.tmux_socket_path, "/tmp/tmux-owner.sock"); - // Orphan conversion preserves terminal metadata for pane cleanup. + // An orphan closes its pane on the herdr server that owns it. let orphan = OrphanProcess::from((12345, entry)); let info = orphan.terminal_info(); - assert_eq!(info.preset_name, "tmux-split"); + assert_eq!(info.preset_name, "kitty"); assert_eq!(info.pane_id, "pane-1"); assert_eq!( info.herdr_socket_path, "/run/herdr/sessions/work/herdr.sock" ); - assert_eq!( - tmux_socket_path_for_instance(dir.path(), "luna", 12345, "pid-1"), - "/tmp/tmux-owner.sock" - ); - assert!(tmux_socket_path_for_instance(dir.path(), "luna", 12345, "pid-other").is_empty()); - } - - #[test] - #[serial_test::serial] - fn test_orphan_terminal_info_uses_persisted_tmux_socket() { - let (_dir, hcom_dir, _home, _guard) = crate::hooks::test_helpers::isolated_test_env(); - let db = crate::db::HcomDb::open().unwrap(); - let pid = std::process::id(); - let process_id = "tmux-owner-process"; - db.conn() - .execute( - "INSERT INTO instances (name, tool, created_at, pid, launch_context) VALUES (?1, ?2, ?3, ?4, ?5)", - rusqlite::params![ - "luna", - "claude", - 1.0f64, - i64::from(pid), - r#"{"process_id":"tmux-owner-process","tmux_socket_path":"/tmp/tmux-owner.sock"}"# - ], - ) - .unwrap(); - drop(db); - - record_pid(&PidRecord { - process_id, - terminal_preset: "tmux-split", - pane_id: "%1", - ..rec(&hcom_dir, pid, "claude", "luna") - }); - - let orphans = get_orphan_processes(&hcom_dir, None); - let orphan = orphans.iter().find(|orphan| orphan.pid == pid).unwrap(); - assert_eq!( - orphan.terminal_info().tmux_socket_path, - "/tmp/tmux-owner.sock" - ); + assert_eq!(info.tmux, "/tmp/tmux-1000/default,4242,0"); } #[test] @@ -949,6 +827,7 @@ mod tests { zellij_session_name: String::new(), herdr_socket_path: String::new(), wezterm_unix_socket: String::new(), + tmux: String::new(), session_id: String::new(), notify_port: 0, inject_port: 0, @@ -980,6 +859,7 @@ mod tests { zellij_session_name: String::new(), herdr_socket_path: String::new(), wezterm_unix_socket: String::new(), + tmux: String::new(), session_id: "sess-retry".into(), notify_port: 0, inject_port: 0, diff --git a/src/shared/terminal_presets.rs b/src/shared/terminal_presets.rs index c71e0306..a7f0ff55 100644 --- a/src/shared/terminal_presets.rs +++ b/src/shared/terminal_presets.rs @@ -459,7 +459,7 @@ pub static TERMINAL_PRESETS: LazyLock> = Laz p( Some("tmux"), None, - argv(&["tmux", "split-window", "-h", "{script}"]), + argv(&["tmux", "split-window", "-h", "bash", "{script}"]), argv(&["tmux", "kill-pane", "-t", "{pane_id}"]), Some("TMUX_PANE"), DL, diff --git a/src/terminal.rs b/src/terminal.rs index a316f8b7..836a9b30 100644 --- a/src/terminal.rs +++ b/src/terminal.rs @@ -98,57 +98,57 @@ fn is_wezterm_argv(argv: &[String]) -> bool { .is_some_and(|stem| stem.eq_ignore_ascii_case("wezterm")) } -fn is_tmux_command(argv: &[String]) -> bool { +/// True if `argv` runs the tmux CLI (`tmux` or a full path). +fn is_tmux_argv(argv: &[String]) -> bool { argv.first() .and_then(|bin| std::path::Path::new(bin).file_stem()) .and_then(|stem| stem.to_str()) - .is_some_and(|stem| stem.eq_ignore_ascii_case("tmux")) + .is_some_and(|stem| stem == "tmux") } -fn has_tmux_server_selector(argv: &[String]) -> bool { - let mut index = 1; - while let Some(arg) = argv.get(index) { - // Tmux server selectors are global options, so options after the - // subcommand cannot select the server used to reach that command. - if !arg.starts_with('-') || arg == "-" { - break; - } - match arg.as_str() { - "-S" | "-L" => return argv.get(index + 1).is_some(), - "-c" | "-f" | "-T" => index += 2, - _ if arg.starts_with("-S") || arg.starts_with("-L") => return true, - _ => index += 1, +/// True if `pane_id` exists on the tmux server named by `tmux` (a pane's +/// `TMUX` value) and that server is still the process that started the pane. +/// False if the server doesn't answer within `TERMINAL_CLOSE_TIMEOUT`, so a +/// hung server can't keep kill from signalling the process. +fn tmux_pane_on_server(tmux_bin: &str, tmux: &str, pane_id: &str) -> bool { + let Some(server_pid) = tmux.split(',').nth(1).filter(|pid| !pid.is_empty()) else { + return false; + }; + let Ok(mut child) = Command::new(tmux_bin) + .args(["display-message", "-p", "-t", pane_id, "#{pid}"]) + .env("TMUX", tmux) + .stdin(std::process::Stdio::null()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .spawn() + else { + return false; + }; + let started = Instant::now(); + loop { + match child.try_wait() { + Ok(Some(status)) if status.success() => break, + Ok(None) if started.elapsed() < TERMINAL_CLOSE_TIMEOUT => { + std::thread::sleep(Duration::from_millis(25)); + } + Ok(None) => { + let _ = child.kill(); + let _ = child.wait(); + eprintln!( + "Timed out after {}s asking tmux about pane {pane_id}; not closing it", + TERMINAL_CLOSE_TIMEOUT.as_secs() + ); + return false; + } + _ => return false, } } - false -} - -fn tmux_close_has_server_identity(argv: &[String], socket_path: &str) -> bool { - !is_tmux_command(argv) || !socket_path.is_empty() || has_tmux_server_selector(argv) -} - -/// Add the pane owner's tmux socket when the close template does not select one. -fn splice_tmux_socket(argv: &mut Vec, socket_path: &str) { - let has_socket = has_tmux_server_selector(argv); - if is_tmux_command(argv) && !socket_path.is_empty() && !has_socket { - argv.splice(1..1, ["-S".to_string(), socket_path.to_string()]); - } -} - -/// Parse the socket path from tmux's `TMUX=socket,pid,pane` environment value. -/// Splitting from the right preserves commas that are part of the socket path. -pub(crate) fn tmux_socket_path_from_env(value: &str) -> Option<&str> { - let mut parts = value.rsplitn(3, ','); - let pane_index = parts.next()?; - let server_pid = parts.next()?; - let socket_path = parts.next()?; - if socket_path.is_empty() - || server_pid.parse::().is_err() - || pane_index.parse::().is_err() - { - return None; - } - Some(socket_path) + let mut out = String::new(); + child + .stdout + .take() + .is_some_and(|mut stdout| std::io::Read::read_to_string(&mut stdout, &mut out).is_ok()) + && out.trim() == server_pid } /// Terminal info resolved for an instance. @@ -164,8 +164,8 @@ pub struct TerminalInfo { pub herdr_socket_path: String, /// The pane's `WEZTERM_UNIX_SOCKET`: which wezterm GUI process owns it. pub wezterm_unix_socket: String, - /// The pane's tmux server socket path; pane IDs are unique only per server. - pub tmux_socket_path: String, + /// The pane's `TMUX` (`socket,server_pid,session`): which tmux server owns it. + pub tmux: String, } /// Result from launch_terminal. @@ -2845,7 +2845,7 @@ pub fn close_terminal_pane(pid: u32, info: &TerminalInfo) -> PaneCloseResult { zellij_session_name, herdr_socket_path, wezterm_unix_socket, - tmux_socket_path, + tmux, } = info; let failed_without_command = || PaneCloseResult { closed: false, @@ -2881,13 +2881,6 @@ pub fn close_terminal_pane(pid: u32, info: &TerminalInfo) -> PaneCloseResult { None => return failed_without_command(), }; - if !tmux_close_has_server_identity(&argv, tmux_socket_path) { - // Without the launcher's server identity, the caller's default tmux - // server may contain an unrelated pane with the same pane ID. - return failed_without_command(); - } - splice_tmux_socket(&mut argv, tmux_socket_path); - let is_zellij = is_zellij_merged(&merged); let zellij_before_close = if is_zellij { @@ -2949,10 +2942,18 @@ pub fn close_terminal_pane(pid: u32, info: &TerminalInfo) -> PaneCloseResult { // environment would close another session's pane. // Same for wezterm: pane ids are per GUI process, and without the socket // `wezterm cli` talks to the most recently started GUI. + // And tmux: pane ids are per server and the CLI picks the server from + // `TMUX`. They also restart at %0 with each server, so a server restarted + // on the same socket may hold an unrelated pane with this id. let close_env: Vec<(&str, &str)> = if is_herdr_argv(&argv) && !herdr_socket_path.is_empty() { vec![("HERDR_SOCKET_PATH", herdr_socket_path.as_str())] } else if is_wezterm_argv(&argv) && !wezterm_unix_socket.is_empty() { vec![("WEZTERM_UNIX_SOCKET", wezterm_unix_socket.as_str())] + } else if is_tmux_argv(&argv) && !tmux.is_empty() { + if !tmux_pane_on_server(&argv[0], tmux, effective_pane_id) { + return failed_without_command(); + } + vec![("TMUX", tmux.as_str())] } else { Vec::new() }; @@ -3156,18 +3157,10 @@ pub fn resolve_terminal_info( .and_then(|e| e.get("WEZTERM_UNIX_SOCKET").and_then(|v| v.as_str())) .unwrap_or("") .to_string(); - info.tmux_socket_path = lc - .get("tmux_socket_path") - .and_then(|v| v.as_str()) - .filter(|v| !v.is_empty()) - .map(str::to_string) - .or_else(|| { - lc_env - .and_then(|e| e.get("TMUX").and_then(|v| v.as_str())) - .and_then(tmux_socket_path_from_env) - .map(str::to_string) - }) - .unwrap_or_default(); + info.tmux = lc_env + .and_then(|e| e.get("TMUX").and_then(|v| v.as_str())) + .unwrap_or("") + .to_string(); } // Legacy kitty launches may have pane/socket metadata but no persisted preset. @@ -3361,97 +3354,6 @@ mod tests { assert_eq!(info.terminal_id, "term-1"); } - #[test] - fn test_resolve_terminal_info_reads_tmux_socket_path() { - let info = resolve_terminal_info( - Some("tmux-split"), - Some(r#"{"pane_id":"%0","tmux_socket_path":"/tmp/tmux-owner.sock"}"#), - ); - assert_eq!(info.tmux_socket_path, "/tmp/tmux-owner.sock"); - - let info = resolve_terminal_info( - Some("tmux-split"), - Some(r#"{"pane_id":"%0","env":{"TMUX":"/tmp/tmux-owner.sock,123,0"}}"#), - ); - assert_eq!(info.tmux_socket_path, "/tmp/tmux-owner.sock"); - - let info = resolve_terminal_info( - Some("tmux-split"), - Some(r#"{"pane_id":"%0","env":{"TMUX":"/tmp/tmux,owner.sock,123,0"}}"#), - ); - assert_eq!(info.tmux_socket_path, "/tmp/tmux,owner.sock"); - } - - #[test] - fn test_tmux_close_requires_owner_or_explicit_server() { - let tmux = argv(&["tmux", "kill-pane", "-t", "%0"]); - assert!(!tmux_close_has_server_identity(&tmux, "")); - assert!(tmux_close_has_server_identity( - &tmux, - "/tmp/tmux-owner.sock" - )); - - let explicit = argv(&["tmux", "-L", "custom", "kill-pane", "-t", "%0"]); - assert!(tmux_close_has_server_identity(&explicit, "")); - - let subcommand_selector = argv(&["tmux", "kill-pane", "-L", "custom", "-t", "%0"]); - assert!(!tmux_close_has_server_identity(&subcommand_selector, "")); - - let selector_value = argv(&[ - "tmux", - "-c", - "-Snot-a-selector", - "-f", - "-Lnot-a-server", - "-T", - "features", - "kill-pane", - "-t", - "%0", - ]); - assert!(!tmux_close_has_server_identity(&selector_value, "")); - - let selector_after_values = argv(&[ - "tmux", - "-c", - "shell", - "-f", - "/tmp/tmux.conf", - "-T", - "features", - "-S", - "/tmp/tmux-owner.sock", - "kill-pane", - "-t", - "%0", - ]); - assert!(tmux_close_has_server_identity(&selector_after_values, "")); - } - - #[test] - fn test_splice_tmux_socket_targets_the_pane_owner_server() { - let mut command = argv(&["tmux", "kill-pane", "-t", "%0"]); - splice_tmux_socket(&mut command, "/tmp/tmux owner.sock"); - assert_eq!( - command, - argv(&[ - "tmux", - "-S", - "/tmp/tmux owner.sock", - "kill-pane", - "-t", - "%0" - ]) - ); - - let mut explicit = argv(&["tmux", "-L", "custom", "kill-pane", "-t", "%0"]); - splice_tmux_socket(&mut explicit, "/tmp/tmux owner.sock"); - assert_eq!( - explicit, - argv(&["tmux", "-L", "custom", "kill-pane", "-t", "%0"]) - ); - } - #[test] fn test_launcher_env_preserves_zellij_session_but_strips_pane() { let env = get_launcher_env_from(vec![ @@ -4616,6 +4518,35 @@ mod tests { assert_eq!(info.herdr_socket_path, "/run/herdr/work.sock"); } + #[test] + fn test_resolve_terminal_info_reads_tmux_from_env_snapshot() { + let info = resolve_terminal_info( + Some("tmux-split"), + Some(r#"{"pane_id":"%1","env":{"TMUX":"/tmp/tmux-1000/other,4242,0"}}"#), + ); + assert_eq!(info.pane_id, "%1"); + assert_eq!(info.tmux, "/tmp/tmux-1000/other,4242,0"); + } + + #[test] + fn test_is_tmux_argv() { + assert!(is_tmux_argv(&["tmux".to_string()])); + assert!(is_tmux_argv(&["/opt/homebrew/bin/tmux".to_string()])); + assert!(!is_tmux_argv(&["tmuxinator".to_string()])); + assert!(!is_tmux_argv(&[])); + } + + #[test] + fn test_tmux_pane_on_server_needs_server_pid() { + // Without the server pid the pane's server can't be confirmed. + assert!(!tmux_pane_on_server("tmux", "/tmp/tmux-1000/default", "%1")); + assert!(!tmux_pane_on_server( + "tmux", + "/tmp/tmux-1000/default,,0", + "%1" + )); + } + #[test] fn test_is_herdr_argv_matches_bare_name_and_paths() { let is = |bin: &str| is_herdr_argv(&[bin.to_string(), "pane".to_string()]);