diff --git a/docs/next/CHANGELOG.md b/docs/next/CHANGELOG.md index cf3531e8ee..b2e5ec58e4 100644 --- a/docs/next/CHANGELOG.md +++ b/docs/next/CHANGELOG.md @@ -22,6 +22,7 @@ ### Fixed - Qwen Code panes now use locale-independent terminal-title states and localized confirmation fallbacks, preventing active or blocked turns from appearing idle. (#2756) +- Closing a terminal running `herdr --remote` no longer produces a local client core dump while the remote session stays alive. (#2424) - Active Space and Agent rows now use dedicated theme colors that remain visible when the host terminal background matches the selected Herdr theme. (#2792) - `agent prompt` now rejects agents already waiting at approval or question dialogs with `agent_blocked`, without sending text or Enter. (#2788) - `prefix+e` now preserves logical lines when opening soft-wrapped scrollback in an editor. (#2733) diff --git a/src/client/mod.rs b/src/client/mod.rs index cba735fa21..b70b6c9c79 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -421,6 +421,7 @@ fn setup_terminal_with_capabilities( Ok(TerminalGuard { reset_modify_other_keys: modify_other_keys_mode.is_some(), reset_host_color_scheme_reports: host_color_scheme_reports, + restored: false, #[cfg(windows)] restore_windows_input_mode: windows_virtual_terminal_input.restore_mode, }) @@ -434,6 +435,7 @@ fn should_enable_host_color_scheme_reports(enable_client_protocols: bool) -> boo struct TerminalGuard { reset_modify_other_keys: bool, reset_host_color_scheme_reports: bool, + restored: bool, #[cfg(windows)] restore_windows_input_mode: Option, } @@ -582,7 +584,7 @@ fn restore_terminal_state( reset_modify_other_keys: bool, reset_host_color_scheme_reports: bool, #[cfg(windows)] restore_windows_input_mode: Option, -) { +) -> io::Result<()> { let _ = clear_received_kitty_graphics(&mut io::stdout()); // Reset modifyOtherKeys if we enabled it. @@ -606,13 +608,16 @@ fn restore_terminal_state( restore_windows_input_mode_value(mode); } - let _ = ratatui::try_restore(); - let _ = write_terminal_restore_postlude(&mut io::stdout(), reset_host_color_scheme_reports); + let restore_result = ratatui::try_restore(); + let postlude_result = + write_terminal_restore_postlude(&mut io::stdout(), reset_host_color_scheme_reports); #[cfg(windows)] if windows_vti_input_backend_enabled() && windows_win32_input_mode_enabled() { let _ = disable_windows_win32_input_mode(&mut io::stdout()); } + + restore_result.and(postlude_result) } #[cfg(not(windows))] @@ -657,14 +662,28 @@ fn disable_windows_win32_input_mode(writer: &mut impl std::io::Write) -> io::Res writer.flush() } -impl Drop for TerminalGuard { - fn drop(&mut self) { +impl TerminalGuard { + fn restore(mut self) -> io::Result<()> { + self.restored = true; restore_terminal_state( self.reset_modify_other_keys, self.reset_host_color_scheme_reports, #[cfg(windows)] self.restore_windows_input_mode, - ); + ) + } +} + +impl Drop for TerminalGuard { + fn drop(&mut self) { + if !self.restored { + let _ = restore_terminal_state( + self.reset_modify_other_keys, + self.reset_host_color_scheme_reports, + #[cfg(windows)] + self.restore_windows_input_mode, + ); + } } } @@ -1281,7 +1300,7 @@ fn run_client_with_mode( let panic_restore_windows_input_mode = terminal_guard.restore_windows_input_mode; let original_hook = std::panic::take_hook(); std::panic::set_hook(Box::new(move |info| { - restore_terminal_state( + let _ = restore_terminal_state( panic_resets_modify_other_keys, panic_resets_host_color_scheme_reports, #[cfg(windows)] @@ -1323,19 +1342,22 @@ fn run_client_with_mode( }); // Restore the terminal before printing any final status message. - drop(terminal_guard); + let terminal_restore_failed = terminal_guard.restore().is_err(); if let Err(err) = result { - eprintln!("herdr: {err}"); + let _ = writeln!(io::stderr(), "herdr: {err}"); rt.shutdown_timeout(Duration::from_millis(100)); crate::logging::shutdown("client"); - if matches!( - err, + let detached = matches!( + &err, ClientError::ServerShutdown { reason: Some(reason) } if reason == "detached" - ) { + ); + let connection_lost_during_terminal_hangup = + terminal_restore_failed && matches!(&err, ClientError::ConnectionLost(_)); + if detached || connection_lost_during_terminal_hangup { return Ok(()); } diff --git a/tests/client_mode.rs b/tests/client_mode.rs index e79375cfaa..855fc31803 100644 --- a/tests/client_mode.rs +++ b/tests/client_mode.rs @@ -1075,6 +1075,65 @@ fn read_until_client_attaches(client: &SpawnedHerdr) -> String { panic!("thin client must attach and render a frame; output: {output:?}"); } +#[test] +fn client_exits_cleanly_when_terminal_and_transport_hang_up() { + let _lock = test_lock(); + let base = unique_test_dir(); + let config_home = base.join("config"); + let runtime_dir = base.join("runtime"); + let api_socket = runtime_dir.join("herdr.sock"); + let client_socket = runtime_dir.join("herdr-client.sock"); + + let mut spawned_server = spawn_server(&config_home, &runtime_dir, &api_socket, &client_socket); + wait_for_socket(&api_socket, Duration::from_secs(10)); + wait_for_socket(&client_socket, Duration::from_secs(10)); + + let mut thin_client = spawn_client_process(&config_home, &runtime_dir, &api_socket); + read_until_client_attaches(&thin_client); + + // Freeze the client so the dead terminal and transport EOF are both + // observable when it resumes, making the `--remote` shutdown race deterministic. + let client_pid = thin_client.child.process_id().expect("thin client pid") as libc::pid_t; + assert_eq!( + unsafe { libc::kill(client_pid, libc::SIGSTOP) }, + 0, + "stop thin client" + ); + let server_pid = spawned_server.child.process_id().expect("server pid") as libc::pid_t; + assert_eq!( + unsafe { libc::kill(server_pid, libc::SIGKILL) }, + 0, + "kill server transport" + ); + spawned_server.close_master(); + thin_client.close_master(); + assert_eq!( + unsafe { libc::kill(client_pid, libc::SIGCONT) }, + 0, + "resume thin client" + ); + + let deadline = Instant::now() + Duration::from_secs(12); + let status = loop { + if let Some(status) = thin_client.child.try_wait().expect("poll thin client") { + break Some(status); + } + if Instant::now() >= deadline { + break None; + } + thread::sleep(Duration::from_millis(20)); + }; + + drop(spawned_server); + cleanup_spawned_herdr(thin_client, base); + + let status = status.expect("thin client should exit after terminal and transport hang up"); + assert!( + status.success(), + "thin client should exit cleanly after terminal and transport hang up, got {status}" + ); +} + #[test] fn client_exits_cleanly_when_terminal_hangs_up() { let _lock = test_lock();