From b7cc4ccbe077db2e121ada261919d37a5bc9fba9 Mon Sep 17 00:00:00 2001 From: jvm Date: Thu, 30 Jul 2026 18:00:34 +0100 Subject: [PATCH 1/6] fix: propagate the attached client's terminal identity to panes Panes inherit a snapshot of the server process environment, and the server is persistent: its TERM_PROGRAM belongs to whichever terminal started it, not to the client currently attached. Under `herdr --remote` the server is started by sshd and has no terminal identity at all, so remote panes saw no TERM_PROGRAM and applications fell back to their no-inline-image path. Carry the identity in ClientMessage::Hello, store it per connection, and apply it in apply_pane_terminal_env, which every pane spawn path already funnels through. The SSH bridge forwards no environment, so the wire is the only channel that reaches a remote server. TERM stays xterm-256color; only the outer identity is propagated. With no attached client TERM_PROGRAM is removed rather than left stale. --- docs/next/api/herdr-api.schema.json | 2 +- src/client/mod.rs | 56 +++++++++++++++++++++++++ src/pane.rs | 64 ++++++++++++++++++++++++++++- src/protocol/wire.rs | 12 +++++- src/server/autodetect.rs | 1 + src/server/client_transport.rs | 9 ++++ src/server/clients.rs | 6 +++ src/server/headless.rs | 21 ++++++++++ tests/api_ping.rs | 2 +- tests/cli/sessions.rs | 12 +++--- tests/cross_area.rs | 1 + tests/multi_client.rs | 1 + tests/server_headless.rs | 1 + tests/support/mod.rs | 3 +- 14 files changed, 180 insertions(+), 11 deletions(-) diff --git a/docs/next/api/herdr-api.schema.json b/docs/next/api/herdr-api.schema.json index 59a0c6a331..b4704cd15c 100644 --- a/docs/next/api/herdr-api.schema.json +++ b/docs/next/api/herdr-api.schema.json @@ -1,6 +1,6 @@ { "$schema": "https://json-schema.org/draft/2020-12/schema", - "protocol": 18, + "protocol": 19, "schema_version": 1, "schemas": { "error_response": { diff --git a/src/client/mod.rs b/src/client/mod.rs index 52fd6dd698..dadfbfb47c 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -763,6 +763,7 @@ fn do_handshake( } else { ClientLaunchMode::App }, + term_program: outer_term_program(), }; protocol::write_message(stream, &hello) .map_err(|e| ClientError::ConnectionFailed(io::Error::other(e.to_string())))?; @@ -2104,6 +2105,32 @@ fn current_terminal_geometry(kitty_graphics_enabled: bool) -> (u16, u16, u32, u3 ) } +/// Identifies the outer terminal emulator that renders this client's output. +/// +/// The server is a persistent process whose own environment belongs to whichever +/// terminal happened to start it, so it cannot infer this. Sending it in `Hello` is +/// the only channel that survives the remote SSH bridge, which forwards no environment. +fn outer_term_program() -> Option { + if let Ok(value) = std::env::var("TERM_PROGRAM") { + if !value.is_empty() { + return Some(value); + } + } + // Kitty deliberately ships no TERM_PROGRAM, so fall back to the identity + // terminal_notify already infers from KITTY_WINDOW_ID/TERM, spelled the way + // TERM_PROGRAM-reading consumers expect. + use crate::terminal_notify::TerminalNotificationBackend as Backend; + Some( + match crate::terminal_notify::detect_backend()? { + Backend::Ghostty => "ghostty", + Backend::Iterm2 => "iTerm.app", + Backend::Kitty => "kitty", + Backend::WezTerm => "WezTerm", + } + .to_string(), + ) +} + /// Reports polled changes and signalled resizes that return to the same size. fn resize_report_required( signalled: bool, @@ -2261,6 +2288,35 @@ mod tests { ); } + const TERMINAL_IDENTITY_ENV: [&str; 3] = ["TERM_PROGRAM", "TERM", "KITTY_WINDOW_ID"]; + + #[test] + fn outer_term_program_prefers_the_terminals_own_report() { + let _guard = env_lock().lock().unwrap(); + let _cleared = EnvVarsRemovedGuard::new(&TERMINAL_IDENTITY_ENV); + let _env = EnvVarGuard::set("TERM_PROGRAM", "vscode"); + + assert_eq!(outer_term_program().as_deref(), Some("vscode")); + } + + #[test] + fn outer_term_program_infers_kitty_which_sets_no_term_program() { + let _guard = env_lock().lock().unwrap(); + let _cleared = EnvVarsRemovedGuard::new(&TERMINAL_IDENTITY_ENV); + let _env = EnvVarGuard::set("KITTY_WINDOW_ID", "3"); + + assert_eq!(outer_term_program().as_deref(), Some("kitty")); + } + + #[test] + fn outer_term_program_is_none_for_an_unidentified_terminal() { + let _guard = env_lock().lock().unwrap(); + let _cleared = EnvVarsRemovedGuard::new(&TERMINAL_IDENTITY_ENV); + let _env = EnvVarGuard::set("TERM", "xterm-256color"); + + assert_eq!(outer_term_program(), None); + } + #[test] fn host_cursor_policy_native_and_drawn_override_auto_detection() { let _guard = env_lock().lock().unwrap(); diff --git a/src/pane.rs b/src/pane.rs index be1a23da20..e5e8566262 100644 --- a/src/pane.rs +++ b/src/pane.rs @@ -3,7 +3,7 @@ use std::io; use std::path::Path; use std::sync::{ atomic::{AtomicBool, AtomicU16, AtomicU32, AtomicU64, Ordering}, - Arc, Mutex, + Arc, Mutex, RwLock, }; use bytes::Bytes; @@ -54,6 +54,22 @@ const RELEASE_REACQUIRE_SUPPRESSION: std::time::Duration = std::time::Duration:: const PANE_TERM: &str = "xterm-256color"; const PANE_COLORTERM: &str = "truecolor"; +/// Terminal emulator identity reported by the foreground client, or `None` when no +/// client is attached or it could not be determined. +/// +/// Panes inherit a snapshot of this process's environment, and the server is persistent: +/// its own terminal variables belong to whichever terminal started it, which may be a +/// different terminal from the attached client's, or none at all under a remote SSH +/// bridge. The attached client is the only authority on what actually renders pane output. +static OUTER_TERM_PROGRAM: RwLock> = RwLock::new(None); + +pub(crate) fn set_outer_term_program(term_program: Option) { + match OUTER_TERM_PROGRAM.write() { + Ok(mut guard) => *guard = term_program, + Err(_) => tracing::warn!("outer terminal identity lock poisoned; panes keep last value"), + } +} + fn apply_pane_terminal_env(cmd: &mut CommandBuilder) { // Each pane is rendered by herdr's own terminal layer, not the outer terminal // that launched the app. Advertising the inherited TERM leaks the host terminal @@ -61,6 +77,18 @@ fn apply_pane_terminal_env(cmd: &mut CommandBuilder) { // when the remote side lacks matching terminfo entries. cmd.env("TERM", PANE_TERM); cmd.env("COLORTERM", PANE_COLORTERM); + + // TERM_PROGRAM names the emulator that ultimately displays this pane, which is the + // client's outer terminal rather than herdr's virtual one. Applications use it to + // pick an inline image protocol, so it must track the attached client. + let outer_term_program = match OUTER_TERM_PROGRAM.read() { + Ok(guard) => guard.clone(), + Err(_) => None, + }; + match outer_term_program { + Some(term_program) => cmd.env("TERM_PROGRAM", term_program), + None => cmd.env_remove("TERM_PROGRAM"), + } } #[derive(Debug, Clone, Default, PartialEq, Eq)] @@ -2974,6 +3002,13 @@ mod tests { assert!(!process_alive_for_shutdown(43, 42, false, |_| false)); } + /// Serializes tests that read or write the process-wide outer terminal identity. + #[cfg(unix)] + fn outer_term_program_lock() -> &'static Mutex<()> { + static LOCK: std::sync::OnceLock> = std::sync::OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())) + } + #[cfg(unix)] fn capture_shell_output(command: &str, extra_env: &[(&str, &str)]) -> String { let pair = native_pty_system() @@ -2998,6 +3033,9 @@ mod tests { cmd.cwd(std::env::current_dir().unwrap()); cmd.env("TERM", "xterm-ghostty"); cmd.env("COLORTERM", "falsecolor"); + // Stand in for the terminal identity a persistent server inherits from whichever + // terminal started it, which is not necessarily the attached client's. + cmd.env("TERM_PROGRAM", "stale-outer"); apply_pane_terminal_env(&mut cmd); for (key, value) in extra_env { cmd.env(key, value); @@ -3288,6 +3326,7 @@ mod tests { #[cfg(unix)] #[test] fn pane_terminal_identity_overrides_outer_terminal_env() { + let _guard = outer_term_program_lock().lock(); let output = capture_shell_output("printf '%s\\n%s\\n' \"$TERM\" \"$COLORTERM\"", &[]); assert_eq!(output, "xterm-256color\ntruecolor\n"); } @@ -3295,6 +3334,7 @@ mod tests { #[cfg(unix)] #[test] fn pane_terminal_identity_allows_explicit_override() { + let _guard = outer_term_program_lock().lock(); let output = capture_shell_output( "printf '%s\\n%s\\n' \"$TERM\" \"$COLORTERM\"", &[("TERM", "vt100"), ("COLORTERM", "24bit")], @@ -3302,6 +3342,28 @@ mod tests { assert_eq!(output, "vt100\n24bit\n"); } + #[cfg(unix)] + #[test] + fn pane_term_program_reports_attached_client_terminal() { + let _guard = outer_term_program_lock().lock(); + set_outer_term_program(Some("kitty".to_string())); + let output = capture_shell_output("printf '%s\\n%s\\n' \"$TERM_PROGRAM\" \"$TERM\"", &[]); + set_outer_term_program(None); + // TERM stays herdr's own virtual terminal; only the outer identity is propagated. + assert_eq!(output, "kitty\nxterm-256color\n"); + } + + #[cfg(unix)] + #[test] + fn pane_term_program_is_cleared_without_an_attached_client() { + let _guard = outer_term_program_lock().lock(); + set_outer_term_program(None); + let output = capture_shell_output("printf '%s\\n' \"$TERM_PROGRAM\"", &[]); + // A stale identity is worse than none: it makes applications pick a graphics + // protocol the real outer terminal may not support. + assert_eq!(output, "\n"); + } + #[cfg(unix)] #[tokio::test] async fn handoff_history_ansi_captures_primary_screen() { diff --git a/src/protocol/wire.rs b/src/protocol/wire.rs index 0d4b14bba7..588b265b1a 100644 --- a/src/protocol/wire.rs +++ b/src/protocol/wire.rs @@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize}; // --------------------------------------------------------------------------- /// Current protocol version. Bumped when wire format changes incompatibly. -pub const PROTOCOL_VERSION: u32 = 18; +pub const PROTOCOL_VERSION: u32 = 19; /// Maximum allowed frame payload size (2 MB). Frames larger than this are /// rejected to prevent denial-of-service via oversized length prefixes. @@ -334,6 +334,10 @@ pub enum ClientMessage { keybindings: ClientKeybindings, /// Whether this connection will render the full app or attach directly to a pane terminal. launch_mode: ClientLaunchMode, + /// Identity of the outer terminal emulator that renders this client's output, or + /// `None` when it could not be determined. The server is a persistent process whose + /// own environment may predate this client, so panes cannot infer it locally. + term_program: Option, }, /// Raw input bytes read from the client's stdin. @@ -977,6 +981,7 @@ mod tests { requested_encoding: RenderEncoding::SemanticFrame, keybindings: ClientKeybindings::Server, launch_mode: ClientLaunchMode::App, + term_program: Some("ghostty".to_string()), }; let encoded = bincode::serde::encode_to_vec(&msg, bincode::config::standard()).unwrap(); let (decoded, _): (ClientMessage, _) = @@ -1014,6 +1019,7 @@ mod tests { requested_encoding: RenderEncoding::SemanticFrame, keybindings: ClientKeybindings::Server, launch_mode: ClientLaunchMode::App, + term_program: None, }), 0 ); @@ -1469,6 +1475,7 @@ mod tests { requested_encoding: RenderEncoding::SemanticFrame, keybindings: ClientKeybindings::Server, launch_mode: ClientLaunchMode::App, + term_program: None, }; let mut buf = Vec::new(); write_message(&mut buf, &msg).unwrap(); @@ -1543,6 +1550,7 @@ mod tests { requested_encoding: RenderEncoding::SemanticFrame, keybindings: ClientKeybindings::Server, launch_mode: ClientLaunchMode::App, + term_program: None, }, 1 => ClientMessage::Input { data: vec![(i % 256) as u8; (i as usize % 50) + 1], @@ -1979,6 +1987,7 @@ mod tests { requested_encoding: RenderEncoding::SemanticFrame, keybindings: ClientKeybindings::Server, launch_mode: ClientLaunchMode::App, + term_program: None, }; let mut buf = Vec::new(); write_message(&mut buf, &msg).unwrap(); @@ -2015,6 +2024,7 @@ mod tests { requested_encoding: RenderEncoding::SemanticFrame, keybindings: ClientKeybindings::Server, launch_mode: ClientLaunchMode::App, + term_program: None, }, ClientMessage::Input { data: b"hello world".to_vec(), diff --git a/src/server/autodetect.rs b/src/server/autodetect.rs index 337c388f9a..aef4f5ed08 100644 --- a/src/server/autodetect.rs +++ b/src/server/autodetect.rs @@ -126,6 +126,7 @@ fn client_protocol_accepts_hello(socket_path: &Path) -> io::Result { requested_encoding: crate::protocol::RenderEncoding::SemanticFrame, keybindings: crate::protocol::ClientKeybindings::Server, launch_mode: crate::protocol::ClientLaunchMode::App, + term_program: None, }; match crate::protocol::write_message(&mut stream, &hello) { diff --git a/src/server/client_transport.rs b/src/server/client_transport.rs index 743a108f89..d3bbbbd17a 100644 --- a/src/server/client_transport.rs +++ b/src/server/client_transport.rs @@ -291,6 +291,7 @@ pub(crate) enum ServerEvent { render_encoding: RenderEncoding, keybindings: Option>, direct_attach_requested: bool, + term_program: Option, writer: ClientWriter, }, /// A client sent an input message. @@ -477,6 +478,7 @@ pub(crate) fn handle_client_handshake( render_encoding, keybindings, direct_attach_requested, + term_program, ) = match hello { ClientMessage::Hello { version, @@ -487,6 +489,7 @@ pub(crate) fn handle_client_handshake( requested_encoding, keybindings, launch_mode, + term_program, } => { // Version check. match protocol::check_client_version(version) { @@ -526,6 +529,7 @@ pub(crate) fn handle_client_handshake( requested_encoding, keybindings, launch_mode == ClientLaunchMode::TerminalAttach, + term_program.filter(|value| !value.is_empty()), ) } _ => { @@ -580,6 +584,7 @@ pub(crate) fn handle_client_handshake( render_encoding, keybindings, direct_attach_requested, + term_program, writer, }); @@ -1122,6 +1127,7 @@ new_tab = "ctrl+notakey" requested_encoding: RenderEncoding::TerminalAnsi, keybindings: ClientKeybindings::Server, launch_mode: ClientLaunchMode::App, + term_program: Some("ghostty".to_owned()), }, ) .expect("write hello"); @@ -1154,6 +1160,7 @@ new_tab = "ctrl+notakey" render_encoding, keybindings, direct_attach_requested, + term_program, writer, } => { assert_eq!(client_id, 42); @@ -1162,6 +1169,7 @@ new_tab = "ctrl+notakey" assert_eq!(render_encoding, RenderEncoding::TerminalAnsi); assert!(keybindings.is_none()); assert!(!direct_attach_requested); + assert_eq!(term_program.as_deref(), Some("ghostty")); drop(writer); } other => panic!("expected ClientConnected, got {other:?}"), @@ -1197,6 +1205,7 @@ new_tab = "ctrl+notakey" requested_encoding: RenderEncoding::TerminalAnsi, keybindings: ClientKeybindings::Server, launch_mode: ClientLaunchMode::TerminalAttach, + term_program: None, }, ) .expect("write hello"); diff --git a/src/server/clients.rs b/src/server/clients.rs index c6c617ecfe..387bab76f6 100644 --- a/src/server/clients.rs +++ b/src/server/clients.rs @@ -48,6 +48,9 @@ pub(crate) struct ClientConnection { pub(crate) host_terminal_appearance_explicit: bool, /// Last reported focus state for this client's outer terminal. pub(crate) outer_terminal_focus: Option, + /// Terminal emulator identity reported by this client at handshake, used as the + /// TERM_PROGRAM for panes spawned while this client is in the foreground. + pub(crate) term_program: Option, /// Stateful parser for app-client input split across transport reads. pub(crate) raw_input: crate::raw_input::RawInputFramer, /// Monotonic activity stamp used to choose the fallback foreground client. @@ -93,6 +96,7 @@ impl ClientConnection { last_activity, render_encoding, false, + None, writer, ) } @@ -107,6 +111,7 @@ impl ClientConnection { last_activity: u64, render_encoding: RenderEncoding, pending_terminal_attach: bool, + term_program: Option, writer: Option, ) -> Self { Self { @@ -121,6 +126,7 @@ impl ClientConnection { host_terminal_appearance_explicit: false, host_terminal_theme, outer_terminal_focus, + term_program, raw_input: crate::raw_input::RawInputFramer::default(), last_activity, render_state: ClientRenderState::new(render_encoding), diff --git a/src/server/headless.rs b/src/server/headless.rs index 343493cdb3..f997daa5e9 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -1054,6 +1054,7 @@ impl HeadlessServer { self.effective_size = (MIN_COLS, MIN_ROWS); self.app.state.outer_terminal_focus = None; self.app.state.host_cell_size = crate::kitty_graphics::HostCellSize::default(); + crate::pane::set_outer_term_program(None); let server_keybindings = self.server_keybindings.clone(); apply_keybindings(&mut self.app, &server_keybindings); self.sync_visible_server_config_diagnostic(false); @@ -1064,6 +1065,7 @@ impl HeadlessServer { self.effective_size = (MIN_COLS, MIN_ROWS); self.app.state.outer_terminal_focus = None; self.app.state.host_cell_size = crate::kitty_graphics::HostCellSize::default(); + crate::pane::set_outer_term_program(None); let server_keybindings = self.server_keybindings.clone(); apply_keybindings(&mut self.app, &server_keybindings); self.sync_visible_server_config_diagnostic(false); @@ -1078,6 +1080,7 @@ impl HeadlessServer { } else { crate::kitty_graphics::HostCellSize::default() }; + let term_program = client.term_program.clone(); let host_terminal_theme = client.host_terminal_theme; let host_terminal_appearance = client.host_terminal_appearance; let host_terminal_appearance_explicit = client.host_terminal_appearance_explicit; @@ -1091,6 +1094,7 @@ impl HeadlessServer { self.effective_size = terminal_size; self.app.state.outer_terminal_focus = outer_terminal_focus; self.app.state.host_cell_size = host_cell_size; + crate::pane::set_outer_term_program(term_program); apply_keybindings(&mut self.app, &keybindings); self.sync_visible_server_config_diagnostic(uses_local_keybindings); if outer_terminal_focus == Some(true) { @@ -2721,6 +2725,7 @@ impl HeadlessServer { writer, render_encoding, direct_attach_requested, + term_program, } => { if self.handoff_in_progress { if let Ok(message) = @@ -2761,6 +2766,7 @@ impl HeadlessServer { last_activity, render_encoding, direct_attach_requested, + term_program, Some(writer), ), ); @@ -5086,6 +5092,7 @@ new_tab = "prefix+t" render_encoding: RenderEncoding::SemanticFrame, keybindings: Some(Box::new(local_keybindings)), direct_attach_requested: false, + term_program: None, writer: writer_a, })); assert_eq!( @@ -5110,6 +5117,7 @@ new_tab = "prefix+t" render_encoding: RenderEncoding::SemanticFrame, keybindings: None, direct_attach_requested: false, + term_program: None, writer: writer_b, })); assert_eq!( @@ -5150,6 +5158,7 @@ new_tab = "prefix+t" render_encoding: RenderEncoding::SemanticFrame, keybindings: Some(Box::new(local_keybindings)), direct_attach_requested: false, + term_program: None, writer: writer_a, })); assert_eq!(server.app.state.config_diagnostic, without_keybindings); @@ -5163,6 +5172,7 @@ new_tab = "prefix+t" render_encoding: RenderEncoding::SemanticFrame, keybindings: None, direct_attach_requested: false, + term_program: None, writer: writer_b, })); assert_eq!( @@ -5206,6 +5216,7 @@ next_tab = "" render_encoding: RenderEncoding::SemanticFrame, keybindings: Some(Box::new(local_keybindings)), direct_attach_requested: false, + term_program: None, writer, })); server.app.state.mode = crate::app::Mode::Settings; @@ -5281,6 +5292,7 @@ next_tab = "" render_encoding: RenderEncoding::SemanticFrame, keybindings: Some(Box::new(local_config.live_keybinds().unwrap())), direct_attach_requested: false, + term_program: None, writer: writer_a, })); server.app.state.mode = crate::app::Mode::Settings; @@ -5301,6 +5313,7 @@ next_tab = "" render_encoding: RenderEncoding::SemanticFrame, keybindings: None, direct_attach_requested: false, + term_program: None, writer: writer_b, })); assert_eq!( @@ -5335,6 +5348,7 @@ next_tab = "" render_encoding: RenderEncoding::TerminalAnsi, keybindings: None, direct_attach_requested: true, + term_program: None, writer, })); assert!(server.clients.contains_key(&7)); @@ -5400,6 +5414,7 @@ next_tab = "" render_encoding: RenderEncoding::TerminalAnsi, keybindings: None, direct_attach_requested: true, + term_program: None, writer, })); control_rx @@ -5702,6 +5717,7 @@ next_tab = "" render_encoding, keybindings: None, direct_attach_requested: false, + term_program: None, writer, })); @@ -5736,6 +5752,7 @@ next_tab = "" render_encoding: RenderEncoding::TerminalAnsi, keybindings: None, direct_attach_requested: true, + term_program: None, writer, })); @@ -5769,6 +5786,7 @@ next_tab = "" render_encoding: RenderEncoding::SemanticFrame, keybindings: None, direct_attach_requested: false, + term_program: None, writer, })); assert!(server.has_app_client()); @@ -5869,6 +5887,7 @@ next_tab = "" render_encoding: RenderEncoding::TerminalAnsi, keybindings: None, direct_attach_requested: true, + term_program: None, writer, })); assert!( @@ -7808,6 +7827,7 @@ next_tab = "" render_encoding: RenderEncoding::TerminalAnsi, keybindings: None, direct_attach_requested: true, + term_program: None, writer, })); assert!( @@ -8381,6 +8401,7 @@ next_tab = "" 1, RenderEncoding::SemanticFrame, false, + None, Some(client_tx), ), ); diff --git a/tests/api_ping.rs b/tests/api_ping.rs index 344fe6ecd0..1033628a92 100644 --- a/tests/api_ping.rs +++ b/tests/api_ping.rs @@ -304,7 +304,7 @@ fn ping_over_socket_returns_version() { assert_eq!(value["result"]["version"], env!("CARGO_PKG_VERSION")); // Intentionally hardcoded so wire protocol bumps require updating this test. // Changing this value means old clients/servers are no longer compatible. - assert_eq!(value["result"]["protocol"], 18); + assert_eq!(value["result"]["protocol"], 19); cleanup_spawned_herdr(child, base); } diff --git a/tests/cli/sessions.rs b/tests/cli/sessions.rs index b7655542ec..8cb0f4b799 100644 --- a/tests/cli/sessions.rs +++ b/tests/cli/sessions.rs @@ -326,7 +326,7 @@ fn status_commands_report_client_and_server_versions() { "stdout: {full_stdout}" ); assert!( - full_stdout.contains(" protocol: 18"), + full_stdout.contains(" protocol: 19"), "stdout: {full_stdout}" ); assert!(full_stdout.contains("server:\n"), "stdout: {full_stdout}"); @@ -359,7 +359,7 @@ fn status_commands_report_client_and_server_versions() { "stdout: {server_stdout}" ); assert!( - server_stdout.contains("protocol: 18"), + server_stdout.contains("protocol: 19"), "stdout: {server_stdout}" ); @@ -371,7 +371,7 @@ fn status_commands_report_client_and_server_versions() { "stdout: {client_stdout}" ); assert!( - client_stdout.contains("protocol: 18"), + client_stdout.contains("protocol: 19"), "stdout: {client_stdout}" ); assert!( @@ -381,7 +381,7 @@ fn status_commands_report_client_and_server_versions() { let full_json = run_cli_json(&socket_path, &["status", "--json"]); assert_eq!(full_json["client"]["version"], env!("CARGO_PKG_VERSION")); - assert_eq!(full_json["client"]["protocol"], 18); + assert_eq!(full_json["client"]["protocol"], 19); assert_eq!(full_json["server"]["status"], "running"); assert_eq!(full_json["server"]["running"], true); assert_eq!(full_json["server"]["compatible"], true); @@ -395,12 +395,12 @@ fn status_commands_report_client_and_server_versions() { let server_json = run_cli_json(&socket_path, &["status", "server", "--json"]); assert_eq!(server_json["status"], "running"); assert_eq!(server_json["version"], env!("CARGO_PKG_VERSION")); - assert_eq!(server_json["protocol"], 18); + assert_eq!(server_json["protocol"], 19); assert_eq!(server_json["compatible"], true); let client_json = run_cli_json(&socket_path, &["status", "client", "--json"]); assert_eq!(client_json["version"], env!("CARGO_PKG_VERSION")); - assert_eq!(client_json["protocol"], 18); + assert_eq!(client_json["protocol"], 19); assert!(client_json["binary"] .as_str() .is_some_and(|path| !path.is_empty())); diff --git a/tests/cross_area.rs b/tests/cross_area.rs index 6f1ab82b22..2a064b2939 100644 --- a/tests/cross_area.rs +++ b/tests/cross_area.rs @@ -429,6 +429,7 @@ fn client_handshake(stream: &mut UnixStream, version: u32, cols: u16, rows: u16) payload.extend_from_slice(&encode_varint_u32(0)); // RenderEncoding::SemanticFrame payload.extend_from_slice(&encode_varint_u32(0)); // ClientKeybindings::Server payload.extend_from_slice(&encode_varint_u32(0)); // ClientLaunchMode::App + payload.push(0); // term_program: Option::None stream .write_all(&frame_message(&payload)) diff --git a/tests/multi_client.rs b/tests/multi_client.rs index 6e367eaf61..8673dc5517 100644 --- a/tests/multi_client.rs +++ b/tests/multi_client.rs @@ -516,6 +516,7 @@ fn client_handshake( &encode_varint_u32(0), // RenderEncoding::SemanticFrame &encode_varint_u32(0), // ClientKeybindings::Server &encode_varint_u32(0), // ClientLaunchMode::App + &[0u8], // term_program: Option::None ], ); stream diff --git a/tests/server_headless.rs b/tests/server_headless.rs index 5abb5aceea..6b4e9373ff 100644 --- a/tests/server_headless.rs +++ b/tests/server_headless.rs @@ -181,6 +181,7 @@ fn client_handshake( &encode_varint_u32(0), // RenderEncoding::SemanticFrame &encode_varint_u32(0), // ClientKeybindings::Server &encode_varint_u32(0), // ClientLaunchMode::App + &[0u8], // term_program: Option::None ], ); let framed = frame_message(&hello_payload); diff --git a/tests/support/mod.rs b/tests/support/mod.rs index c964512991..daeaca6374 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -15,7 +15,7 @@ static INIT: Once = Once::new(); static CLEANUP_GUARD: OnceLock = OnceLock::new(); const WATCHDOG_SCAN_INTERVAL: Duration = Duration::from_secs(1); const RUNTIME_OWNER_MARKER: &str = ".herdr-test-owner-pid"; -pub const CURRENT_PROTOCOL: u32 = 18; +pub const CURRENT_PROTOCOL: u32 = 19; pub fn register_spawned_herdr_pid(pid: Option) { let Some(pid) = pid else { @@ -241,6 +241,7 @@ pub fn client_handshake( &encode_varint_u32(0), // RenderEncoding::SemanticFrame &encode_varint_u32(0), // ClientKeybindings::Server &encode_varint_u32(0), // ClientLaunchMode::App + &[0u8], // term_program: Option::None ], ); let framed = frame_message(&hello_payload); From e37c8c669a75e88b453784aa37025e101fd29d78 Mon Sep 17 00:00:00 2001 From: jvm Date: Thu, 30 Jul 2026 18:01:14 +0100 Subject: [PATCH 2/6] fix: drop stale terminal identity variables from pane environment TERM_PROGRAM is not the only identity panes inherit from the server process. A server started under kitty and later attached from a terminal without inline image support still handed panes KITTY_WINDOW_ID, so applications selected a graphics protocol the real outer terminal could not render. Remove the identity variables that no longer describe the attached client alongside TERM_PROGRAM. Applications that read them fall back to the TERM_PROGRAM the client actually reported. --- src/pane.rs | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/src/pane.rs b/src/pane.rs index e5e8566262..655b0ca63a 100644 --- a/src/pane.rs +++ b/src/pane.rs @@ -63,6 +63,17 @@ const PANE_COLORTERM: &str = "truecolor"; /// bridge. The attached client is the only authority on what actually renders pane output. static OUTER_TERM_PROGRAM: RwLock> = RwLock::new(None); +/// Stale terminal identity leaked from this process's environment. Applications read +/// these to select graphics protocols, so a value from the wrong terminal is worse than +/// no value at all. +const STALE_TERMINAL_IDENTITY_ENV: [&str; 5] = [ + "TERM_PROGRAM_VERSION", + "KITTY_WINDOW_ID", + "WEZTERM_PANE", + "ITERM_SESSION_ID", + "GHOSTTY_RESOURCES_DIR", +]; + pub(crate) fn set_outer_term_program(term_program: Option) { match OUTER_TERM_PROGRAM.write() { Ok(mut guard) => *guard = term_program, @@ -89,6 +100,9 @@ fn apply_pane_terminal_env(cmd: &mut CommandBuilder) { Some(term_program) => cmd.env("TERM_PROGRAM", term_program), None => cmd.env_remove("TERM_PROGRAM"), } + for key in STALE_TERMINAL_IDENTITY_ENV { + cmd.env_remove(key); + } } #[derive(Debug, Clone, Default, PartialEq, Eq)] @@ -3036,6 +3050,7 @@ mod tests { // Stand in for the terminal identity a persistent server inherits from whichever // terminal started it, which is not necessarily the attached client's. cmd.env("TERM_PROGRAM", "stale-outer"); + cmd.env("KITTY_WINDOW_ID", "99"); apply_pane_terminal_env(&mut cmd); for (key, value) in extra_env { cmd.env(key, value); @@ -3347,10 +3362,14 @@ mod tests { fn pane_term_program_reports_attached_client_terminal() { let _guard = outer_term_program_lock().lock(); set_outer_term_program(Some("kitty".to_string())); - let output = capture_shell_output("printf '%s\\n%s\\n' \"$TERM_PROGRAM\" \"$TERM\"", &[]); + let output = capture_shell_output( + "printf '%s\\n%s\\n%s\\n' \"$TERM_PROGRAM\" \"$TERM\" \"$KITTY_WINDOW_ID\"", + &[], + ); set_outer_term_program(None); - // TERM stays herdr's own virtual terminal; only the outer identity is propagated. - assert_eq!(output, "kitty\nxterm-256color\n"); + // TERM stays herdr's own virtual terminal; only the outer identity is propagated, + // and the server's own stale KITTY_WINDOW_ID does not leak through. + assert_eq!(output, "kitty\nxterm-256color\n\n"); } #[cfg(unix)] @@ -3358,10 +3377,13 @@ mod tests { fn pane_term_program_is_cleared_without_an_attached_client() { let _guard = outer_term_program_lock().lock(); set_outer_term_program(None); - let output = capture_shell_output("printf '%s\\n' \"$TERM_PROGRAM\"", &[]); + let output = capture_shell_output( + "printf '%s\\n%s\\n' \"$TERM_PROGRAM\" \"$KITTY_WINDOW_ID\"", + &[], + ); // A stale identity is worse than none: it makes applications pick a graphics // protocol the real outer terminal may not support. - assert_eq!(output, "\n"); + assert_eq!(output, "\n\n"); } #[cfg(unix)] From 413385d2532a9c5d68f3e6c36e33a276986a0e86 Mon Sep 17 00:00:00 2001 From: jvm Date: Thu, 30 Jul 2026 18:19:10 +0100 Subject: [PATCH 3/6] refactor: consolidate outer terminal identity and cover monolithic mode Move outer_term_program next to detect_backend in terminal_notify, which already owns terminal identification, and give the backend enum the TERM_PROGRAM spelling it maps to. The client no longer keeps its own copy of that table. Monolithic mode has no client to report an identity, so panes were losing TERM_PROGRAM with nothing set in its place. That process owns the terminal directly, so seed the identity from its own environment. Also fold the two identical no-foreground-client resets in sync_foreground_client_state into one, apply the identity without cloning it per spawn, keep a poisoned lock from stranding panes on a departed client, and extend the scrub list to the identity variables the previous set missed. LC_TERMINAL matters most: SSH forwards LC_* by default, so it reaches a remote server naming the local terminal. --- src/client/mod.rs | 60 ++--------------------------- src/main.rs | 4 ++ src/pane.rs | 59 +++++++++++++++++------------ src/server/headless.rs | 15 ++------ src/terminal_notify.rs | 85 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 133 insertions(+), 90 deletions(-) diff --git a/src/client/mod.rs b/src/client/mod.rs index dadfbfb47c..bbbc6f2bde 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -763,7 +763,10 @@ fn do_handshake( } else { ClientLaunchMode::App }, - term_program: outer_term_program(), + // The server is persistent and its own environment belongs to whichever terminal + // started it, so only the client can report this. The remote SSH bridge forwards + // no environment, making Hello the only channel that reaches a remote server. + term_program: crate::terminal_notify::outer_term_program(), }; protocol::write_message(stream, &hello) .map_err(|e| ClientError::ConnectionFailed(io::Error::other(e.to_string())))?; @@ -2105,32 +2108,6 @@ fn current_terminal_geometry(kitty_graphics_enabled: bool) -> (u16, u16, u32, u3 ) } -/// Identifies the outer terminal emulator that renders this client's output. -/// -/// The server is a persistent process whose own environment belongs to whichever -/// terminal happened to start it, so it cannot infer this. Sending it in `Hello` is -/// the only channel that survives the remote SSH bridge, which forwards no environment. -fn outer_term_program() -> Option { - if let Ok(value) = std::env::var("TERM_PROGRAM") { - if !value.is_empty() { - return Some(value); - } - } - // Kitty deliberately ships no TERM_PROGRAM, so fall back to the identity - // terminal_notify already infers from KITTY_WINDOW_ID/TERM, spelled the way - // TERM_PROGRAM-reading consumers expect. - use crate::terminal_notify::TerminalNotificationBackend as Backend; - Some( - match crate::terminal_notify::detect_backend()? { - Backend::Ghostty => "ghostty", - Backend::Iterm2 => "iTerm.app", - Backend::Kitty => "kitty", - Backend::WezTerm => "WezTerm", - } - .to_string(), - ) -} - /// Reports polled changes and signalled resizes that return to the same size. fn resize_report_required( signalled: bool, @@ -2288,35 +2265,6 @@ mod tests { ); } - const TERMINAL_IDENTITY_ENV: [&str; 3] = ["TERM_PROGRAM", "TERM", "KITTY_WINDOW_ID"]; - - #[test] - fn outer_term_program_prefers_the_terminals_own_report() { - let _guard = env_lock().lock().unwrap(); - let _cleared = EnvVarsRemovedGuard::new(&TERMINAL_IDENTITY_ENV); - let _env = EnvVarGuard::set("TERM_PROGRAM", "vscode"); - - assert_eq!(outer_term_program().as_deref(), Some("vscode")); - } - - #[test] - fn outer_term_program_infers_kitty_which_sets_no_term_program() { - let _guard = env_lock().lock().unwrap(); - let _cleared = EnvVarsRemovedGuard::new(&TERMINAL_IDENTITY_ENV); - let _env = EnvVarGuard::set("KITTY_WINDOW_ID", "3"); - - assert_eq!(outer_term_program().as_deref(), Some("kitty")); - } - - #[test] - fn outer_term_program_is_none_for_an_unidentified_terminal() { - let _guard = env_lock().lock().unwrap(); - let _cleared = EnvVarsRemovedGuard::new(&TERMINAL_IDENTITY_ENV); - let _env = EnvVarGuard::set("TERM", "xterm-256color"); - - assert_eq!(outer_term_program(), None); - } - #[test] fn host_cursor_policy_native_and_drawn_override_auto_detection() { let _guard = env_lock().lock().unwrap(); diff --git a/src/main.rs b/src/main.rs index 098693f1bc..49ab650b9e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -741,6 +741,10 @@ fn main() -> io::Result<()> { init_logging(); + // No client attaches in this mode, so nothing else reports the outer terminal. This + // process owns the terminal directly, so its own environment is the right answer. + pane::set_outer_term_program(terminal_notify::outer_term_program()); + let (api_tx, api_rx) = tokio::sync::mpsc::unbounded_channel(); let event_hub = api::EventHub::default(); let _api_server = match api::start_server_with_capabilities(api_tx, event_hub.clone(), None) { diff --git a/src/pane.rs b/src/pane.rs index 655b0ca63a..abb8b973ce 100644 --- a/src/pane.rs +++ b/src/pane.rs @@ -63,22 +63,32 @@ const PANE_COLORTERM: &str = "truecolor"; /// bridge. The attached client is the only authority on what actually renders pane output. static OUTER_TERM_PROGRAM: RwLock> = RwLock::new(None); -/// Stale terminal identity leaked from this process's environment. Applications read -/// these to select graphics protocols, so a value from the wrong terminal is worse than -/// no value at all. -const STALE_TERMINAL_IDENTITY_ENV: [&str; 5] = [ +/// Terminal identity variables a pane may inherit from this process that no longer +/// describe the attached client. Applications read these to select graphics protocols, +/// so a value from the wrong terminal is worse than no value at all. +/// +/// `LC_TERMINAL` matters most for remote attach: SSH forwards `LC_*` by default, so it +/// reaches a remote server naming the *local* terminal that started the bridge. +const STALE_TERMINAL_IDENTITY_ENV: [&str; 12] = [ "TERM_PROGRAM_VERSION", + "TERM_SESSION_ID", + "LC_TERMINAL", + "LC_TERMINAL_VERSION", "KITTY_WINDOW_ID", + "KITTY_PID", "WEZTERM_PANE", + "WEZTERM_UNIX_SOCKET", "ITERM_SESSION_ID", + "ITERM_PROFILE", "GHOSTTY_RESOURCES_DIR", + "ALACRITTY_WINDOW_ID", ]; pub(crate) fn set_outer_term_program(term_program: Option) { - match OUTER_TERM_PROGRAM.write() { - Ok(mut guard) => *guard = term_program, - Err(_) => tracing::warn!("outer terminal identity lock poisoned; panes keep last value"), - } + // A poisoned lock must not strand panes on a departed client's identity. + *OUTER_TERM_PROGRAM + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner) = term_program; } fn apply_pane_terminal_env(cmd: &mut CommandBuilder) { @@ -92,11 +102,11 @@ fn apply_pane_terminal_env(cmd: &mut CommandBuilder) { // TERM_PROGRAM names the emulator that ultimately displays this pane, which is the // client's outer terminal rather than herdr's virtual one. Applications use it to // pick an inline image protocol, so it must track the attached client. - let outer_term_program = match OUTER_TERM_PROGRAM.read() { - Ok(guard) => guard.clone(), - Err(_) => None, - }; - match outer_term_program { + match OUTER_TERM_PROGRAM + .read() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .as_deref() + { Some(term_program) => cmd.env("TERM_PROGRAM", term_program), None => cmd.env_remove("TERM_PROGRAM"), } @@ -3016,11 +3026,16 @@ mod tests { assert!(!process_alive_for_shutdown(43, 42, false, |_| false)); } - /// Serializes tests that read or write the process-wide outer terminal identity. + /// Serializes tests that read or write the process-wide outer terminal identity, and + /// resets it so each starts from a known state regardless of test order. #[cfg(unix)] - fn outer_term_program_lock() -> &'static Mutex<()> { - static LOCK: std::sync::OnceLock> = std::sync::OnceLock::new(); - LOCK.get_or_init(|| Mutex::new(())) + fn outer_term_program_lock() -> std::sync::MutexGuard<'static, ()> { + static LOCK: Mutex<()> = Mutex::new(()); + let guard = LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + set_outer_term_program(None); + guard } #[cfg(unix)] @@ -3341,7 +3356,7 @@ mod tests { #[cfg(unix)] #[test] fn pane_terminal_identity_overrides_outer_terminal_env() { - let _guard = outer_term_program_lock().lock(); + let _guard = outer_term_program_lock(); let output = capture_shell_output("printf '%s\\n%s\\n' \"$TERM\" \"$COLORTERM\"", &[]); assert_eq!(output, "xterm-256color\ntruecolor\n"); } @@ -3349,7 +3364,7 @@ mod tests { #[cfg(unix)] #[test] fn pane_terminal_identity_allows_explicit_override() { - let _guard = outer_term_program_lock().lock(); + let _guard = outer_term_program_lock(); let output = capture_shell_output( "printf '%s\\n%s\\n' \"$TERM\" \"$COLORTERM\"", &[("TERM", "vt100"), ("COLORTERM", "24bit")], @@ -3360,13 +3375,12 @@ mod tests { #[cfg(unix)] #[test] fn pane_term_program_reports_attached_client_terminal() { - let _guard = outer_term_program_lock().lock(); + let _guard = outer_term_program_lock(); set_outer_term_program(Some("kitty".to_string())); let output = capture_shell_output( "printf '%s\\n%s\\n%s\\n' \"$TERM_PROGRAM\" \"$TERM\" \"$KITTY_WINDOW_ID\"", &[], ); - set_outer_term_program(None); // TERM stays herdr's own virtual terminal; only the outer identity is propagated, // and the server's own stale KITTY_WINDOW_ID does not leak through. assert_eq!(output, "kitty\nxterm-256color\n\n"); @@ -3375,8 +3389,7 @@ mod tests { #[cfg(unix)] #[test] fn pane_term_program_is_cleared_without_an_attached_client() { - let _guard = outer_term_program_lock().lock(); - set_outer_term_program(None); + let _guard = outer_term_program_lock(); let output = capture_shell_output( "printf '%s\\n%s\\n' \"$TERM_PROGRAM\" \"$KITTY_WINDOW_ID\"", &[], diff --git a/src/server/headless.rs b/src/server/headless.rs index f997daa5e9..ca9f0039c4 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -1050,17 +1050,10 @@ impl HeadlessServer { } fn sync_foreground_client_state(&mut self) { - let Some(client_id) = self.foreground_client_id else { - self.effective_size = (MIN_COLS, MIN_ROWS); - self.app.state.outer_terminal_focus = None; - self.app.state.host_cell_size = crate::kitty_graphics::HostCellSize::default(); - crate::pane::set_outer_term_program(None); - let server_keybindings = self.server_keybindings.clone(); - apply_keybindings(&mut self.app, &server_keybindings); - self.sync_visible_server_config_diagnostic(false); - return; - }; - let Some(client) = self.clients.get(&client_id) else { + let Some(client) = self + .foreground_client_id + .and_then(|client_id| self.clients.get(&client_id)) + else { self.foreground_client_id = None; self.effective_size = (MIN_COLS, MIN_ROWS); self.app.state.outer_terminal_focus = None; diff --git a/src/terminal_notify.rs b/src/terminal_notify.rs index e6380c7b5b..8052387252 100644 --- a/src/terminal_notify.rs +++ b/src/terminal_notify.rs @@ -8,6 +8,32 @@ pub enum TerminalNotificationBackend { WezTerm, } +impl TerminalNotificationBackend { + /// The `TERM_PROGRAM` value this terminal is identified by. + /// + /// Kitty deliberately sets no `TERM_PROGRAM`; `"kitty"` is the spelling + /// TERM_PROGRAM-reading consumers check for it. + pub(crate) fn term_program(self) -> &'static str { + match self { + Self::Ghostty => "ghostty", + Self::Iterm2 => "iTerm.app", + Self::Kitty => "kitty", + Self::WezTerm => "WezTerm", + } + } +} + +/// Identifies the outer terminal emulator rendering this process's output. +/// +/// Prefers the terminal's own report and falls back to the identity inferred from +/// `TERM`/`KITTY_WINDOW_ID`, so terminals that ship no `TERM_PROGRAM` are still named. +pub(crate) fn outer_term_program() -> Option { + std::env::var("TERM_PROGRAM") + .ok() + .filter(|value| !value.is_empty()) + .or_else(|| detect_backend().map(|backend| backend.term_program().to_string())) +} + pub fn detect_backend() -> Option { let term_program = std::env::var("TERM_PROGRAM").ok(); let term = std::env::var("TERM").ok(); @@ -109,6 +135,65 @@ fn wrap_tmux_passthrough(sequence: &[u8]) -> Vec { mod tests { use super::*; + /// Terminal identity variables `outer_term_program` reads, cleared so the ambient + /// terminal running the suite cannot decide the result. + const IDENTITY_ENV: [&str; 3] = ["TERM_PROGRAM", "TERM", "KITTY_WINDOW_ID"]; + + struct IdentityEnvGuard { + previous: Vec<(&'static str, Option)>, + } + + impl IdentityEnvGuard { + fn set(pairs: &[(&'static str, &str)]) -> Self { + let previous = IDENTITY_ENV + .iter() + .map(|key| (*key, std::env::var_os(key))) + .collect(); + for key in IDENTITY_ENV { + std::env::remove_var(key); + } + for (key, value) in pairs { + std::env::set_var(key, value); + } + Self { previous } + } + } + + impl Drop for IdentityEnvGuard { + fn drop(&mut self) { + for (key, value) in &self.previous { + match value { + Some(value) => std::env::set_var(key, value), + None => std::env::remove_var(key), + } + } + } + } + + #[test] + fn outer_term_program_prefers_the_terminals_own_report() { + let _lock = crate::integration::integration_env_lock(); + let _env = IdentityEnvGuard::set(&[("TERM_PROGRAM", "vscode")]); + + assert_eq!(outer_term_program().as_deref(), Some("vscode")); + } + + #[test] + fn outer_term_program_infers_kitty_which_sets_no_term_program() { + let _lock = crate::integration::integration_env_lock(); + let _env = IdentityEnvGuard::set(&[("KITTY_WINDOW_ID", "3")]); + + assert_eq!(outer_term_program().as_deref(), Some("kitty")); + } + + #[test] + fn outer_term_program_is_none_for_an_unidentified_terminal() { + let _lock = crate::integration::integration_env_lock(); + let _env = IdentityEnvGuard::set(&[("TERM", "xterm-256color")]); + + assert_eq!(outer_term_program(), None); + } + #[test] fn split_message_splits_title_and_body() { assert_eq!( From 4c59de62d2b2778b50f63b799e27d57e31d25e10 Mon Sep 17 00:00:00 2001 From: jvm Date: Thu, 30 Jul 2026 23:10:32 +0100 Subject: [PATCH 4/6] fix: give a restarting server its terminal identity before restoring panes Panes are spawned while the app is built, which happens before any client can attach and report its terminal. Restored panes therefore had TERM_PROGRAM removed and never received one. Remote attach hits this every time: a server that has to restart restores the whole session first, so every pane the user lands in predates the client. Seed the identity at server startup from whoever launched the process. A locally spawned daemon inherits it from the client that started it. A remote server is started by the ssh bridge, which runs under `ssh -T` and so is given no pty and neither TERM_PROGRAM nor TERM; name the local terminal on that command line instead. An attaching client still overrides it. --- src/pane.rs | 16 +++++++++ src/remote/unix.rs | 75 +++++++++++++++++++++++++++++++++++------- src/server/headless.rs | 5 +++ 3 files changed, 85 insertions(+), 11 deletions(-) diff --git a/src/pane.rs b/src/pane.rs index abb8b973ce..39d54f2e77 100644 --- a/src/pane.rs +++ b/src/pane.rs @@ -84,6 +84,22 @@ const STALE_TERMINAL_IDENTITY_ENV: [&str; 12] = [ "ALACRITTY_WINDOW_ID", ]; +/// Set on a server started for a specific outer terminal that the server process itself +/// cannot see. A remote server is started over `ssh -T`, which requests no pty and so +/// forwards neither `TERM_PROGRAM` nor `TERM`. +pub(crate) const OUTER_TERM_PROGRAM_ENV_VAR: &str = "HERDR_OUTER_TERM_PROGRAM"; + +/// The outer terminal identity available when this process was launched. +/// +/// A locally spawned daemon inherits it from the client that started it, so its own +/// environment is correct at that moment; a remote server is told explicitly. +pub(crate) fn launched_outer_term_program() -> Option { + std::env::var(OUTER_TERM_PROGRAM_ENV_VAR) + .ok() + .filter(|value| !value.is_empty()) + .or_else(crate::terminal_notify::outer_term_program) +} + pub(crate) fn set_outer_term_program(term_program: Option) { // A poisoned lock must not strand panes on a departed client's identity. *OUTER_TERM_PROGRAM diff --git a/src/remote/unix.rs b/src/remote/unix.rs index 6fe927c1cc..24f384fce6 100644 --- a/src/remote/unix.rs +++ b/src/remote/unix.rs @@ -1634,8 +1634,23 @@ fn confirm_remote_install( Ok(()) } -fn remote_bridge_command(remote_herdr: &RemoteHerdr, session_name: &str) -> String { - let mut command = format!("exec {}", remote_herdr.shell_path); +fn remote_bridge_command( + remote_herdr: &RemoteHerdr, + session_name: &str, + term_program: Option<&str>, +) -> String { + let mut command = String::from("exec "); + // The bridge starts the remote server when none is running, and that server restores + // panes before any client attaches. `ssh -T` requests no pty, so it forwards neither + // TERM_PROGRAM nor TERM; naming the local terminal here is what reaches those panes. + if let Some(term_program) = term_program { + command.push_str("env "); + command.push_str(crate::pane::OUTER_TERM_PROGRAM_ENV_VAR); + command.push('='); + command.push_str(&shell_quote(term_program)); + command.push(' '); + } + command.push_str(&remote_herdr.shell_path); if session_name != crate::session::DEFAULT_SESSION_NAME { command.push_str(" --session "); command.push_str(&shell_quote(session_name)); @@ -1868,10 +1883,11 @@ fn bridge_connection( ) -> io::Result<()> { let mut command = Command::new("ssh"); apply_managed_ssh_options(&mut command, ssh_options); - command - .arg("-T") - .arg(target) - .arg(remote_bridge_command(remote_herdr, session_name)); + command.arg("-T").arg(target).arg(remote_bridge_command( + remote_herdr, + session_name, + crate::terminal_notify::outer_term_program().as_deref(), + )); command .stdin(Stdio::piped()) .stdout(Stdio::piped()) @@ -2430,11 +2446,48 @@ mod tests { arch: "x86_64", }); assert_eq!( - remote_bridge_command(&remote_herdr, crate::session::DEFAULT_SESSION_NAME), + remote_bridge_command(&remote_herdr, crate::session::DEFAULT_SESSION_NAME, None), "exec \"$HOME/.local/bin/herdr\" remote-client-bridge" ); } + #[test] + fn remote_bridge_command_carries_outer_terminal_to_the_remote_server() { + let remote_herdr = RemoteHerdr::for_platform(RemotePlatform { + os: "linux", + arch: "x86_64", + }); + // The bridge starts the remote server, which restores panes before any client + // attaches, so the identity has to arrive on this command line. + assert_eq!( + remote_bridge_command( + &remote_herdr, + crate::session::DEFAULT_SESSION_NAME, + Some("ghostty") + ), + "exec env HERDR_OUTER_TERM_PROGRAM=ghostty \"$HOME/.local/bin/herdr\" remote-client-bridge" + ); + } + + #[test] + fn remote_bridge_command_quotes_a_hostile_terminal_name() { + let remote_herdr = RemoteHerdr::for_platform(RemotePlatform { + os: "linux", + arch: "x86_64", + }); + // The value comes from the local environment and is interpolated into a remote + // shell command, so it must not be able to break out of its quoting. + assert_eq!( + remote_bridge_command( + &remote_herdr, + crate::session::DEFAULT_SESSION_NAME, + Some("x'; rm -rf /; echo '") + ), + "exec env HERDR_OUTER_TERM_PROGRAM='x'\\''; rm -rf /; echo '\\''' \ + \"$HOME/.local/bin/herdr\" remote-client-bridge" + ); + } + #[test] fn remote_path_discovery_uses_path_binary() { let remote_herdr = RemoteHerdr::for_platform(RemotePlatform { @@ -2445,7 +2498,7 @@ mod tests { .expect("path binary"); assert_eq!( - remote_bridge_command(&remote_herdr, crate::session::DEFAULT_SESSION_NAME), + remote_bridge_command(&remote_herdr, crate::session::DEFAULT_SESSION_NAME, None), "exec /usr/bin/herdr remote-client-bridge" ); } @@ -2461,7 +2514,7 @@ mod tests { .expect("path binary"); assert_eq!( - remote_bridge_command(&remote_herdr, crate::session::DEFAULT_SESSION_NAME), + remote_bridge_command(&remote_herdr, crate::session::DEFAULT_SESSION_NAME, None), "exec '/opt/herdr bin/herdr' remote-client-bridge" ); } @@ -2477,7 +2530,7 @@ mod tests { .expect("path binary"); assert_eq!( - remote_bridge_command(&remote_herdr, crate::session::DEFAULT_SESSION_NAME), + remote_bridge_command(&remote_herdr, crate::session::DEFAULT_SESSION_NAME, None), "exec /opt/homebrew/bin/herdr remote-client-bridge" ); assert_eq!(remote_herdr.platform.asset_key(), "macos-aarch64"); @@ -2564,7 +2617,7 @@ mod tests { .expect("path binary"); assert_eq!( - remote_bridge_command(&remote_herdr, crate::session::DEFAULT_SESSION_NAME), + remote_bridge_command(&remote_herdr, crate::session::DEFAULT_SESSION_NAME, None), "exec '/opt/herdr'\\''s/bin/herdr' remote-client-bridge" ); } diff --git a/src/server/headless.rs b/src/server/headless.rs index ca9f0039c4..2dce85364f 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -4373,6 +4373,11 @@ pub fn run_server() -> io::Result<()> { init_logging(); crate::platform::raise_server_nofile_limit(); + // Restored panes are spawned while building the app, before any client can report + // its terminal. Seed the identity from whoever launched this server so those panes + // are not stranded without one; an attaching client overrides it. + crate::pane::set_outer_term_program(crate::pane::launched_outer_term_program()); + let args: Vec = std::env::args().collect(); if args.get(2).map(String::as_str) == Some("--handoff-import") { let socket_path = args From 3c08fd513ab0f4ed67b5ece2ff469ec4ff52fba9 Mon Sep 17 00:00:00 2001 From: jvm Date: Thu, 30 Jul 2026 23:58:46 +0100 Subject: [PATCH 5/6] fix: keep terminal identity through headless startup and nested panes Two gaps in the identity propagation, both found in review. sync_foreground_client_state cleared the identity whenever no client was in the foreground, and it runs before every API request. The headless workspace and tab creates that spawn panes at startup have no client yet, so the launch seed was dropped before the panes it exists for were spawned. Clearing now happens in promote_latest_remaining_client, the actual detach transition. Pane children also inherited HERDR_OUTER_TERM_PROGRAM. It outranks TERM_PROGRAM in launched_outer_term_program, so a nested persistent herdr started inside a pane preferred the outer server's identity over the pane's own. It joins the stale identity variables stripped at spawn. Both tests fail without their fix. --- src/pane.rs | 54 +++++++++++++++++++++++++++++++++++------- src/server/headless.rs | 40 ++++++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/src/pane.rs b/src/pane.rs index 39d54f2e77..2f33db4794 100644 --- a/src/pane.rs +++ b/src/pane.rs @@ -69,7 +69,12 @@ static OUTER_TERM_PROGRAM: RwLock> = RwLock::new(None); /// /// `LC_TERMINAL` matters most for remote attach: SSH forwards `LC_*` by default, so it /// reaches a remote server naming the *local* terminal that started the bridge. -const STALE_TERMINAL_IDENTITY_ENV: [&str; 12] = [ +/// +/// `HERDR_OUTER_TERM_PROGRAM` is this server's own launch seed. It outranks `TERM_PROGRAM` +/// in `launched_outer_term_program`, so leaving it set would make a nested persistent herdr +/// started inside a pane prefer the outer server's identity over the one this pane carries. +const STALE_TERMINAL_IDENTITY_ENV: [&str; 13] = [ + OUTER_TERM_PROGRAM_ENV_VAR, "TERM_PROGRAM_VERSION", "TERM_SESSION_ID", "LC_TERMINAL", @@ -107,6 +112,26 @@ pub(crate) fn set_outer_term_program(term_program: Option) { .unwrap_or_else(std::sync::PoisonError::into_inner) = term_program; } +/// Serializes tests that read or write the process-wide outer terminal identity, and +/// resets it so each starts from a known state regardless of test order. +#[cfg(test)] +pub(crate) fn outer_term_program_test_lock() -> std::sync::MutexGuard<'static, ()> { + static LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + let guard = LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + set_outer_term_program(None); + guard +} + +#[cfg(test)] +pub(crate) fn outer_term_program_for_test() -> Option { + OUTER_TERM_PROGRAM + .read() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .clone() +} + fn apply_pane_terminal_env(cmd: &mut CommandBuilder) { // Each pane is rendered by herdr's own terminal layer, not the outer terminal // that launched the app. Advertising the inherited TERM leaks the host terminal @@ -3042,16 +3067,10 @@ mod tests { assert!(!process_alive_for_shutdown(43, 42, false, |_| false)); } - /// Serializes tests that read or write the process-wide outer terminal identity, and - /// resets it so each starts from a known state regardless of test order. + /// Shared with the server tests, which exercise the same process-wide identity. #[cfg(unix)] fn outer_term_program_lock() -> std::sync::MutexGuard<'static, ()> { - static LOCK: Mutex<()> = Mutex::new(()); - let guard = LOCK - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); - set_outer_term_program(None); - guard + super::outer_term_program_test_lock() } #[cfg(unix)] @@ -3082,6 +3101,8 @@ mod tests { // terminal started it, which is not necessarily the attached client's. cmd.env("TERM_PROGRAM", "stale-outer"); cmd.env("KITTY_WINDOW_ID", "99"); + // The launch seed a bridged or locally spawned server carries in its own environment. + cmd.env(OUTER_TERM_PROGRAM_ENV_VAR, "stale-launcher"); apply_pane_terminal_env(&mut cmd); for (key, value) in extra_env { cmd.env(key, value); @@ -3402,6 +3423,21 @@ mod tests { assert_eq!(output, "kitty\nxterm-256color\n\n"); } + #[cfg(unix)] + #[test] + fn pane_does_not_inherit_the_servers_launch_seed() { + let _guard = outer_term_program_lock(); + set_outer_term_program(Some("ghostty".to_string())); + let output = capture_shell_output( + &format!("printf '%s\\n%s\\n' \"$TERM_PROGRAM\" \"${OUTER_TERM_PROGRAM_ENV_VAR}\""), + &[], + ); + // launched_outer_term_program prefers the seed over TERM_PROGRAM, so a nested + // persistent herdr started in this pane would otherwise report the outer server's + // terminal instead of the client actually displaying the pane. + assert_eq!(output, "ghostty\n\n"); + } + #[cfg(unix)] #[test] fn pane_term_program_is_cleared_without_an_attached_client() { diff --git a/src/server/headless.rs b/src/server/headless.rs index 2dce85364f..811ee34408 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -1058,7 +1058,10 @@ impl HeadlessServer { self.effective_size = (MIN_COLS, MIN_ROWS); self.app.state.outer_terminal_focus = None; self.app.state.host_cell_size = crate::kitty_graphics::HostCellSize::default(); - crate::pane::set_outer_term_program(None); + // Terminal identity is deliberately not cleared here. This runs before every API + // request, including the headless workspace/tab creates that spawn panes at + // startup, which would drop the launch seed before any client can attach. + // Clearing belongs to the detach path, in promote_latest_remaining_client. let server_keybindings = self.server_keybindings.clone(); apply_keybindings(&mut self.app, &server_keybindings); self.sync_visible_server_config_diagnostic(false); @@ -1438,6 +1441,12 @@ impl HeadlessServer { let next_foreground = latest_app_client(&self.clients); let changed = next_foreground != self.foreground_client_id; self.foreground_client_id = next_foreground; + if next_foreground.is_none() { + // The last client that could describe an outer terminal is gone, so panes spawned + // from here on have no display to name. The launch seed is not restored: it + // described whoever started the server, not whoever just left. + crate::pane::set_outer_term_program(None); + } self.sync_foreground_client_state(); changed } @@ -4659,6 +4668,35 @@ mod tests { ); } + #[test] + fn startup_terminal_identity_survives_headless_requests_without_a_client() { + let _guard = crate::pane::outer_term_program_test_lock(); + let mut server = test_headless_server(); + crate::pane::set_outer_term_program(Some("ghostty".to_string())); + + // Runs before every API request, including the headless workspace and tab creates + // that spawn panes at startup, long before any client can attach. + server.sync_foreground_client_state(); + + assert_eq!( + crate::pane::outer_term_program_for_test().as_deref(), + Some("ghostty") + ); + } + + #[test] + fn detaching_the_last_client_clears_terminal_identity() { + let _guard = crate::pane::outer_term_program_test_lock(); + let mut server = test_headless_server(); + crate::pane::set_outer_term_program(Some("ghostty".to_string())); + // A client that has already been removed from the map, as on disconnect. + server.foreground_client_id = Some(1); + + assert!(server.promote_latest_remaining_client()); + + assert_eq!(crate::pane::outer_term_program_for_test(), None); + } + fn test_headless_server() -> HeadlessServer { test_headless_server_with_event_hub(api::EventHub::default()) } From 0df12e4457747d8e0f91623275a50d065b7257cd Mon Sep 17 00:00:00 2001 From: jvm Date: Fri, 31 Jul 2026 00:10:14 +0100 Subject: [PATCH 6/6] fix: reject malformed client-reported terminal identity The identity a client reports for itself becomes an environment value on every pane it spawns, and it arrives from whatever is on the other end of the socket, including a remote bridge. Only empty values were filtered. No real TERM_PROGRAM contains control characters, and an interior NUL cannot survive the conversion to a C string at spawn time, so a malformed value would have failed every pane spawn for that client. Treat it as no identity instead. --- src/server/client_transport.rs | 45 +++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/server/client_transport.rs b/src/server/client_transport.rs index d3bbbbd17a..4c00b9064f 100644 --- a/src/server/client_transport.rs +++ b/src/server/client_transport.rs @@ -355,6 +355,17 @@ pub(crate) enum ServerEvent { QuitSignal, } +/// Normalize the terminal identity a client reports for itself. +/// +/// The value becomes an environment variable on every pane this client spawns, and it +/// arrives from whatever is on the other end of the socket, including a remote bridge. +/// No real `TERM_PROGRAM` contains control characters, and an interior NUL cannot survive +/// the conversion to a C string at spawn time, so treat a malformed value as no identity +/// rather than letting it reach the spawn path. +pub(crate) fn normalize_reported_term_program(term_program: Option) -> Option { + term_program.filter(|value| !value.is_empty() && !value.contains(char::is_control)) +} + /// Clamp client-reported terminal dimensions to a minimum viable size. pub(crate) fn clamp_terminal_size(cols: u16, rows: u16) -> (u16, u16) { let clamped_cols = cols.max(MIN_CLIENT_COLS); @@ -529,7 +540,7 @@ pub(crate) fn handle_client_handshake( requested_encoding, keybindings, launch_mode == ClientLaunchMode::TerminalAttach, - term_program.filter(|value| !value.is_empty()), + normalize_reported_term_program(term_program), ) } _ => { @@ -812,6 +823,38 @@ mod tests { use interprocess::local_socket::traits::Listener as _; use std::path::PathBuf; + #[test] + fn reported_term_program_keeps_a_plausible_identity() { + assert_eq!( + normalize_reported_term_program(Some("ghostty".to_string())).as_deref(), + Some("ghostty") + ); + assert_eq!( + normalize_reported_term_program(Some("iTerm.app".to_string())).as_deref(), + Some("iTerm.app") + ); + } + + #[test] + fn reported_term_program_rejects_values_that_cannot_be_an_identity() { + assert_eq!(normalize_reported_term_program(None), None); + assert_eq!(normalize_reported_term_program(Some(String::new())), None); + // An interior NUL cannot survive the conversion to a C string at spawn time, so + // letting it through would fail every pane spawn for this client. + assert_eq!( + normalize_reported_term_program(Some("ghost\0ty".to_string())), + None + ); + assert_eq!( + normalize_reported_term_program(Some("ghost\nty".to_string())), + None + ); + assert_eq!( + normalize_reported_term_program(Some("ghost\u{1b}[31mty".to_string())), + None + ); + } + struct TestSocketPath(PathBuf); impl Drop for TestSocketPath {