diff --git a/crates/maestro-daemon/src/panes.rs b/crates/maestro-daemon/src/panes.rs index 1769025..088a65e 100644 --- a/crates/maestro-daemon/src/panes.rs +++ b/crates/maestro-daemon/src/panes.rs @@ -39,6 +39,10 @@ pub trait Panes: std::fmt::Debug + Send + Sync + 'static { fn pipe_pane(&self, id: Uuid, sink_command: &str) -> Result<(), TmuxError>; fn resize_window(&self, id: Uuid, cols: u16, rows: u16) -> Result<(), TmuxError>; + + /// Escape-sequence snapshot of the pane's current screen, or `None` when + /// there is no pane to capture. See [`Tmux::capture_pane`]. + fn capture_pane(&self, id: Uuid) -> Result>, TmuxError>; } impl Panes for Tmux { @@ -76,6 +80,10 @@ impl Panes for Tmux { fn resize_window(&self, id: Uuid, cols: u16, rows: u16) -> Result<(), TmuxError> { Tmux::resize_window(self, id, cols, rows) } + + fn capture_pane(&self, id: Uuid) -> Result>, TmuxError> { + Tmux::capture_pane(self, id) + } } // --------------------------------------------------------------------------- @@ -92,6 +100,7 @@ pub enum Op { SendKeys, PipePane, ResizeWindow, + CapturePane, } /// One recorded call, in the order the manager made it. Rollback ordering is a @@ -116,6 +125,7 @@ pub enum Call { cols: u16, rows: u16, }, + CapturePane(Uuid), } /// In-memory [`Panes`] for tests. @@ -133,6 +143,7 @@ struct FakeState { live: HashSet, panes: HashMap, calls: Vec, + captures: HashMap>, fail: HashMap, } @@ -162,6 +173,12 @@ impl FakePanes { self.lock().live.contains(&id) } + /// Screen snapshot `capture_pane` hands back for `id`. Unset means "no + /// pane", which is what a dead session looks like. + pub fn set_capture(&self, id: Uuid, bytes: impl Into>) { + self.lock().captures.insert(id, bytes.into()); + } + pub fn set_pane_info(&self, id: Uuid, info: PaneInfo) { self.lock().panes.insert(id, info); } @@ -242,6 +259,12 @@ impl Panes for FakePanes { fn resize_window(&self, id: Uuid, cols: u16, rows: u16) -> Result<(), TmuxError> { self.record(Call::ResizeWindow { id, cols, rows }, Op::ResizeWindow) } + + fn capture_pane(&self, id: Uuid) -> Result>, TmuxError> { + self.record(Call::CapturePane(id), Op::CapturePane)?; + let st = self.lock(); + Ok(st.captures.get(&id).cloned()) + } } #[cfg(test)] diff --git a/crates/maestro-daemon/src/pty.rs b/crates/maestro-daemon/src/pty.rs index bc4855d..44f079d 100644 --- a/crates/maestro-daemon/src/pty.rs +++ b/crates/maestro-daemon/src/pty.rs @@ -141,8 +141,21 @@ impl PtyBridge { /// `subscribe()` keeps the reader task from publishing in between, so /// the seam has no gap and no duplicates. pub fn subscribe_with_replay(&self) -> (Vec, broadcast::Receiver>) { + self.subscribe_with_snapshot(|| None) + } + + /// As [`Self::subscribe_with_replay`], but lets the caller supply the + /// replay payload instead of the raw byte buffer — used to hand a fresh + /// client tmux's own view of the screen, which the byte tail cannot + /// reconstruct for a full-screen program. `capture` runs while the reader + /// is held off, so the payload and the live stream still meet with no gap + /// and no duplicate. Returning `None` falls back to the byte buffer. + pub fn subscribe_with_snapshot(&self, capture: F) -> (Vec, broadcast::Receiver>) + where + F: FnOnce() -> Option>, + { let guard = self.replay.lock_for_snapshot(); - let snapshot = guard.snapshot(); + let snapshot = capture().unwrap_or_else(|| guard.snapshot()); let rx = match &self.sender { Some(tx) => tx.subscribe(), // Sealed: hand back the scrollback plus a receiver that is already diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index 8aff997..d34468a 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -260,8 +260,29 @@ impl SessionManager { &self, id: Uuid, ) -> Option<(Vec, broadcast::Receiver>)> { + let tmux = self.inner.tmux.clone(); let sessions = self.inner.sessions.lock().await; - sessions.get(&id).map(|r| r.bridge.subscribe_with_replay()) + let rec = sessions.get(&id)?; + // For a live session, tmux's own screen beats the byte tail: the tail + // starts mid-stream, so a full-screen program (an agent TUI) replayed + // from it renders as garbage until something forces a repaint. A dead + // session has no pane left to capture, so it keeps the byte tail. + let alive = rec.session.status == SessionStatus::Running; + // The capture shells out to tmux while the replay lock is held, which + // parks the reader for the length of one tmux round-trip. That only + // happens on attach, and the alternative is a seam in the stream. + Some(rec.bridge.subscribe_with_snapshot(|| { + if !alive { + return None; + } + match tmux.capture_pane(id) { + Ok(snapshot) => snapshot, + Err(e) => { + tracing::warn!(error = ?e, %id, "capture-pane failed; replaying raw bytes"); + None + } + } + })) } pub async fn has(&self, id: Uuid) -> bool { diff --git a/crates/maestro-daemon/src/tmux.rs b/crates/maestro-daemon/src/tmux.rs index 1d278b9..545c02f 100644 --- a/crates/maestro-daemon/src/tmux.rs +++ b/crates/maestro-daemon/src/tmux.rs @@ -246,6 +246,66 @@ impl Tmux { check(out) } + /// Escape-sequence snapshot of a pane's current screen (#…), for clients + /// that just attached. + /// + /// The raw byte replay buffer cannot reconstruct a full-screen program: + /// its tail starts mid-stream, long after the app set up the modes, + /// colours and cursor position that give those bytes meaning. A browser + /// refresh therefore painted garbage until something forced the app to + /// repaint. tmux already holds the authoritative screen, so ask it. + /// + /// Returns `Ok(None)` when the session or the tmux server is gone — the + /// caller falls back to the byte replay, which is all a dead session has. + pub fn capture_pane(&self, id: Uuid) -> Result>, TmuxError> { + let name = session_name(id); + let start = format!("-{CAPTURE_HISTORY_LINES}"); + // One invocation, so tmux runs both commands in a single pass of its + // command queue: the cursor position cannot belong to an older screen + // than the captured content. Two invocations raced — a program that + // switched to the alternate screen in between produced a snapshot with + // the new screen and the old cursor. + let out = self + .cmd() + .args([ + "display-message", + "-p", + "-t", + &name, + "-F", + "#{cursor_x} #{cursor_y} #{pane_height} #{alternate_on} #{cursor_flag}", + ";", + "capture-pane", + "-p", + "-e", + "-t", + &name, + "-S", + &start, + ]) + .output()?; + let Some(stdout) = soft_output(out)? else { + return Ok(None); + }; + let Some((head, content)) = stdout.split_once('\n') else { + return Ok(None); + }; + let Some(geom) = parse_screen_geometry(head) else { + return Ok(None); + }; + let mut lines: Vec<&str> = content + .strip_suffix('\n') + .unwrap_or(content) + .split('\n') + .collect(); + // Scrollback belongs to the normal screen. Replaying it under an + // alternate-screen program would paste the shell's history behind it. + if geom.alternate { + lines.drain(..lines.len().saturating_sub(geom.pane_height)); + } + Ok(Some(render_screen_snapshot(&lines, geom))) + } + /// `tmux resize-window -t maestro- -x -y `. pub fn resize_window(&self, id: Uuid, cols: u16, rows: u16) -> Result<(), TmuxError> { let name = session_name(id); @@ -446,6 +506,124 @@ fn new_session_args( args } +/// How much scrollback a fresh attach inherits from tmux's history, in lines. +/// Enough to land in context after a browser refresh without making the first +/// frame huge. +const CAPTURE_HISTORY_LINES: usize = 300; + +/// Cursor position and screen geometry of a pane, from `display-message`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct ScreenGeometry { + cursor_x: usize, + cursor_y: usize, + pane_height: usize, + alternate: bool, + cursor_visible: bool, +} + +fn parse_screen_geometry(s: &str) -> Option { + let line = s.lines().next()?; + let mut it = line.split_whitespace(); + let cursor_x = it.next()?.parse().ok()?; + let cursor_y = it.next()?.parse().ok()?; + let pane_height: usize = it.next()?.parse().ok()?; + let alternate = it.next()? == "1"; + // Older tmux without `cursor_flag` just means "assume visible". + let cursor_visible = it.next().map(|f| f == "1").unwrap_or(true); + if pane_height == 0 { + return None; + } + Some(ScreenGeometry { + cursor_x, + cursor_y, + pane_height, + alternate, + cursor_visible, + }) +} + +/// Turn captured pane lines into bytes a fresh terminal can replay: clear, +/// print the lines, then put the cursor back where the pane has it. +/// +/// The cursor is restored with *relative* moves so the snapshot renders the +/// same whatever height the client's terminal has — it may not match the +/// pane's yet, since the client's first `Resize` arrives after this. +fn render_screen_snapshot(lines: &[&str], geom: ScreenGeometry) -> Vec { + // Screen rows are the last `pane_height` captured lines; anything before + // them is scrollback. + let screen_top = lines.len().saturating_sub(geom.pane_height); + let cursor_row = (screen_top + geom.cursor_y).min(lines.len().saturating_sub(1)); + // Trailing blank rows would scroll the real content off a short client + // terminal, so drop them — but never the cursor's own row or anything + // above it, which is where a program that draws below its cursor lives. + let last = lines + .iter() + .rposition(|l| !strip_sgr(l).trim().is_empty()) + .unwrap_or(0) + .max(cursor_row); + + let mut out = Vec::new(); + // Put the client on the same buffer the pane is using, so a program's + // repaints stay out of the client's scrollback while it runs. + if geom.alternate { + out.extend_from_slice(b"\x1b[?1049h"); + } + out.extend_from_slice(b"\x1b[H\x1b[2J\x1b[m"); + for (i, line) in lines[..=last].iter().enumerate() { + if i > 0 { + out.extend_from_slice(b"\r\n"); + } + out.extend_from_slice(line.as_bytes()); + } + out.extend_from_slice(b"\x1b[m\r"); + if last > cursor_row { + out.extend_from_slice(format!("\x1b[{}A", last - cursor_row).as_bytes()); + } + if geom.cursor_x > 0 { + out.extend_from_slice(format!("\x1b[{}C", geom.cursor_x).as_bytes()); + } + if !geom.cursor_visible { + out.extend_from_slice(b"\x1b[?25l"); + } + out +} + +/// Text of a line with SGR escapes removed, for emptiness checks. +fn strip_sgr(line: &str) -> String { + let mut out = String::with_capacity(line.len()); + let mut chars = line.chars(); + while let Some(c) = chars.next() { + if c != '\x1b' { + out.push(c); + continue; + } + // Skip CSI ... final-byte. Anything else escape-ish is not produced by + // `capture-pane -e`, so treating it the same way is harmless. + for c in chars.by_ref() { + if c.is_ascii_alphabetic() { + break; + } + } + } + out +} + +/// `Ok(None)` when tmux failed because the server or the target is gone, +/// `Ok(Some(stdout))` on success, `Err` for anything else. +fn soft_output(out: Output) -> Result, TmuxError> { + if out.status.success() { + return Ok(Some(String::from_utf8_lossy(&out.stdout).into_owned())); + } + let stderr = String::from_utf8_lossy(&out.stderr); + if is_no_server_error(&stderr) || stderr.contains("can't find") { + return Ok(None); + } + Err(TmuxError::NonZero { + code: out.status.code().unwrap_or(-1), + stderr: stderr.into_owned(), + }) +} + fn check(out: Output) -> Result<(), TmuxError> { if out.status.success() { Ok(()) @@ -461,6 +639,85 @@ fn check(out: Output) -> Result<(), TmuxError> { mod tests { use super::*; + #[test] + fn parse_screen_geometry_reads_cursor_and_alternate_flag() { + let g = parse_screen_geometry("12 3 40 1 0\n").unwrap(); + assert_eq!( + g, + ScreenGeometry { + cursor_x: 12, + cursor_y: 3, + pane_height: 40, + alternate: true, + cursor_visible: false, + } + ); + let g = parse_screen_geometry("0 0 24 0").unwrap(); + assert!(!g.alternate); + assert!(g.cursor_visible, "missing cursor_flag means visible"); + assert!(parse_screen_geometry("0 0 0 0").is_none()); + assert!(parse_screen_geometry("garbage").is_none()); + } + + #[test] + fn snapshot_clears_prints_lines_and_parks_the_cursor() { + // Two scrollback lines, then a 3-row screen with the cursor on row 1. + let lines = ["old", "older", "prompt> hi", "", ""]; + let out = render_screen_snapshot( + &lines, + ScreenGeometry { + cursor_x: 10, + cursor_y: 0, + pane_height: 3, + alternate: false, + cursor_visible: true, + }, + ); + let out = String::from_utf8(out).unwrap(); + assert_eq!( + out, + "\x1b[H\x1b[2J\x1b[mold\r\nolder\r\nprompt> hi\x1b[m\r\x1b[10C" + ); + } + + #[test] + fn snapshot_keeps_rows_below_the_cursor_and_moves_back_up() { + let lines = ["prompt> hi", "hint line", ""]; + let out = render_screen_snapshot( + &lines, + ScreenGeometry { + cursor_x: 0, + cursor_y: 0, + pane_height: 3, + alternate: true, + cursor_visible: false, + }, + ); + let out = String::from_utf8(out).unwrap(); + assert!(out.starts_with("\x1b[?1049h\x1b[H\x1b[2J"), "{out:?}"); + assert!( + out.ends_with("prompt> hi\r\nhint line\x1b[m\r\x1b[1A\x1b[?25l"), + "{out:?}" + ); + } + + #[test] + fn snapshot_ignores_colour_only_rows_when_trimming() { + let lines = ["text", "\x1b[38;5;4m\x1b[m", ""]; + let out = String::from_utf8(render_screen_snapshot( + &lines, + ScreenGeometry { + cursor_x: 4, + cursor_y: 0, + pane_height: 3, + alternate: false, + cursor_visible: true, + }, + )) + .unwrap(); + assert_eq!(out, "\x1b[H\x1b[2J\x1b[mtext\x1b[m\r\x1b[4C"); + } + /// Every phrasing must be recognised: an unmatched one makes /// `list_sessions` fail, and a failed pass reaps nothing. #[test] diff --git a/crates/maestro-daemon/tests/session_manager.rs b/crates/maestro-daemon/tests/session_manager.rs index 5389026..a07114a 100644 --- a/crates/maestro-daemon/tests/session_manager.rs +++ b/crates/maestro-daemon/tests/session_manager.rs @@ -188,6 +188,64 @@ async fn create_captures_initial_output_in_replay_buffer() { fx.manager.kill(s.id, false).await.unwrap(); } +/// A fresh attach must land on the pane's *current* screen. Replaying the raw +/// byte tail instead reproduced a browser refresh painting garbage over an +/// agent TUI: the tail starts mid-stream, so the clears, modes and cursor +/// moves that gave those bytes meaning are long gone. +#[tokio::test] +async fn attach_replay_is_the_current_screen_not_the_byte_tail() { + let fx = DaemonFixture::new(); + let cwd = TempDir::new().unwrap(); + + let s = fx + .manager + .create(CreateSessionRequest { + cwd: cwd.path().to_path_buf(), + // Print a line, then switch to the alternate screen the way a + // full-screen program does on startup. Only the second marker is + // on the pane; the first belongs to the screen behind it. + command: "sh -c 'printf STALE_MARKER_LINE\\\\n; sleep 0.5; printf \"\\\\033[?1049h\\\\033[2J\\\\033[HLIVE_MARKER\"; exec sleep 30'" + .into(), + env: HashMap::new(), + metadata: HashMap::new(), + worktree: None, + }) + .await + .expect("create"); + + let want = b"LIVE_MARKER"; + let deadline = std::time::Instant::now() + Duration::from_secs(5); + let mut replay = Vec::new(); + while std::time::Instant::now() < deadline { + let (bytes, _rx) = fx + .manager + .subscribe_pty_with_replay(s.id) + .await + .expect("subscribe"); + replay = bytes; + if replay.windows(want.len()).any(|w| w == want) { + break; + } + tokio::time::sleep(Duration::from_millis(50)).await; + } + + let text = String::from_utf8_lossy(&replay).into_owned(); + assert!( + replay.starts_with(b"\x1b[?1049h\x1b[H\x1b[2J"), + "replay did not start with an alternate-screen snapshot: {text:?}" + ); + assert!( + text.contains("LIVE_MARKER"), + "replay lost the live screen: {text:?}" + ); + assert!( + !text.contains("STALE_MARKER_LINE"), + "replay carried bytes from behind the alternate screen: {text:?}" + ); + + fx.manager.kill(s.id, false).await.unwrap(); +} + /// Regression for NAP-228: after `recover`, PTY output must reach new /// subscribers. Before the fix tmux's stale `pipe-pane` (with `-o`) silently /// kept feeding the old FIFO and the revived manager's subscribers saw