fix: restore saved machine screens after reconnect (#4126)

refs #3842

2 files changed, 178 insertions, 20 deletions

diff --git a/tests/client_mode.rs b/tests/client_mode.rs
index d3205e5a..9a6bee7d 100644
--- a/tests/client_mode.rs
+++ b/tests/client_mode.rs
@@ -3,6 +3,8 @@
 #![cfg(unix)]
 
 pub mod support;
+#[path = "support/terminal_screen.rs"]
+mod terminal_screen;
 
 use std::fs;
 use std::io::{BufRead, BufReader, Read, Write};
@@ -656,20 +658,28 @@ fn output_has_mouse_teardown(output: &str) -> bool {
 /// keeps the blocking `Box<dyn Read>` (which has no timeout) off the test's
 /// main thread, so a client that never exits fails the deadline instead of
 /// hanging the whole test forever.
-type SharedOutput = std::sync::Arc<Mutex<String>>;
+#[derive(Default)]
+struct PtyOutput {
+    bytes: Vec<u8>,
+    // Keep legacy raw-string watermarks stable even across split UTF-8 reads.
+    text: String,
+}
+
+type SharedOutput = std::sync::Arc<Mutex<PtyOutput>>;
 
 fn spawn_pty_drain(mut reader: Box<dyn Read + Send>) -> SharedOutput {
-    let output: SharedOutput = std::sync::Arc::new(Mutex::new(String::new()));
+    let output: SharedOutput = std::sync::Arc::new(Mutex::new(PtyOutput::default()));
     let thread_output = output.clone();
     thread::spawn(move || {
         let mut buf = [0u8; 4096];
         loop {
             match reader.read(&mut buf) {
                 Ok(0) => break,
-                Ok(n) => thread_output
-                    .lock()
-                    .unwrap_or_else(|p| p.into_inner())
-                    .push_str(&String::from_utf8_lossy(&buf[..n])),
+                Ok(n) => {
+                    let mut captured = thread_output.lock().unwrap_or_else(|p| p.into_inner());
+                    captured.bytes.extend_from_slice(&buf[..n]);
+                    captured.text.push_str(&String::from_utf8_lossy(&buf[..n]));
+                }
                 Err(_) => break,
             }
         }
@@ -677,15 +687,33 @@ fn spawn_pty_drain(mut reader: Box<dyn Read + Send>) -> SharedOutput {
     output
 }
 
+#[test]
+fn screen_capture_preserves_split_utf8_bytes() {
+    let reader = std::io::Cursor::new(b"caf\xc3").chain(std::io::Cursor::new(b"\xa9"));
+    let output = spawn_pty_drain(Box::new(reader));
+    assert!(wait_until(
+        Duration::from_secs(2),
+        Duration::from_millis(10),
+        || {
+            let bytes = output.lock().unwrap().bytes.clone();
+            bytes == "café".as_bytes() && terminal_screen::text(&bytes, 80, 24).contains("café")
+        }
+    ));
+}
+
 fn read_output(output: &SharedOutput) -> String {
-    output.lock().unwrap_or_else(|p| p.into_inner()).clone()
+    output
+        .lock()
+        .unwrap_or_else(|p| p.into_inner())
+        .text
+        .clone()
 }
 
 /// Current captured byte length, used as a watermark so a test can search only
 /// the output emitted *after* a trigger. The teardown markers also appear in
 /// normal attach-phase output, so matching the whole buffer is meaningless.
 fn output_len(output: &SharedOutput) -> usize {
-    output.lock().unwrap_or_else(|p| p.into_inner()).len()
+    output.lock().unwrap_or_else(|p| p.into_inner()).text.len()
 }
 
 /// Spawns a server + real thin client under a PTY and waits until the client
@@ -890,9 +918,10 @@ fn federated_client_starts_without_local_and_survives_its_restart() {
     let quote =
         |path: &std::path::Path| format!("'{}'", path.display().to_string().replace('\'', "'\\''"));
     let ssh_commands = base.join("ssh-commands");
+    let bridge_pid = base.join("bridge-pid");
     fs::write(bin.join("ssh"), format!(
-        "#!/bin/sh\nexport HOME={} XDG_CONFIG_HOME={} XDG_RUNTIME_DIR={} HERDR_SOCKET_PATH={}\nunset HERDR_CLIENT_SOCKET_PATH HERDR_SESSION\nfor arg do last=\"$arg\"; done\nprintf '%s\\n' \"$last\" >> {}\nexec /bin/sh -c \"$last\"\n",
-        quote(&base.join("home")), quote(&remote_config), quote(&remote_runtime), quote(&remote_api), quote(&ssh_commands),
+        "#!/bin/sh\nexport HOME={} XDG_CONFIG_HOME={} XDG_RUNTIME_DIR={} HERDR_SOCKET_PATH={}\nunset HERDR_CLIENT_SOCKET_PATH HERDR_SESSION\nfor arg do last=\"$arg\"; done\nprintf '%s\\n' \"$last\" >> {}\ncase \"$last\" in *remote-client-bridge*) printf '%s\\n' \"$$\" > {};; esac\nexec /bin/sh -c \"$last\"\n",
+        quote(&base.join("home")), quote(&remote_config), quote(&remote_runtime), quote(&remote_api), quote(&ssh_commands), quote(&bridge_pid),
     )).unwrap();
     fs::set_permissions(bin.join("ssh"), fs::Permissions::from_mode(0o700)).unwrap();
     let path = format!(
@@ -908,9 +937,17 @@ fn federated_client_starts_without_local_and_survives_its_restart() {
         &[("PATH", &path)],
     );
     let output = spawn_pty_drain(client._master.as_ref().unwrap().try_clone_reader().unwrap());
+    let screen_text = || {
+        let bytes = output
+            .lock()
+            .unwrap_or_else(|p| p.into_inner())
+            .bytes
+            .clone();
+        terminal_screen::text(&bytes, 80, 24)
+    };
     assert!(
         wait_until(Duration::from_secs(12), Duration::from_millis(20), || {
-            read_output(&output).contains("REMOTE_INITIAL_FRAME")
+            screen_text().contains("REMOTE_INITIAL_FRAME")
         }),
         "remote must be usable before Local exists: {}",
         read_output(&output)
@@ -922,6 +959,39 @@ fn federated_client_starts_without_local_and_survives_its_restart() {
         "saved machine discovery must opt into the advertised bridge idle timeout"
     );
 
+    let mut input = client._master.as_ref().unwrap().take_writer().unwrap();
+    send_pane_shell_command(&remote_api, remote_pane, "reconnect_survivor=ALIVE");
+    for cycle in 1..=3 {
+        let pid: libc::pid_t = fs::read_to_string(&bridge_pid)
+            .unwrap()
+            .trim()
+            .parse()
+            .unwrap();
+        assert_eq!(unsafe { libc::kill(pid, libc::SIGTERM) }, 0);
+        let marker = format!("REMOTE_RECONNECTED_{cycle}");
+        send_pane_shell_command(&remote_api, remote_pane, &format!("printf '{marker}\\n'"));
+        assert!(
+            wait_until(Duration::from_secs(15), Duration::from_millis(20), || {
+                screen_text().contains(&marker)
+            }),
+            "remote reconnect {cycle} must restore the visible screen without switching machines"
+        );
+        assert!(
+            wait_until(Duration::from_secs(8), Duration::from_millis(100), || {
+                if screen_text().contains(&format!("REMOTE_ALIVE_INPUT_{cycle}")) {
+                    return true;
+                }
+                write!(
+                    input,
+                    "printf 'REMOTE_%s_INPUT_{cycle}\\n' \"$reconnect_survivor\"\r"
+                )
+                .unwrap();
+                false
+            }),
+            "remote reconnect {cycle} must restore visible input and preserve the shell"
+        );
+    }
+
     let mut local = spawn_server(&config_home, &runtime_dir, &api_socket, &client_socket);
     wait_for_socket(&api_socket, Duration::from_secs(10));
     let created = send_json_request(
@@ -936,22 +1006,21 @@ fn federated_client_starts_without_local_and_survives_its_restart() {
     assert!(wait_until(
         Duration::from_secs(10),
         Duration::from_millis(20),
-        || read_output(&output).contains("local-online")
+        || screen_text().contains("local-online")
     ));
 
     local.child.kill().unwrap();
     local.close_master();
     drop(local);
-    let watermark = output_len(&output);
-    let mut input = client._master.as_ref().unwrap().take_writer().unwrap();
     input
         .write_all(b"printf 'REMOTE_%s\\n' SURVIVED\r")
         .unwrap();
     assert!(
         wait_until(Duration::from_secs(8), Duration::from_millis(20), || {
-            read_output(&output)[watermark..].contains("REMOTE_SURVIVED")
+            screen_text().contains("REMOTE_SURVIVED")
         }),
-        "Local loss must not interrupt remote input or output"
+        "Local loss must not interrupt remote input or output: {}",
+        screen_text()
     );
     assert!(client.child.try_wait().unwrap().is_none());
 
@@ -968,19 +1037,19 @@ fn federated_client_starts_without_local_and_survives_its_restart() {
     assert_eq!(created["result"]["type"], "workspace_created");
     assert!(
         wait_until(Duration::from_secs(12), Duration::from_millis(20), || {
-            read_output(&output).contains("local-returned")
+            screen_text().contains("local-returned")
         }),
         "Local must reconnect with fresh metadata"
     );
-    let watermark = output_len(&output);
     input
         .write_all(b"printf 'REMOTE_%s\\n' STILL_SELECTED\r")
         .unwrap();
     assert!(
         wait_until(Duration::from_secs(8), Duration::from_millis(20), || {
-            read_output(&output)[watermark..].contains("REMOTE_STILL_SELECTED")
+            screen_text().contains("REMOTE_STILL_SELECTED")
         }),
-        "Local recovery must not steal selection"
+        "Local recovery must not steal selection: {}",
+        screen_text()
     );
     let watermark = output_len(&output);
     remote_server.child.kill().unwrap();
diff --git a/tests/support/terminal_screen.rs b/tests/support/terminal_screen.rs
new file mode 100644
index 00000000..45d7731b
--- /dev/null
+++ b/tests/support/terminal_screen.rs
@@ -0,0 +1,89 @@
+use std::{mem::size_of, ptr};
+
+// Reuse the generated C API; this test observer needs only terminal parsing and formatting.
+#[allow(
+    dead_code,
+    non_camel_case_types,
+    non_snake_case,
+    non_upper_case_globals
+)]
+#[path = "../../src/ghostty/bindings.rs"]
+mod ffi;
+
+struct Screen {
+    terminal: ffi::GhosttyTerminal,
+    formatter: ffi::GhosttyFormatter,
+}
+
+impl Drop for Screen {
+    fn drop(&mut self) {
+        unsafe {
+            ffi::ghostty_formatter_free(self.formatter);
+            ffi::ghostty_terminal_free(self.terminal);
+        }
+    }
+}
+
+pub fn text(output: &[u8], cols: u16, rows: u16) -> String {
+    let mut screen = Screen {
+        terminal: ptr::null_mut(),
+        formatter: ptr::null_mut(),
+    };
+    let options = ffi::GhosttyFormatterTerminalOptions {
+        size: size_of::<ffi::GhosttyFormatterTerminalOptions>(),
+        emit: ffi::GhosttyFormatterFormat_GHOSTTY_FORMATTER_FORMAT_PLAIN,
+        trim: true,
+        extra: ffi::GhosttyFormatterTerminalExtra {
+            size: size_of::<ffi::GhosttyFormatterTerminalExtra>(),
+            screen: ffi::GhosttyFormatterScreenExtra {
+                size: size_of::<ffi::GhosttyFormatterScreenExtra>(),
+                ..Default::default()
+            },
+            ..Default::default()
+        },
+        ..Default::default()
+    };
+    unsafe {
+        assert_eq!(
+            ffi::ghostty_terminal_new(ptr::null(), &mut screen.terminal, cols, rows),
+            ffi::GhosttyResult_GHOSTTY_SUCCESS
+        );
+        ffi::ghostty_terminal_vt_write(screen.terminal, output.as_ptr(), output.len());
+        assert_eq!(
+            ffi::ghostty_formatter_terminal_new(
+                ptr::null(),
+                &mut screen.formatter,
+                screen.terminal,
+                options,
+            ),
+            ffi::GhosttyResult_GHOSTTY_SUCCESS
+        );
+        let mut len = 0;
+        let result =
+            ffi::ghostty_formatter_format_buf(screen.formatter, ptr::null_mut(), 0, &mut len);
+        assert!(matches!(
+            result,
+            ffi::GhosttyResult_GHOSTTY_SUCCESS | ffi::GhosttyResult_GHOSTTY_OUT_OF_SPACE
+        ));
+        let mut bytes = vec![0; len];
+        assert_eq!(
+            ffi::ghostty_formatter_format_buf(
+                screen.formatter,
+                bytes.as_mut_ptr(),
+                bytes.len(),
+                &mut len
+            ),
+            ffi::GhosttyResult_GHOSTTY_SUCCESS
+        );
+        String::from_utf8_lossy(&bytes[..len]).into_owned()
+    }
+}
+
+#[test]
+fn screen_text_reconstructs_partial_redraws() {
+    let output = b"REMOTE_SURVIVED\x1b[1;8HSTILL_SELECTED";
+    assert!(!output
+        .windows(b"REMOTE_STILL_SELECTED".len())
+        .any(|part| part == b"REMOTE_STILL_SELECTED"));
+    assert!(text(output, 80, 24).contains("REMOTE_STILL_SELECTED"));
+}