git-ents.gitmain
⌘K
foforge
commit 9f3c0a1
feat: forward local terminal resizes into a debug session

Debug sessions previously left the remote shell at a fixed 80x24: the broker gave the sprite’s sprite exec --tty child plain pipes, which have no window size to change. The broker now allocates its own local pty for that child (portable-pty, since resizing a pty needs an ioctl this workspace’s unsafe_code = "forbid" lint blocks doing by hand) and applies a "<cols> <rows>" control frame sent as a text message over the same WebSocket, distinct from the binary data frames.

The CLI sends that frame once on connect and again on every local SIGWINCH, read via signal-hook on a dedicated thread since the byte-passthrough loop can’t go through `crossterm’s own event reader (that would parse and consume the very bytes it needs to forward raw).

feat: allocate a local pty for the debug broker’s sprite-exec child so it can be resized feat: forward SIGWINCH from the CLI as a resize control frame Assisted-by: Claude:claude-sonnet-5

Joseph D. Carpinelli · 1 month ago

Reviews

No reviews of this commit yet — record a verdict below.

Start a review

verdict

Cargo.lock @@ -9,7 +9,7 @@ dependencies = [ "acdc-parser", "bitflags 2.13.0", - "thiserror", + "thiserror 2.0.18", "tracing", ] @@ -21,7 +21,7 @@ "acdc-converters-core", "acdc-parser", "chrono", - "thiserror", + "thiserror 2.0.18", "tracing", ] @@ -40,7 +40,7 @@ "self_cell", "serde", "serde_json", - "thiserror", + "thiserror 2.0.18", "tracing", "url", ] @@ -125,6 +125,12 @@ "windows-sys 0.61.2", ] +[[package]] +name = "anyhow" +version = "1.0.103" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a4385e2e34eb35d6b3efe798b9eb88096925d87726c0798709bf56d9ed84af3" + [[package]] name = "arborium" version = "2.18.0" @@ -599,6 +605,12 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" +[[package]] +name = "cfg_aliases" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fd16c4719339c4530435d38e511904438d07cce7950afa3718a84ac36c10e89e" + [[package]] name = "chrono" version = "0.4.45" @@ -838,7 +850,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "10d60334b3b2e7c9d91ef8150abfb6fa4c1c39ebbcf4a81c2e346aad939fee3e" dependencies = [ - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -904,6 +916,12 @@ "litrs", ] +[[package]] +name = "downcast-rs" +version = "1.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "75b325c5dbd37f80359721ad39aca5a29fb04c89279657cffdda8736d0c0b9d2" + [[package]] name = "dunce" version = "1.0.5" @@ -974,7 +992,7 @@ "gix-hash", "gix-object", "gix-odb", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1059,6 +1077,17 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9f1f227452a390804cdb637b74a86990f2a7d7ba4b7d5693aac9b4dd6defd8d6" +[[package]] +name = "filedescriptor" +version = "0.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e40758ed24c9b2eeb76c35fb0aebc66c626084edd827e07e1552279814c6682d" +dependencies = [ + "libc", + "thiserror 1.0.69", + "winapi", +] + [[package]] name = "filetime" version = "0.2.29" @@ -1219,8 +1248,9 @@ "futures-util", "git-store", "iddqd", + "signal-hook 0.3.18", "tempfile", - "thiserror", + "thiserror 2.0.18", "tokio", "tokio-tungstenite", "ureq", @@ -1248,6 +1278,7 @@ "gix-hash", "gix-object", "maud", + "portable-pty", "rstest", "tempfile", "tokio", @@ -1262,7 +1293,7 @@ "facet-git-tree", "gix", "tempfile", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1324,7 +1355,7 @@ "regex", "signal-hook 0.4.4", "smallvec 1.15.2", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1364,7 +1395,7 @@ "gix-trace", "kstring", "smallvec 1.15.2", - "thiserror", + "thiserror 2.0.18", "unicode-bom", ] @@ -1394,7 +1425,7 @@ "gix-traverse", "gix-worktree", "smallvec 1.15.2", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1447,7 +1478,7 @@ "gix-ref", "gix-sec", "smallvec 1.15.2", - "thiserror", + "thiserror 2.0.18", "unicode-bom", ] @@ -1461,7 +1492,7 @@ "bstr", "gix-path", "libc", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1479,7 +1510,7 @@ "gix-sec", "gix-trace", "gix-url", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1515,7 +1546,7 @@ "gix-trace", "gix-traverse", "gix-worktree", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1535,7 +1566,7 @@ "gix-trace", "gix-utils", "gix-worktree", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1550,7 +1581,7 @@ "gix-path", "gix-ref", "gix-sec", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1579,7 +1610,7 @@ "once_cell", "parking_lot", "prodash", - "thiserror", + "thiserror 2.0.18", "walkdir", "zlib-rs", ] @@ -1602,7 +1633,7 @@ "gix-trace", "gix-utils", "smallvec 1.15.2", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1616,7 +1647,7 @@ "gix-features", "gix-path", "gix-utils", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1640,7 +1671,7 @@ "faster-hex", "gix-features", "sha1-checked", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1702,7 +1733,7 @@ "memmap2", "rustix", "smallvec 1.15.2", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1713,7 +1744,7 @@ dependencies = [ "gix-tempfile", "gix-utils", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1758,7 +1789,7 @@ "gix-validate", "itoa", "smallvec 1.15.2", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1779,7 +1810,7 @@ "memmap2", "parking_lot", "tempfile", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1798,7 +1829,7 @@ "gix-path", "memmap2", "smallvec 1.15.2", - "thiserror", + "thiserror 2.0.18", "uluru", ] @@ -1811,7 +1842,7 @@ "bstr", "faster-hex", "gix-trace", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1823,7 +1854,7 @@ "bstr", "gix-trace", "gix-validate", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1838,7 +1869,7 @@ "gix-config-value", "gix-glob", "gix-path", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1851,7 +1882,7 @@ "gix-config-value", "parking_lot", "rustix", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1870,7 +1901,7 @@ "gix-utils", "maybe-async", "nonempty", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1901,7 +1932,7 @@ "gix-utils", "gix-validate", "memmap2", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1917,7 +1948,7 @@ "gix-revision", "gix-validate", "smallvec 1.15.2", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1952,7 +1983,7 @@ "gix-hashtable", "gix-object", "smallvec 1.15.2", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -1977,7 +2008,7 @@ "gix-hash", "gix-lock", "nonempty", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -2000,7 +2031,7 @@ "gix-pathspec", "gix-worktree", "portable-atomic", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -2015,7 +2046,7 @@ "gix-pathspec", "gix-refspec", "gix-url", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -2052,7 +2083,7 @@ "gix-quote", "gix-sec", "gix-url", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -2069,7 +2100,7 @@ "gix-object", "gix-revwalk", "smallvec 1.15.2", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -2081,7 +2112,7 @@ "bstr", "gix-path", "percent-encoding", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -2137,7 +2168,7 @@ "gix-path", "gix-worktree", "io-close", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -2542,6 +2573,12 @@ "static_assertions", ] +[[package]] +name = "lazy_static" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" + [[package]] name = "libc" version = "0.2.186" @@ -2671,6 +2708,18 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bc0287524726960e07b119cebd01678f852f147742ae0d925e6a520dca956126" +[[package]] +name = "nix" +version = "0.28.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ab2156c4fce2f8df6c499cc1c763e4394b7482525bf2a9701c9d79d215f519e4" +dependencies = [ + "bitflags 2.13.0", + "cfg-if", + "cfg_aliases", + "libc", +] + [[package]] name = "nonempty" version = "0.12.0" @@ -2775,6 +2824,27 @@ "portable-atomic", ] +[[package]] +name = "portable-pty" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b4a596a2b3d2752d94f51fac2d4a96737b8705dddd311a32b9af47211f08671e" +dependencies = [ + "anyhow", + "bitflags 1.3.2", + "downcast-rs", + "filedescriptor", + "lazy_static", + "libc", + "log", + "nix", + "serial2", + "shared_library", + "shell-words", + "winapi", + "winreg", +] + [[package]] name = "potential_utf" version = "0.1.5" @@ -3167,6 +3237,17 @@ "serde", ] +[[package]] +name = "serial2" +version = "0.2.37" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9eb6ea5562eeaed6936b8b54e086aa0f88b9e5b1bef45beb038e2519fa1185b1" +dependencies = [ + "cfg-if", + "libc", + "windows-sys 0.61.2", +] + [[package]] name = "sha1" version = "0.10.6" @@ -3188,6 +3269,16 @@ "sha1", ] +[[package]] +name = "shared_library" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a9e7e0f2bfae24d8a5b5a66c5b257a83c7412304311512a0c054cd5e619da11" +dependencies = [ + "lazy_static", + "libc", +] + [[package]] name = "shell-words" version = "1.1.1" @@ -3346,13 +3437,33 @@ "windows-sys 0.61.2", ] +[[package]] +name = "thiserror" +version = "1.0.69" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6aaf5339b578ea85b50e080feb250a3e8ae8cfcdff9a461c9ec2904bc923f52" +dependencies = [ + "thiserror-impl 1.0.69", +] + [[package]] name = "thiserror" version = "2.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4288b5bcbc7920c07a1149a35cf9590a2aa808e0bc1eafaade0b80947865fbc4" dependencies = [ - "thiserror-impl", + "thiserror-impl 2.0.18", +] + +[[package]] +name = "thiserror-impl" +version = "1.0.69" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" +dependencies = [ + "proc-macro2", + "quote", + "syn", ] [[package]] @@ -3555,7 +3666,7 @@ "rustls", "rustls-pki-types", "sha1", - "thiserror", + "thiserror 2.0.18", ] [[package]] @@ -3962,6 +4073,15 @@ "memchr", ] +[[package]] +name = "winreg" +version = "0.10.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "80d0f4e272c85def139476380b12f9ac60926689dd2e01d4923222f40580869d" +dependencies = [ + "winapi", +] + [[package]] name = "wit-bindgen" version = "0.57.1"
crates/git-ents-server/Cargo.toml @@ -27,6 +27,7 @@ tempfile = { workspace = true } tokio = { workspace = true } uuid = { workspace = true } +portable-pty = "0.9.0" [dev-dependencies] rstest = { workspace = true }
crates/git-ents/Cargo.toml @@ -13,6 +13,7 @@ futures-util = { version = "0.3.32", default-features = false, features = ["sink", "std"] } git-store = { workspace = true } iddqd = { workspace = true } +signal-hook = { version = "0.3", features = ["iterator"] } tempfile = { workspace = true } thiserror = { workspace = true } tokio = { workspace = true, features = ["rt-multi-thread", "macros", "sync"] }
crates/git-ents/src/debug_session.rs @@ -58,6 +58,14 @@ } }); + let (resize_tx, mut resize_rx) = tokio::sync::mpsc::unbounded_channel::<(u16, u16)>(); + spawn_resize_watcher(resize_tx.clone()); + // Size the remote pty to match this terminal before the first byte flows, + // rather than leaving it at the broker's default until the first resize. + if let Ok(size) = crossterm::terminal::size() { + let _sent = resize_tx.send(size); + } + loop { tokio::select! { input = rx.recv() => { @@ -70,6 +78,14 @@ None => return Ok(()), } } + resize = resize_rx.recv() => { + if let Some((cols, rows)) = resize { + let frame = Message::Text(format!("{cols} {rows}").into()); + if sink.send(frame).await.is_err() { + return Ok(()); + } + } + } frame = source.next() => { match frame { Some(Ok(Message::Binary(data))) => { @@ -84,3 +100,24 @@ } } } + +/// Watch for local terminal resizes (`SIGWINCH`) on a dedicated thread, +/// sending the new `(cols, rows)` through `tx` each time. `crossterm`'s own +/// event reader also reports resizes, but isn't an option here — we forward +/// raw stdin bytes rather than events it would parse and consume them from. +fn spawn_resize_watcher(tx: tokio::sync::mpsc::UnboundedSender<(u16, u16)>) { + let Ok(mut signals) = signal_hook::iterator::Signals::new([signal_hook::consts::SIGWINCH]) + else { + return; + }; + std::thread::spawn(move || { + for _signal in signals.forever() { + let Ok(size) = crossterm::terminal::size() else { + continue; + }; + if tx.send(size).is_err() { + break; + } + } + }); +}
crates/git-ents-server/src/web/debug.rs @@ -8,15 +8,26 @@ //! literally named `_debug` is shadowed, the same tradeoff `/login` already //! makes against a repo named `login`. +use std::io::{Read as _, Write as _}; + use axum::extract::ws::{Message, WebSocket, WebSocketUpgrade}; use axum::extract::{Path, State}; use axum::http::{HeaderMap, StatusCode}; use axum::response::{IntoResponse, Response}; -use tokio::io::{AsyncReadExt as _, AsyncWriteExt as _}; -use tokio::process::Command; +use portable_pty::{CommandBuilder, PtySize, native_pty_system}; use crate::AppState; +/// The pty's initial size, before the CLI's first resize control frame +/// arrives — the CLI sends one immediately on connecting, so this only +/// matters for the handful of frames in between. +const INITIAL_SIZE: PtySize = PtySize { + rows: 24, + cols: 80, + pixel_width: 0, + pixel_height: 0, +}; + /// Upgrade an authenticated member's request into an interactive shell in /// `repo_path`'s checks Sprite. pub(crate) async fn handshake( @@ -75,49 +86,101 @@ } /// Spawn an interactive shell in `sprite` and relay it over `socket` until -/// either side closes: the Sprite CLI's own `--tty` handles the pseudo-TTY, so -/// the broker only ever pumps bytes. +/// either side closes: the Sprite CLI's own `--tty` handles the remote +/// pseudo-TTY, but the broker allocates its *own* local pty for the `sprite +/// exec --tty` process so a resize control frame (see below) has something to +/// apply to — plain pipes have no window size to change. async fn relay(mut socket: WebSocket, sprite: String) { - let child = Command::new("sprite") - .args(["exec", "--tty", "-s", &sprite, "--", "/bin/bash"]) - .stdin(std::process::Stdio::piped()) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .spawn(); - let mut child = match child { + let pair = match native_pty_system().openpty(INITIAL_SIZE) { + Ok(pair) => pair, + Err(_could_not_allocate) => return, + }; + let mut cmd = CommandBuilder::new("sprite"); + cmd.args(["exec", "--tty", "-s", &sprite, "--", "/bin/bash"]); + let mut child = match pair.slave.spawn_command(cmd) { Ok(child) => child, Err(_could_not_spawn) => return, }; - let (Some(mut stdin), Some(mut stdout)) = (child.stdin.take(), child.stdout.take()) else { + // Drop our copy of the slave side once the child holds it, so the + // master's reader sees EOF when the child actually exits rather than + // when this process happens to close it. + drop(pair.slave); + + let master = pair.master; + let (Ok(reader), Ok(mut writer)) = (master.try_clone_reader(), master.take_writer()) else { return; }; - let mut buf = [0u8; 4096]; + // The pty's Read/Write are blocking, so each direction gets its own + // thread; the read side hands chunks to the async loop over a channel, + // the write side is fed the same way so a slow write never blocks the + // select loop. + let (out_tx, mut out_rx) = tokio::sync::mpsc::unbounded_channel::<Vec<u8>>(); + std::thread::spawn(move || { + let mut reader = reader; + let mut buf = [0u8; 4096]; + loop { + match reader.read(&mut buf) { + Ok(0) | Err(_) => break, + Ok(n) => { + let Some(chunk) = buf.get(..n) else { break }; + if out_tx.send(chunk.to_vec()).is_err() { + break; + } + } + } + } + }); + let (in_tx, in_rx) = std::sync::mpsc::channel::<Vec<u8>>(); + std::thread::spawn(move || { + while let Ok(data) = in_rx.recv() { + if writer.write_all(&data).is_err() { + break; + } + } + }); + loop { tokio::select! { - read = stdout.read(&mut buf) => { - match read { - Ok(0) | Err(_) => break, - Ok(n) => { - let Some(chunk) = buf.get(..n) else { break }; - if socket.send(Message::Binary(chunk.to_vec().into())).await.is_err() { + chunk = out_rx.recv() => { + match chunk { + Some(data) => { + if socket.send(Message::Binary(data.into())).await.is_err() { break; } } + None => break, } } frame = socket.recv() => { match frame { Some(Ok(Message::Binary(data))) => { - if stdin.write_all(&data).await.is_err() { + if in_tx.send(data.to_vec()).is_err() { break; } } + Some(Ok(Message::Text(text))) => { + if let Some(size) = parse_resize(&text) { + let _resized = master.resize(size); + } + } Some(Ok(Message::Close(_))) | None | Some(Err(_)) => break, _ => {} } } } } - let _killed = child.kill().await; + let _killed = child.kill(); +} + +/// Parse a resize control frame, `"<cols> <rows>"`, as sent by the CLI on +/// connect and on every local `SIGWINCH`. +fn parse_resize(text: &str) -> Option<PtySize> { + let (cols, rows) = text.split_once(' ')?; + Some(PtySize { + cols: cols.parse().ok()?, + rows: rows.parse().ok()?, + pixel_width: 0, + pixel_height: 0, + }) }