From e4d7b79ed553eb4d5507d6f8976a07c2cfcd83eb Mon Sep 17 00:00:00 2001 From: Ethan Pailes Date: Tue, 10 Feb 2026 18:35:05 +0000 Subject: [PATCH 1/2] feat: dynamically load session restore mode This patch updates shpool to dynamically load the session restore mode from the config. I just noticed this TODO while prepping for integrating shpool-vterm and figured it was pretty easy to do. Fixes #173 --- libshpool/src/daemon/server.rs | 2 - libshpool/src/daemon/shell.rs | 12 +--- .../mod.rs => session_restore.rs} | 5 +- shpool/tests/attach.rs | 69 +++++++++++++++++++ shpool/tests/data/dynamic_restore.toml.tmpl | 9 +++ 5 files changed, 84 insertions(+), 13 deletions(-) rename libshpool/src/{session_restore/mod.rs => session_restore.rs} (96%) create mode 100644 shpool/tests/data/dynamic_restore.toml.tmpl diff --git a/libshpool/src/daemon/server.rs b/libshpool/src/daemon/server.rs index b60bff15..09e5225c 100644 --- a/libshpool/src/daemon/server.rs +++ b/libshpool/src/daemon/server.rs @@ -977,8 +977,6 @@ impl Server { (None, Some(config::SessionRestoreMode::Lines(l))) => *l as usize, (None, _) => DEFAULT_OUTPUT_SPOOL_LINES, }, - session_restore_mode: - self.config.get().session_restore_mode.clone().unwrap_or_default(), client_connection: client_connection_rx, client_connection_ack: client_connection_ack_tx, tty_size_change: tty_size_change_rx, diff --git a/libshpool/src/daemon/shell.rs b/libshpool/src/daemon/shell.rs index 22c80c3b..94399535 100644 --- a/libshpool/src/daemon/shell.rs +++ b/libshpool/src/daemon/shell.rs @@ -203,7 +203,6 @@ pub struct ShellToClientArgs { pub conn_id: usize, pub tty_size: TtySize, pub scrollback_lines: usize, - pub session_restore_mode: config::SessionRestoreMode, pub client_connection: crossbeam_channel::Receiver, pub client_connection_ack: crossbeam_channel::Sender, pub tty_size_change: crossbeam_channel::Receiver, @@ -242,13 +241,8 @@ impl SessionInner { let closure = move || { let _s = span!(Level::INFO, "shell->client", s = name, cid = args.conn_id).entered(); - let mut output_spool = session_restore::new( - config, - // TODO: #173 - fetch session restore mode from config dynamically - &args.session_restore_mode, - &args.tty_size, - args.scrollback_lines, - ); + let mut output_spool = + session_restore::new(config, &args.tty_size, args.scrollback_lines); let mut buf: Vec = vec![0; consts::BUF_SIZE]; let mut poll_fds = [poll::PollFd::new( watchable_master.borrow_fd().ok_or(anyhow!("no master fd"))?, @@ -432,7 +426,7 @@ impl SessionInner { } if do_reattach { - info!("executing reattach protocol (mode={:?})", &args.session_restore_mode); + info!("executing reattach protocol"); let restore_buf = output_spool.restore_buffer(); if let (true, ClientConnectionMsg::New(conn)) = (!restore_buf.is_empty(), &mut client_conn) diff --git a/libshpool/src/session_restore/mod.rs b/libshpool/src/session_restore.rs similarity index 96% rename from libshpool/src/session_restore/mod.rs rename to libshpool/src/session_restore.rs index 44d0a5da..83b01a2e 100644 --- a/libshpool/src/session_restore/mod.rs +++ b/libshpool/src/session_restore.rs @@ -64,6 +64,7 @@ impl SessionSpool for NullSpool { fn resize(&mut self, _: TtySize) {} fn restore_buffer(&self) -> Vec { + info!("generating null restore buf"); vec![] } @@ -121,10 +122,10 @@ impl SessionSpool for Vt100Lines { /// Creates a spool given a `mode`. pub fn new( config: config::Manager, - mode: &SessionRestoreMode, size: &TtySize, scrollback_lines: usize, ) -> Box { + let mode = config.get().session_restore_mode.clone().unwrap_or_default(); let vterm_width = config.vterm_width(); match mode { SessionRestoreMode::Simple => Box::new(NullSpool), @@ -134,7 +135,7 @@ pub fn new( }), SessionRestoreMode::Lines(nlines) => Box::new(Vt100Lines { parser: shpool_vt100::Parser::new(size.rows, vterm_width, scrollback_lines), - nlines: *nlines, + nlines, config, }), } diff --git a/shpool/tests/attach.rs b/shpool/tests/attach.rs index 69f9f8df..f01f96e3 100644 --- a/shpool/tests/attach.rs +++ b/shpool/tests/attach.rs @@ -1621,3 +1621,72 @@ fn up_arrow_no_crash() -> anyhow::Result<()> { Ok(()) } + +#[test] +#[timeout(30000)] +#[cfg_attr(target_os = "macos", ignore)] +fn dynamic_session_restore_mode() -> anyhow::Result<()> { + let tmp_dir = tmpdir::Dir::new("/tmp/shpool-test")?; + let config_tmpl = fs::read_to_string(support::testdata_file("dynamic_restore.toml.tmpl"))?; + let config_file = tmp_dir.path().join("shpool.toml"); + + // Start with "simple" mode + let config_contents = config_tmpl.replace("REPLACE_ME", "\"simple\""); + fs::write(&config_file, &config_contents)?; + + let mut daemon_proc = support::daemon::Proc::new(&config_file, DaemonArgs::default()) + .context("starting daemon proc")?; + + // Register all expected events up front. + let mut waiter = daemon_proc.events.take().unwrap().waiter([ + "daemon-bidi-stream-done", // s1 detach + "daemon-reload-config", // config reload + "daemon-bidi-stream-done", // s2 detach + ]); + + // Create session s1 + { + let mut a1 = daemon_proc.attach("s1", Default::default()).context("starting s1")?; + let mut lm1 = a1.line_matcher()?; + a1.run_cmd("echo foo")?; + lm1.scan_until_re("foo$")?; + } // detach s1 + waiter.wait_event("daemon-bidi-stream-done")?; + + // Change config to { lines = 2 } + let config_contents = config_tmpl.replace("REPLACE_ME", "{ lines = 2 }"); + fs::write(&config_file, config_contents)?; + + waiter.wait_event("daemon-reload-config")?; + + // Create session s2 + { + let mut a2 = daemon_proc.attach("s2", Default::default()).context("starting s2")?; + let mut lm2 = a2.line_matcher()?; + a2.run_cmd("echo bar")?; + lm2.scan_until_re("bar$")?; + } // detach s2 + daemon_proc.events = Some(waiter.wait_final_event("daemon-bidi-stream-done")?); + + // Verify s1 still has "simple" behavior (no restore) + { + let mut a1 = daemon_proc.attach("s1", Default::default()).context("reattaching s1")?; + let mut lm1 = a1.line_matcher()?; + lm1.never_matches("foo$")?; + + a1.run_cmd("echo s1_alive")?; + lm1.scan_until_re("s1_alive$")?; + + a1.proc.kill()?; + } + + // Verify s2 has "lines = 2" behavior (restores bar) + { + let mut a2 = daemon_proc.attach("s2", Default::default()).context("reattaching s2")?; + let mut lm2 = a2.line_matcher()?; + // It SHOULD see "bar" again on re-attach + lm2.scan_until_re("bar$")?; + } + + Ok(()) +} diff --git a/shpool/tests/data/dynamic_restore.toml.tmpl b/shpool/tests/data/dynamic_restore.toml.tmpl new file mode 100644 index 00000000..9b402a3a --- /dev/null +++ b/shpool/tests/data/dynamic_restore.toml.tmpl @@ -0,0 +1,9 @@ +norc = true +noecho = true +shell = "/bin/bash" +session_restore_mode = REPLACE_ME +prompt_prefix = "" + +[env] +PS1 = "prompt> " +TERM = "" From 33eac12b1c1945268844eefcb68b9d100d64709c Mon Sep 17 00:00:00 2001 From: Ethan Pailes Date: Tue, 10 Feb 2026 20:30:09 +0000 Subject: [PATCH 2/2] feat: add experimental shpool-vterm engine This patch integrates the new shpool-vterm crate with shpool as a replacement for shpool_vt100. For now, this is opt in, and I expect it to be pretty broken in lots of ways that we'll find out as people start trying it out. By default, we still use the old shpool_vt100 crate. This is progress on #46 --- Cargo.lock | 31 +++++++++++- libshpool/Cargo.toml | 1 + libshpool/src/config.rs | 23 +++++++++ libshpool/src/session_restore.rs | 45 +++++++++++++++-- shpool/tests/data/vterm_lines.toml | 10 ++++ shpool/tests/data/vterm_screen.toml | 10 ++++ shpool/tests/vterm.rs | 76 +++++++++++++++++++++++++++++ 7 files changed, 190 insertions(+), 6 deletions(-) create mode 100644 shpool/tests/data/vterm_lines.toml create mode 100644 shpool/tests/data/vterm_screen.toml create mode 100644 shpool/tests/vterm.rs diff --git a/Cargo.lock b/Cargo.lock index 21d6008b..7eda1b8e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -560,6 +560,7 @@ dependencies = [ "serde_json", "shell-words", "shpool-protocol", + "shpool-vterm", "shpool_pty", "shpool_vt100", "signal-hook", @@ -1008,6 +1009,22 @@ dependencies = [ "serde_derive", ] +[[package]] +name = "shpool-vterm" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72a58af96d28b38601307c0706c52cf1a1e4c2536429c39bbe288738979239d6" +dependencies = [ + "anyhow", + "itoa", + "log", + "smallvec", + "static_assertions", + "tracing", + "unicode-width 0.2.2", + "vte 0.15.0", +] + [[package]] name = "shpool_pty" version = "0.3.2" @@ -1027,7 +1044,7 @@ checksum = "e839aaacba63b9c8fba16affed9507042eb33d802cf8040c4a6f37e3945029c3" dependencies = [ "itoa", "log", - "unicode-width", + "unicode-width 0.1.11", "vte 0.15.0", ] @@ -1056,6 +1073,12 @@ version = "1.13.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6ecd384b10a64542d77071bd64bd7b231f4ed5940fba55e98c3de13824cf3d7" +[[package]] +name = "static_assertions" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" + [[package]] name = "strip-ansi-escapes" version = "0.2.0" @@ -1238,6 +1261,12 @@ version = "0.1.11" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e51733f11c9c4f72aa0c160008246859e340b00807569a0da0e7a1079b27ba85" +[[package]] +name = "unicode-width" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b4ac048d71ede7ee76d585517add45da530660ef4390e49b098733c6e897f254" + [[package]] name = "utf8parse" version = "0.2.1" diff --git a/libshpool/Cargo.toml b/libshpool/Cargo.toml index e4608841..69e0bd1f 100644 --- a/libshpool/Cargo.toml +++ b/libshpool/Cargo.toml @@ -36,6 +36,7 @@ log = "0.4" # logging facade (not used directly, but required if we have tracing tracing = "0.1" # logging and performance monitoring facade rmp-serde = "1" # serialization for the control protocol shpool_vt100 = "0.1.3" # terminal emulation for the scrollback buffer +shpool-vterm = "0.1.0" # alt terminal emulation for the scrollback buffer shell-words = "1" # parsing the -c/--cmd argument motd = { version = "0.2.2", default-features = false, features = [] } # getting the message-of-the-day termini = "1.0.0" # terminfo database diff --git a/libshpool/src/config.rs b/libshpool/src/config.rs index b86c3fdd..3ee2203a 100644 --- a/libshpool/src/config.rs +++ b/libshpool/src/config.rs @@ -245,6 +245,12 @@ pub struct Config { /// existing session. pub session_restore_mode: Option, + /// Selects the virtual terminal to use for lines and screen + /// mode session restoration. By default, this is Vt100, + /// but you can opt into the experimental Vterm engine + /// if you want to try it out. + pub session_restore_engine: Option, + /// The number of lines worth of output to keep in the output /// spool which is maintained along side a shell session. /// By default, 10000 lines. @@ -312,6 +318,7 @@ impl Config { forward_env: self.forward_env.or(another.forward_env), initial_path: self.initial_path.or(another.initial_path), session_restore_mode: self.session_restore_mode.or(another.session_restore_mode), + session_restore_engine: self.session_restore_engine.or(another.session_restore_engine), output_spool_lines: self.output_spool_lines.or(another.output_spool_lines), vt100_output_spool_width: self .vt100_output_spool_width @@ -352,6 +359,16 @@ pub enum SessionRestoreMode { Lines(u16), } +#[derive(Deserialize, Debug, Clone, Default)] +#[serde(rename_all = "lowercase")] +pub enum SessionRestoreEngine { + /// Use the shpool_vt100 crate for session restore. + #[default] + Vt100, + /// Use the shpool-vterm crate for session restore. + Vterm, +} + #[derive(Deserialize, Debug, Clone, Default)] #[serde(rename_all = "lowercase")] pub enum MotdDisplayMode { @@ -413,6 +430,12 @@ mod test { binding = "Ctrl-q a" action = "detach" "#, + r#" + session_restore_engine = "vt100" + "#, + r#" + session_restore_engine = "vterm" + "#, ]; for case in cases.into_iter() { diff --git a/libshpool/src/session_restore.rs b/libshpool/src/session_restore.rs index 83b01a2e..0c8f3eac 100644 --- a/libshpool/src/session_restore.rs +++ b/libshpool/src/session_restore.rs @@ -15,7 +15,7 @@ use shpool_protocol::TtySize; use tracing::info; -use crate::config::{self, SessionRestoreMode}; +use crate::config::{self, SessionRestoreEngine, SessionRestoreMode}; // To prevent data getting dropped, we set this to be large, but we don't want // to use u16::MAX, since the vt100 crate eagerly fills in its rows, and doing @@ -119,24 +119,59 @@ impl SessionSpool for Vt100Lines { } } +/// A spool that restores the last screenful of content using shpool-vterm. +pub struct Vterm { + term: shpool_vterm::Term, + mode: SessionRestoreMode, +} + +impl SessionSpool for Vterm { + fn resize(&mut self, size: TtySize) { + self.term + .resize(shpool_vterm::Size { height: size.rows as usize, width: size.cols as usize }); + } + + fn restore_buffer(&self) -> Vec { + match self.mode { + SessionRestoreMode::Simple => vec![], + SessionRestoreMode::Screen => self.term.contents(shpool_vterm::ContentRegion::Screen), + SessionRestoreMode::Lines(nlines) => { + self.term.contents(shpool_vterm::ContentRegion::BottomLines(nlines as usize)) + } + } + } + + fn process(&mut self, bytes: &[u8]) { + self.term.process(bytes); + } +} + /// Creates a spool given a `mode`. pub fn new( config: config::Manager, size: &TtySize, scrollback_lines: usize, ) -> Box { + let restore_engine = config.get().session_restore_engine.clone().unwrap_or_default(); let mode = config.get().session_restore_mode.clone().unwrap_or_default(); let vterm_width = config.vterm_width(); - match mode { - SessionRestoreMode::Simple => Box::new(NullSpool), - SessionRestoreMode::Screen => Box::new(Vt100Screen { + match (mode, restore_engine) { + (SessionRestoreMode::Simple, _) => Box::new(NullSpool), + (SessionRestoreMode::Screen, SessionRestoreEngine::Vt100) => Box::new(Vt100Screen { parser: shpool_vt100::Parser::new(size.rows, vterm_width, scrollback_lines), config, }), - SessionRestoreMode::Lines(nlines) => Box::new(Vt100Lines { + (SessionRestoreMode::Lines(nlines), SessionRestoreEngine::Vt100) => Box::new(Vt100Lines { parser: shpool_vt100::Parser::new(size.rows, vterm_width, scrollback_lines), nlines, config, }), + (mode, SessionRestoreEngine::Vterm) => Box::new(Vterm { + term: shpool_vterm::Term::new( + scrollback_lines, + shpool_vterm::Size { width: size.cols as usize, height: size.rows as usize }, + ), + mode, + }), } } diff --git a/shpool/tests/data/vterm_lines.toml b/shpool/tests/data/vterm_lines.toml new file mode 100644 index 00000000..db442cc6 --- /dev/null +++ b/shpool/tests/data/vterm_lines.toml @@ -0,0 +1,10 @@ +norc = true +noecho = true +shell = "/bin/bash" +session_restore_mode = { lines = 5 } +session_restore_engine = "vterm" +prompt_prefix = "" + +[env] +PS1 = "prompt> " +TERM = "" diff --git a/shpool/tests/data/vterm_screen.toml b/shpool/tests/data/vterm_screen.toml new file mode 100644 index 00000000..3824a3d3 --- /dev/null +++ b/shpool/tests/data/vterm_screen.toml @@ -0,0 +1,10 @@ +norc = true +noecho = true +shell = "/bin/bash" +session_restore_mode = "screen" +session_restore_engine = "vterm" +prompt_prefix = "" + +[env] +PS1 = "prompt> " +TERM = "" diff --git a/shpool/tests/vterm.rs b/shpool/tests/vterm.rs new file mode 100644 index 00000000..87a6f4c1 --- /dev/null +++ b/shpool/tests/vterm.rs @@ -0,0 +1,76 @@ +#![allow(clippy::literal_string_with_formatting_args)] + +use anyhow::Context; +use ntest::timeout; + +mod support; + +use crate::support::daemon::DaemonArgs; + +#[test] +#[timeout(30000)] +fn screen_restore() -> anyhow::Result<()> { + let mut daemon_proc = support::daemon::Proc::new("vterm_screen.toml", DaemonArgs::default()) + .context("starting daemon proc")?; + let bidi_done_w = daemon_proc.events.take().unwrap().waiter(["daemon-bidi-stream-done"]); + + { + let mut attach_proc = + daemon_proc.attach("sh1", Default::default()).context("starting attach proc")?; + let mut line_matcher = attach_proc.line_matcher()?; + + attach_proc.run_cmd("echo foo")?; + line_matcher.scan_until_re("foo$")?; + } + + // wait until the daemon has noticed that the connection + // has dropped before we attempt to open the connection again + daemon_proc.events = Some(bidi_done_w.wait_final_event("daemon-bidi-stream-done")?); + + { + let mut attach_proc = + daemon_proc.attach("sh1", Default::default()).context("starting attach proc")?; + let mut line_matcher = attach_proc.line_matcher()?; + + // the re-attach should redraw the screen for us, so we should + // get a line with "foo" as part of the re-drawn screen. + line_matcher.scan_until_re("foo$")?; + + attach_proc.proc.kill()?; + } + + Ok(()) +} + +#[test] +#[timeout(30000)] +fn lines_restore() -> anyhow::Result<()> { + let mut daemon_proc = support::daemon::Proc::new("vterm_lines.toml", DaemonArgs::default()) + .context("starting daemon proc")?; + let bidi_done_w = daemon_proc.events.take().unwrap().waiter(["daemon-bidi-stream-done"]); + + { + let mut attach_proc = + daemon_proc.attach("sh1", Default::default()).context("starting attach proc")?; + let mut line_matcher = attach_proc.line_matcher()?; + + attach_proc.run_cmd("echo foo")?; + line_matcher.scan_until_re("foo$")?; + } + + // wait until the daemon has noticed that the connection + // has dropped before we attempt to open the connection again + daemon_proc.events = Some(bidi_done_w.wait_final_event("daemon-bidi-stream-done")?); + + { + let mut attach_proc = + daemon_proc.attach("sh1", Default::default()).context("starting attach proc")?; + let mut line_matcher = attach_proc.line_matcher()?; + + // the re-attach should redraw the last 2 lines for us, so we should + // get a line with "foo" as part of the re-drawn screen. + line_matcher.scan_until_re("foo$")?; + } + + Ok(()) +}