Merge remote-tracking branch 'origin/main' into blitz/command-bar

crates/maestro-daemon/src/attach.rs

@@ -26,11 +26,17 @@ pub(crate) async fn serve(
 ) -> Result<(), axum::Error> {
     let (mut sink, mut stream) = socket.split();
 
+    // This client's claim on the shared window size. Registered up front and
+    // released on every exit below, so a socket that dies mid-session stops
+    // constraining the size for the clients still attached.
+    let viewport = manager.attach_viewport();
+
     // NAP-229: send buffered scrollback up-front so the client sees the last
     // screenful immediately instead of a blank terminal. The replay lock the
     // bridge held during snapshot+subscribe guarantees no byte appears both
     // here and on the live receiver, and none is dropped at the seam.
     if !replay.is_empty() && sink.send(Message::Binary(replay)).await.is_err() {
+        manager.release_viewport(id, viewport).await;
         return Ok(());
     }
 
@@ -70,7 +76,7 @@ pub(crate) async fn serve(
                     if let Ok(ctrl) = serde_json::from_str::<ClientControl>(&text) {
                         match ctrl {
                             ClientControl::Resize { cols, rows } => {
-                                let _ = manager.resize(id, cols, rows).await;
+                                let _ = manager.set_viewport(id, viewport, cols, rows).await;
                             }
                             ClientControl::Ping => {
                                 let pong = serde_json::to_string(&ClientControl::Pong)
@@ -88,6 +94,8 @@ pub(crate) async fn serve(
         }
     }
 
+    manager.release_viewport(id, viewport).await;
+
     if session_ended {
         let _ = sink.send(Message::Close(None)).await;
     }

crates/maestro-daemon/src/lib.rs

@@ -30,6 +30,7 @@ pub mod spawn_history;
 #[cfg(feature = "web")]
 pub mod static_bundle;
 pub mod tmux;
+pub mod viewport;
 pub mod worktree;
 pub mod worktree_probe;
 

crates/maestro-daemon/src/session.rs

@@ -7,6 +7,7 @@
 
 use std::collections::{HashMap, HashSet};
 use std::path::PathBuf;
+use std::sync::atomic::{AtomicU64, Ordering};
 use std::sync::Arc;
 use std::time::Duration;
 
@@ -23,6 +24,7 @@ use crate::pty::{pipe_pane_command, PtyBridge};
 use crate::sidecar::{SidecarError, SidecarStore};
 use crate::spawn_history::{RecentDir, SpawnHistory, SpawnHistoryRecord};
 use crate::tmux::TmuxError;
+use crate::viewport::{ViewportId, Viewports};
 use crate::worktree::{WorktreeError, WorktreeManager};
 
 /// Capacity of the session-events broadcast channel. A subscriber that falls
@@ -166,6 +168,14 @@ struct Inner {
     report_ttl: Duration,
     sessions: Mutex<HashMap<Uuid, SessionRecord>>,
     events: broadcast::Sender<CatalogEvent>,
+    /// Sizes reported by the clients attached to each session, and what tmux
+    /// was last told. See [`crate::viewport`] for why the window is not simply
+    /// set to whatever arrived most recently.
+    viewports: Mutex<Viewports>,
+    /// Hands out [`ViewportId`]s. Monotonic for the daemon's lifetime, so a
+    /// client that reconnects never collides with the registration its
+    /// previous socket has not finished releasing.
+    next_viewport: AtomicU64,
 }
 
 impl SessionManager {
@@ -206,6 +216,8 @@ impl SessionManager {
                 report_ttl: crate::activity::REPORT_TTL,
                 sessions: Mutex::new(HashMap::new()),
                 events,
+                viewports: Mutex::new(Viewports::new()),
+                next_viewport: AtomicU64::new(0),
             }),
         }
     }
@@ -770,6 +782,52 @@ impl SessionManager {
             .map_err(SessionError::Tmux)
     }
 
+    /// Register a client that is about to start reporting its display size,
+    /// and hand back the handle it reports under.
+    ///
+    /// Cheap and infallible: the session it will report against is not
+    /// consulted until the first [`Self::set_viewport`].
+    pub fn attach_viewport(&self) -> ViewportId {
+        self.inner.next_viewport.fetch_add(1, Ordering::Relaxed)
+    }
+
+    /// A client reported the size it can display.
+    ///
+    /// The shared tmux window is resized only when the size arbitrated across
+    /// every attached client actually changes — see [`crate::viewport`]. Two
+    /// clients of different sizes used to resize the window past each other on
+    /// every report, which leaves a full-screen agent repainting into rows it
+    /// never clears and looking, to the operator, like a pane that has stopped
+    /// accepting input.
+    pub async fn set_viewport(
+        &self,
+        id: Uuid,
+        viewport: ViewportId,
+        cols: u16,
+        rows: u16,
+    ) -> Result<(), SessionError> {
+        let target = self
+            .inner
+            .viewports
+            .lock()
+            .await
+            .set(id, viewport, cols, rows);
+        match target {
+            Some((cols, rows)) => self.resize(id, cols, rows).await,
+            None => Ok(()),
+        }
+    }
+
+    /// Drop a detached client's claim on the window size, resizing to what the
+    /// clients still attached can display. Errors are the caller's to ignore:
+    /// a socket closing because its session died has nothing left to resize.
+    pub async fn release_viewport(&self, id: Uuid, viewport: ViewportId) {
+        let target = self.inner.viewports.lock().await.release(id, viewport);
+        if let Some((cols, rows)) = target {
+            let _ = self.resize(id, cols, rows).await;
+        }
+    }
+
     /// Rename a session (#125): set `metadata.name`, or clear it when the
     /// requested name is blank so the session falls back to its command.
     ///

crates/maestro-daemon/src/viewport.rs

@@ -0,0 +1,186 @@
+//! What size the shared tmux window should be, given every client watching it.
+//!
+//! A tmux window has one size, but a session can have several clients attached
+//! at once — two browser panes, a browser and the TUI. Each reports the size
+//! *it* can display, and applying whichever arrived last makes the window
+//! flip-flop. Every flip forces a full-screen application (Claude Code, vim) to
+//! repaint at a new height, and the rows it drew at the old height are never
+//! cleared: the operator ends up looking at a stale input box, types into a
+//! box that is not where the screen says it is, and concludes the pane is
+//! stuck.
+//!
+//! So the window is sized to the smallest attached client on each axis — the
+//! same rule tmux uses for its own multiple clients, and the only size every
+//! client can render without truncation. The size last handed to tmux is
+//! remembered, so a report that changes nothing costs no `tmux` fork at all.
+
+use std::collections::HashMap;
+
+use uuid::Uuid;
+
+/// One attached client within a session. Allocated by
+/// [`crate::session::SessionManager`] and opaque to everything else.
+pub type ViewportId = u64;
+
+/// Per-session client sizes, and what tmux was last told.
+#[derive(Debug, Default)]
+pub struct Viewports {
+    clients: HashMap<Uuid, HashMap<ViewportId, (u16, u16)>>,
+    applied: HashMap<Uuid, (u16, u16)>,
+}
+
+impl Viewports {
+    pub fn new() -> Self {
+        Self::default()
+    }
+
+    /// Record what `viewport` can display. Returns the size tmux must be set
+    /// to, or `None` when the window already has it.
+    pub fn set(
+        &mut self,
+        session: Uuid,
+        viewport: ViewportId,
+        cols: u16,
+        rows: u16,
+    ) -> Option<(u16, u16)> {
+        self.clients
+            .entry(session)
+            .or_default()
+            .insert(viewport, (cols, rows));
+        self.settle(session)
+    }
+
+    /// Drop a client that has detached. Returns the size tmux must be set to
+    /// now that it no longer constrains the window.
+    ///
+    /// The last client leaving settles nothing: the window keeps the size it
+    /// has rather than snapping to some remembered default no one is watching.
+    pub fn release(&mut self, session: Uuid, viewport: ViewportId) -> Option<(u16, u16)> {
+        let clients = self.clients.get_mut(&session)?;
+        clients.remove(&viewport);
+        if clients.is_empty() {
+            self.clients.remove(&session);
+            self.applied.remove(&session);
+            return None;
+        }
+        self.settle(session)
+    }
+
+    /// The smallest size every attached client can render, but only when it
+    /// differs from what tmux was last told.
+    ///
+    /// A zero on either axis is a client that cannot display anything yet — a
+    /// pane still laying out, a hidden tab — and must not drag the shared
+    /// window down with it, so it is held but left out of the arbitration.
+    fn settle(&mut self, session: Uuid) -> Option<(u16, u16)> {
+        let clients = self.clients.get(&session)?;
+        let mut cols = u16::MAX;
+        let mut rows = u16::MAX;
+        for &(c, r) in clients.values() {
+            if c == 0 || r == 0 {
+                continue;
+            }
+            cols = cols.min(c);
+            rows = rows.min(r);
+        }
+        if cols == u16::MAX || rows == u16::MAX {
+            return None;
+        }
+        let target = (cols, rows);
+        if self.applied.get(&session) == Some(&target) {
+            return None;
+        }
+        self.applied.insert(session, target);
+        Some(target)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn the_first_report_settles_the_window() {
+        let mut v = Viewports::new();
+        let s = Uuid::new_v4();
+        assert_eq!(v.set(s, 1, 80, 24), Some((80, 24)));
+    }
+
+    #[test]
+    fn a_report_that_changes_nothing_costs_no_resize() {
+        let mut v = Viewports::new();
+        let s = Uuid::new_v4();
+        v.set(s, 1, 80, 24);
+        assert_eq!(v.set(s, 1, 80, 24), None);
+        assert_eq!(v.set(s, 1, 80, 24), None);
+    }
+
+    /// The bug this module exists for: two clients of different sizes used to
+    /// resize the window past each other on every report.
+    #[test]
+    fn two_clients_settle_on_the_smaller_instead_of_fighting() {
+        let mut v = Viewports::new();
+        let s = Uuid::new_v4();
+        assert_eq!(v.set(s, 1, 53, 46), Some((53, 46)));
+        assert_eq!(v.set(s, 2, 53, 20), Some((53, 20)));
+        // Whatever order they re-report in, the answer stays the smaller one.
+        assert_eq!(v.set(s, 1, 53, 46), None);
+        assert_eq!(v.set(s, 2, 53, 20), None);
+        assert_eq!(v.set(s, 1, 53, 46), None);
+    }
+
+    #[test]
+    fn each_axis_is_arbitrated_on_its_own() {
+        let mut v = Viewports::new();
+        let s = Uuid::new_v4();
+        v.set(s, 1, 200, 20);
+        assert_eq!(v.set(s, 2, 53, 46), Some((53, 20)));
+    }
+
+    #[test]
+    fn a_zero_sized_client_does_not_shrink_the_window() {
+        let mut v = Viewports::new();
+        let s = Uuid::new_v4();
+        assert_eq!(v.set(s, 1, 80, 24), Some((80, 24)));
+        assert_eq!(v.set(s, 2, 0, 0), None);
+        assert_eq!(v.set(s, 2, 0, 30), None);
+    }
+
+    #[test]
+    fn releasing_the_small_client_gives_the_window_back() {
+        let mut v = Viewports::new();
+        let s = Uuid::new_v4();
+        v.set(s, 1, 80, 46);
+        v.set(s, 2, 80, 20);
+        assert_eq!(v.release(s, 2), Some((80, 46)));
+    }
+
+    #[test]
+    fn releasing_the_last_client_settles_nothing() {
+        let mut v = Viewports::new();
+        let s = Uuid::new_v4();
+        v.set(s, 1, 80, 24);
+        assert_eq!(v.release(s, 1), None);
+        // ...and the session is forgotten, so a fresh attach settles again.
+        assert_eq!(v.set(s, 2, 80, 24), Some((80, 24)));
+    }
+
+    #[test]
+    fn releasing_an_unknown_viewport_is_harmless() {
+        let mut v = Viewports::new();
+        let s = Uuid::new_v4();
+        assert_eq!(v.release(s, 7), None);
+        v.set(s, 1, 80, 24);
+        assert_eq!(v.release(s, 7), None);
+    }
+
+    #[test]
+    fn sessions_do_not_arbitrate_against_each_other() {
+        let mut v = Viewports::new();
+        let a = Uuid::new_v4();
+        let b = Uuid::new_v4();
+        assert_eq!(v.set(a, 1, 80, 24), Some((80, 24)));
+        assert_eq!(v.set(b, 2, 53, 20), Some((53, 20)));
+        assert_eq!(v.set(a, 1, 80, 24), None);
+    }
+}

crates/maestro-daemon/tests/session_decisions.rs

@@ -555,3 +555,63 @@ async fn spawn_overrides_client_supplied_maestro_variables() {
         Some(&"http://127.0.0.1:7070".to_string())
     );
 }
+
+/// Two clients attached to one session used to resize the shared window past
+/// each other on every report they made. Every flip repaints a full-screen
+/// agent at a new height and orphans the rows it drew at the old one, which
+/// reads to the operator as a pane that has stopped taking input.
+#[tokio::test]
+async fn clients_of_different_sizes_do_not_resize_the_window_past_each_other() {
+    let fx = Fx::new();
+    let s = fx.manager.create(fx.request("bash")).await.expect("create");
+    let web = fx.manager.attach_viewport();
+    let tui = fx.manager.attach_viewport();
+
+    fx.manager.set_viewport(s.id, web, 53, 46).await.unwrap();
+    fx.manager.set_viewport(s.id, tui, 53, 20).await.unwrap();
+    // Both keep reporting, as a browser's ResizeObserver and the TUI's own
+    // redraw loop do. Neither report changes the arbitrated size.
+    for _ in 0..5 {
+        fx.manager.set_viewport(s.id, web, 53, 46).await.unwrap();
+        fx.manager.set_viewport(s.id, tui, 53, 20).await.unwrap();
+    }
+
+    let resizes: Vec<_> = fx
+        .fake
+        .calls()
+        .into_iter()
+        .filter_map(|c| match c {
+            Call::ResizeWindow { cols, rows, .. } => Some((cols, rows)),
+            _ => None,
+        })
+        .collect();
+    assert_eq!(
+        resizes,
+        vec![(53, 46), (53, 20)],
+        "the window was resized more than the arbitrated size actually changed"
+    );
+}
+
+/// The smaller client leaving hands the window back to the one still attached.
+#[tokio::test]
+async fn releasing_a_client_resizes_to_what_is_left() {
+    let fx = Fx::new();
+    let s = fx.manager.create(fx.request("bash")).await.expect("create");
+    let web = fx.manager.attach_viewport();
+    let tui = fx.manager.attach_viewport();
+    fx.manager.set_viewport(s.id, web, 80, 46).await.unwrap();
+    fx.manager.set_viewport(s.id, tui, 80, 20).await.unwrap();
+
+    fx.manager.release_viewport(s.id, tui).await;
+
+    let resizes: Vec<_> = fx
+        .fake
+        .calls()
+        .into_iter()
+        .filter_map(|c| match c {
+            Call::ResizeWindow { cols, rows, .. } => Some((cols, rows)),
+            _ => None,
+        })
+        .collect();
+    assert_eq!(resizes, vec![(80, 46), (80, 20), (80, 46)]);
+}