From fcc21c7aefaa36b8490dba7cb2f42209c9d742c5 Mon Sep 17 00:00:00 2001 From: DuProcess <273172371+DuProcess@users.noreply.github.com> Date: Sun, 21 Jun 2026 10:51:12 -0400 Subject: [PATCH] Make restart reconnect persistent --- install.sh | 1 + packaging/systemd/dosh-server.service | 1 + src/bin/dosh-client.rs | 54 ++++++++++------------ src/bin/dosh-server.rs | 36 +++++++++------ src/config.rs | 15 ++---- tests/integration_smoke.rs | 66 +++++++++++++++++++++++++++ 6 files changed, 118 insertions(+), 55 deletions(-) diff --git a/install.sh b/install.sh index c083756..403cf01 100755 --- a/install.sh +++ b/install.sh @@ -432,6 +432,7 @@ native_auth_rate_limit_per_minute = 30 allow_tcp_forwarding = true allow_remote_forwarding = false allow_agent_forwarding = false +persist_sessions = true EOF fi diff --git a/packaging/systemd/dosh-server.service b/packaging/systemd/dosh-server.service index 8fc83bd..5a1c660 100644 --- a/packaging/systemd/dosh-server.service +++ b/packaging/systemd/dosh-server.service @@ -8,6 +8,7 @@ Type=simple ExecStart=%h/.local/bin/dosh-server serve Restart=on-failure RestartSec=1 +KillMode=process NoNewPrivileges=true PrivateTmp=true ProtectSystem=strict diff --git a/src/bin/dosh-client.rs b/src/bin/dosh-client.rs index efa5fee..daacd58 100644 --- a/src/bin/dosh-client.rs +++ b/src/bin/dosh-client.rs @@ -691,7 +691,7 @@ async fn run_selftest_command(config: &dosh::config::ClientConfig, args: &Args) let overlay = render_status_overlay("[dosh] reconnecting — last contact 3s ago", 24); ensure_tui_safe_status_overlay(&overlay)?; - println!("[ok] disconnect UI: save/restore bottom-row overlay"); + println!("[ok] disconnect UI: save/restore top status bar"); parse_local_forward("18080:127.0.0.1:80")?; parse_remote_forward("19090:127.0.0.1:5432")?; @@ -1659,9 +1659,9 @@ fn select_session(requested: Option<&str>, _force_new: bool) -> String { if let Some(session) = requested { return session.to_string(); } - // Implicit, ephemeral session: the server recognizes this name shape - // (`protocol::is_implicit_session_name`) and does NOT persist it across - // restarts, since you can never reattach to a random name. + // Implicit session: this gives plain `dosh host` a fresh terminal by + // default. The server can still persist it across restarts because the + // reconnect ticket cache keeps the generated name. protocol::generate_implicit_session_name() } @@ -4735,16 +4735,13 @@ fn render_frame(frame: &Frame) -> Result<()> { /// idle period (the run loop only pings after 2s of quiet) never flashes it. const DISCONNECT_STATUS_THRESHOLD_SECS: u64 = 2; -/// Non-destructive, mosh-style disconnect status line. +/// Non-destructive, mosh-style disconnect status bar. /// -/// When server packets stop arriving we paint a single, unobtrusive line on the -/// terminal's *bottom row* — e.g. reverse-video cyan `[dosh] last contact 3s ago -/// — reconnecting…` — using save/restore cursor (ESC 7 / ESC 8) so the real -/// application cursor never moves and full-screen TUIs are not corrupted. (The -/// old in-band overlay wrote over the active line and corrupted TUIs; this draws -/// only the last row, parks the cursor there, then restores, touching nothing -/// the app owns.) The line is refreshed on the status timer tick and cleared the -/// instant a packet resumes. +/// When server packets stop arriving we paint one blue line on the terminal's +/// top row — e.g. `[dosh] reconnecting — last contact 3s ago` — using +/// save/restore cursor (ESC 7 / ESC 8) so the real application cursor never +/// moves and full-screen TUIs are not corrupted. The bar is refreshed on the +/// status timer tick and cleared the instant a packet resumes. /// /// The decision logic and the rendered text are pure and unit-tested; only /// [`DisconnectStatus::emit`] / [`DisconnectStatus::emit_clear`] touch stdout. @@ -4763,7 +4760,7 @@ struct DisconnectStatus { enum StatusAction { /// Leave the screen as-is (already correct). None, - /// Paint/repaint the status line with this exact text on the bottom row. + /// Paint/repaint the status bar with this exact text on the top row. Show(String), /// Erase the status line (link recovered or feature disabled). Clear, @@ -4868,26 +4865,25 @@ impl DisconnectStatus { } } -/// Pure: build the escape sequence that paints `text` on `rows`-tall terminal's -/// bottom row with reverse-video cyan, leaving the cursor where it was. -fn render_status_overlay(text: &str, rows: u16) -> Vec { +/// Pure: build the escape sequence that paints `text` on a top blue status bar, +/// leaving the cursor where it was. +fn render_status_overlay(text: &str, _rows: u16) -> Vec { let mut out = Vec::with_capacity(text.len() + 24); out.extend_from_slice(b"\x1b7"); // save cursor - // Move to bottom row, column 1. - out.extend_from_slice(format!("\x1b[{};1H", rows.max(1)).as_bytes()); + out.extend_from_slice(b"\x1b[1;1H"); // move to top row, column 1 out.extend_from_slice(b"\x1b[2K"); // clear entire row - out.extend_from_slice(b"\x1b[7;36m"); // reverse video + cyan + out.extend_from_slice(b"\x1b[44;37m"); // blue background + white text out.extend_from_slice(text.as_bytes()); out.extend_from_slice(b"\x1b[0m"); // reset attributes out.extend_from_slice(b"\x1b8"); // restore cursor out } -/// Pure: build the escape sequence that clears the bottom-row status line. -fn render_status_clear(rows: u16) -> Vec { +/// Pure: build the escape sequence that clears the top status bar. +fn render_status_clear(_rows: u16) -> Vec { let mut out = Vec::with_capacity(12); out.extend_from_slice(b"\x1b7"); - out.extend_from_slice(format!("\x1b[{};1H", rows.max(1)).as_bytes()); + out.extend_from_slice(b"\x1b[1;1H"); out.extend_from_slice(b"\x1b[2K"); out.extend_from_slice(b"\x1b8"); out @@ -5993,19 +5989,19 @@ mod tests { } /// The rendered overlay must be non-destructive: save cursor (ESC 7), park on - /// the bottom row, paint reverse-video cyan, then restore cursor (ESC 8) so + /// the top row, paint the blue reconnect bar, then restore cursor (ESC 8) so /// the application's cursor and full-screen TUIs are never disturbed. #[test] - fn disconnect_status_overlay_is_save_restore_bottom_row() { + fn disconnect_status_overlay_is_save_restore_top_blue_bar() { let bytes = render_status_overlay("[dosh] reconnecting — last contact 3s ago", 24); ensure_tui_safe_status_overlay(&bytes).unwrap(); assert!(bytes.starts_with(b"\x1b7"), "must save cursor first"); assert!(bytes.ends_with(b"\x1b8"), "must restore cursor last"); - // Positions to the last row (row 24), column 1. - assert!(bytes.windows(7).any(|w| w == b"\x1b[24;1H")); - // Clears the row and uses reverse-video (7) + cyan (36). + // Positions to the first row, column 1. + assert!(bytes.windows(6).any(|w| w == b"\x1b[1;1H")); + // Clears the row and uses a blue background with white text. assert!(bytes.windows(4).any(|w| w == b"\x1b[2K")); - assert!(bytes.windows(7).any(|w| w == b"\x1b[7;36m")); + assert!(bytes.windows(8).any(|w| w == b"\x1b[44;37m")); // Resets attributes before restoring the cursor. assert!(bytes.windows(4).any(|w| w == b"\x1b[0m")); diff --git a/src/bin/dosh-server.rs b/src/bin/dosh-server.rs index 7d44a7e..54316b2 100644 --- a/src/bin/dosh-server.rs +++ b/src/bin/dosh-server.rs @@ -203,21 +203,31 @@ async fn serve(config_path: Option) -> Result<()> { let retransmit_state = Arc::clone(&state); let retransmit_socket = Arc::clone(&socket); tokio::spawn(async move { - let mut interval = tokio::time::interval(Duration::from_millis(20)); + let mut interval = tokio::time::interval(Duration::from_millis(100)); loop { interval.tick().await; if let Err(err) = retransmit_pending(&retransmit_state, &retransmit_socket).await { eprintln!("retransmit error: {err:#}"); } - if let Err(err) = flush_paced_snapshots(&retransmit_state, &retransmit_socket).await { - eprintln!("paced snapshot error: {err:#}"); - } if let Err(err) = maybe_rekey_clients(&retransmit_state, &retransmit_socket).await { eprintln!("rekey error: {err:#}"); } } }); + let pacing_state = Arc::clone(&state); + let pacing_socket = Arc::clone(&socket); + let pacing_interval_ms = config.output_frame_interval_ms.max(1); + tokio::spawn(async move { + let mut interval = tokio::time::interval(Duration::from_millis(pacing_interval_ms)); + loop { + interval.tick().await; + if let Err(err) = flush_paced_snapshots(&pacing_state, &pacing_socket).await { + eprintln!("paced snapshot error: {err:#}"); + } + } + }); + let cleanup_state = Arc::clone(&state); tokio::spawn(async move { let mut interval = tokio::time::interval(Duration::from_secs(5)); @@ -491,16 +501,12 @@ impl ServerState { /// Whether a session named `name` should be backed by a persistent holder. /// - /// Persistence is only worthwhile for sessions a user can actually reattach - /// to by name: prewarmed sessions and explicitly-named ones (`--session`). - /// Implicit `term--` names (from `dosh host` with no session) - /// are ephemeral — persisting them just leaves unreattachable holders - /// lingering across restarts, so they use the in-process (dies-on-restart) - /// shell that reaps normally on disconnect. - fn should_persist_session(&self, name: &str) -> bool { + /// With persistence enabled, every PTY session gets a detached holder. That + /// includes implicit `term--` sessions: a reconnecting client has + /// the exact session name in its ticket cache, so it can reattach after a + /// quick server restart without forcing users to name sessions manually. + fn should_persist_session(&self, _name: &str) -> bool { self.config.persist_sessions - && (self.config.prewarm_sessions.iter().any(|s| s == name) - || !protocol::is_implicit_session_name(name)) } /// Spawn (or, in the persistent case, spawn-and-adopt) a PTY for a session. @@ -3209,7 +3215,7 @@ mod tests { use super::*; #[test] - fn persists_named_and_prewarmed_but_not_implicit_sessions() { + fn persists_all_sessions_when_enabled() { let (pty_tx, _rx) = mpsc::unbounded_channel(); let config = ServerConfig { persist_sessions: true, @@ -3219,7 +3225,7 @@ mod tests { let state = ServerState::new(config, [0u8; 32], pty_tx); assert!(state.should_persist_session("default")); // prewarmed assert!(state.should_persist_session("work")); // explicitly named - assert!(!state.should_persist_session("term-1781470634216-76685")); // implicit + assert!(state.should_persist_session("term-1781470634216-76685")); // implicit reconnect } #[test] diff --git a/src/config.rs b/src/config.rs index c32f421..75b485c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -48,16 +48,9 @@ pub struct ServerConfig { pub allow_agent_forwarding: bool, #[serde(default = "default_accept_env")] pub accept_env: Vec, - /// Run each terminal session's shell under a small per-session *holder* - /// process so it survives a `dosh-server` restart (crash/upgrade/ - /// `systemctl restart`). On startup the server re-adopts live holders and - /// reattaching clients land on the same shell with screen state intact. When - /// `false`, sessions behave exactly as before: the shell is a child of the - /// server and dies with it. - // Opt-in for now: this changes the core session model (per-session holder - // processes + fd passing), so it stays OFF by default until it has been - // stress-tested on a real host. Set `persist_sessions = true` to enable. - #[serde(default)] + /// Run terminal sessions under per-session holder processes so shells + /// survive a quick `dosh-server` restart and clients can reconnect. + #[serde(default = "default_true")] pub persist_sessions: bool, } @@ -90,7 +83,7 @@ impl Default for ServerConfig { allow_remote_non_loopback_bind: false, allow_agent_forwarding: false, accept_env: default_accept_env(), - persist_sessions: false, + persist_sessions: true, } } } diff --git a/tests/integration_smoke.rs b/tests/integration_smoke.rs index 673de67..908896f 100644 --- a/tests/integration_smoke.rs +++ b/tests/integration_smoke.rs @@ -2338,6 +2338,72 @@ fn session_survives_server_restart_same_shell_and_screen() { ); } +#[test] +fn implicit_session_survives_server_restart_for_same_ticket_holder() { + let dir = tempfile::tempdir().unwrap(); + let sessions_dir = dir.path().join("sessions"); + let port = free_udp_port(); + let config = write_persistent_server_config(&dir, port); + let session = protocol::generate_implicit_session_name(); + + let mut server = start_server(&dir, &config); + let (socket, bootstrap, ok) = direct_attach_session(&config, port, "read-write", &session); + let key = bootstrap.session_key; + type_line( + &socket, + port, + &ok, + &key, + 2, + "export IMPLICIT_MARK=implicit_restart_42\n", + ); + let _ = collect_frame_text(&socket, &key, 1000); + thread::sleep(Duration::from_secs(3)); + + let shell_pid_before = holder_shell_pid(&sessions_dir, &session); + assert!( + shell_pid_before.is_some(), + "implicit sessions should get a persistent holder when persistence is enabled" + ); + + let _ = server.kill(); + let _ = server.wait(); + thread::sleep(Duration::from_millis(300)); + + let pid = shell_pid_before.unwrap(); + assert!( + unsafe { libc::kill(pid, 0) } == 0, + "implicit session shell pid {pid} must survive server restart" + ); + + let mut server = start_server(&dir, &config); + let (socket2, bootstrap2, ok2) = direct_attach_session(&config, port, "read-write", &session); + let key2 = bootstrap2.session_key; + type_line( + &socket2, + port, + &ok2, + &key2, + 2, + "printf 'IMPLICIT_MARK=%s\\n' \"$IMPLICIT_MARK\"\n", + ); + let post = collect_frame_text(&socket2, &key2, 2000); + let shell_pid_after = holder_shell_pid(&sessions_dir, &session); + + let _ = server.kill(); + let _ = server.wait(); + kill_holder(&sessions_dir, &session); + + assert_eq!( + shell_pid_after, shell_pid_before, + "implicit session should re-adopt the same shell pid" + ); + assert!( + post.contains("IMPLICIT_MARK=implicit_restart_42"), + "implicit session state must survive restart, got {post:?}" + ); +} + #[test] fn multiple_persistent_named_sessions_survive_restart_independently() { let dir = tempfile::tempdir().unwrap();