From 96360c544ee3caa1bce73442402ab2b85a9eafc5 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:45:59 +0800 Subject: [PATCH 1/6] feat(daemon): version handshake so an upgraded GUI restarts a stale daemon The daemon outlives the GUI binary, so after an app upgrade the running daemon can speak an older wire dialect. ensure_running now asks a live daemon for its protocol version (new Version request/reply, kind 40) before reusing it and restarts it on a mismatch; a pre-versioning daemon drops the unknown kind, which reads as "replace it" too. --- src/daemon/protocol.rs | 44 +++++++++++++++ src/daemon/server.rs | 12 +++- src/daemon/spawn.rs | 122 ++++++++++++++++++++++++++++++++++------- 3 files changed, 158 insertions(+), 20 deletions(-) diff --git a/src/daemon/protocol.rs b/src/daemon/protocol.rs index d59b68a6..2b7f1fa7 100644 --- a/src/daemon/protocol.rs +++ b/src/daemon/protocol.rs @@ -32,6 +32,27 @@ use serde::{Deserialize, Serialize}; /// protocol desync and we error rather than allocate. pub const MAX_FRAME: usize = 64 * 1024 * 1024; +/// Version of this wire protocol. The daemon outlives the GUI binary, so after +/// an app upgrade the two can be different builds; the GUI asks a running +/// daemon for its version (`ClientMsg::Version`) before reusing it and +/// restarts it on a mismatch (see `spawn::ensure_running`). +/// +/// Bump this on any change an old peer would *misread*: a repurposed kind +/// byte, a changed payload shape, altered framing. Purely additive changes — +/// a brand-new kind, a new `#[serde(default)]` field — don't need a bump; +/// the existing unknown-kind / missing-field behavior already covers them. +pub const PROTOCOL_VERSION: u32 = 1; + +/// Reply to `ClientMsg::Version`: the protocol dialect the daemon speaks, plus +/// its crate version for logs/diagnostics. Only `protocol` drives decisions. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct DaemonVersion { + pub protocol: u32, + /// The daemon binary's `CARGO_PKG_VERSION`. Display only. + #[serde(default)] + pub build: String, +} + /// Terminal geometry shared by spawn/attach/resize. Cell pixel size travels too /// so the daemon can set an accurate `TIOCSWINSZ` (`ws_xpixel`/`ws_ypixel`), /// which some full-screen apps read. @@ -750,6 +771,12 @@ pub enum ClientMsg { /// Ask for the managed forwards attributed to `pane_id`. Control-connection /// message; the daemon replies with a `ForwardList`. ListForwards { pane_id: u64 }, + /// Ask which protocol version the daemon speaks (control connection); the + /// daemon replies `Version`. A daemon that predates versioning doesn't know + /// this kind and drops the connection instead of replying — the client + /// reads any non-answer as "older than every versioned daemon" and + /// restarts it (see `spawn::ensure_running`). + Version, } /// Messages the daemon sends back to the GUI client. @@ -820,6 +847,8 @@ pub enum DaemonMsg { /// Reply to `AddForward` / `RemoveForward` / `ListForwards`: the managed /// forwards currently attributed to the requested pane (WS4). ForwardList(Vec), + /// Reply to `Version`. + Version(DaemonVersion), /// A request failed (e.g. `Attach` to an unknown/dead pane id). Error(String), } @@ -870,6 +899,9 @@ mod kind { pub const REMOVE_FORWARD: u8 = 21; /// `ListForwards` — list a pane's managed forwards (WS4). pub const LIST_FORWARDS: u8 = 22; + /// `Version` — protocol-version handshake. 40 sits clear of every reserved + /// range above (WS3 16–19, WS4 20–24, SFTP 30–36). + pub const VERSION: u8 = 40; // Daemon -> client pub const SPAWNED: u8 = 1; @@ -902,6 +934,9 @@ mod kind { pub const AGENT: u8 = 21; /// `AgentStatus` — the pane's rich agent-session status (or its clear). pub const AGENT_STATUS: u8 = 22; + /// `Version` — reply to the client-space `VERSION` request (same value by + /// design; the spaces are independent). + pub const VERSION_REPLY: u8 = 40; } /// Write one framed message: `[u32 LE len][u8 kind][payload]`. @@ -1046,6 +1081,7 @@ impl ClientMsg { ClientMsg::ListForwards { pane_id } => { write_frame(w, kind::LIST_FORWARDS, &to_json(pane_id)?) } + ClientMsg::Version => write_frame(w, kind::VERSION, &[]), } } @@ -1121,6 +1157,7 @@ impl ClientMsg { kind::LIST_FORWARDS => ClientMsg::ListForwards { pane_id: from_json(&payload)?, }, + kind::VERSION => ClientMsg::Version, other => { return Err(io::Error::new( io::ErrorKind::InvalidData, @@ -1184,6 +1221,7 @@ impl DaemonMsg { write_frame(w, kind::SFTP_TRANSFER_PROGRESS, &to_json(jobs)?) } DaemonMsg::ForwardList(list) => write_frame(w, kind::FORWARD_LIST, &to_json(list)?), + DaemonMsg::Version(version) => write_frame(w, kind::VERSION_REPLY, &to_json(version)?), DaemonMsg::Error(msg) => write_frame(w, kind::ERROR, &to_json(msg)?), } } @@ -1230,6 +1268,7 @@ impl DaemonMsg { }, kind::SFTP_TRANSFER_PROGRESS => DaemonMsg::SftpTransferProgress(from_json(&payload)?), kind::FORWARD_LIST => DaemonMsg::ForwardList(from_json(&payload)?), + kind::VERSION_REPLY => DaemonMsg::Version(from_json(&payload)?), kind::ERROR => DaemonMsg::Error(from_json(&payload)?), other => { return Err(io::Error::new( @@ -1447,6 +1486,7 @@ mod tests { forward_id: 3, }, ClientMsg::ListForwards { pane_id: 7 }, + ClientMsg::Version, ]; let mut buf = Vec::new(); for m in &msgs { @@ -1585,6 +1625,10 @@ mod tests { status: ForwardStatus::Error("bind refused".into()), }, ]), + DaemonMsg::Version(DaemonVersion { + protocol: PROTOCOL_VERSION, + build: "0.15.0".into(), + }), DaemonMsg::Error("nope".into()), ]; let mut buf = Vec::new(); diff --git a/src/daemon/server.rs b/src/daemon/server.rs index 53bdddce..a1c8895a 100644 --- a/src/daemon/server.rs +++ b/src/daemon/server.rs @@ -24,7 +24,7 @@ use std::sync::mpsc::{self, Receiver}; use std::sync::{Arc, Mutex}; use crate::daemon::pane::DaemonPane; -use crate::daemon::protocol::{ClientMsg, DaemonMsg, RemoteKind}; +use crate::daemon::protocol::{ClientMsg, DaemonMsg, DaemonVersion, RemoteKind, PROTOCOL_VERSION}; use crate::daemon::ssh::SshConnection; use crate::daemon::transport::{self, Stream}; @@ -330,6 +330,16 @@ fn handle_conn(stream: Stream, registry: Arc) -> anyhow::Result<()> { Ok(()) } + ClientMsg::Version => { + let mut w = write_stream; + DaemonMsg::Version(DaemonVersion { + protocol: PROTOCOL_VERSION, + build: env!("CARGO_PKG_VERSION").to_string(), + }) + .encode(&mut w)?; + Ok(()) + } + ClientMsg::Shutdown => { // Force a full daemon stop (the GUI's "Restart Background Service"): // hang up every child so nothing is orphaned, drop the endpoint diff --git a/src/daemon/spawn.rs b/src/daemon/spawn.rs index 4ac4a70e..8b185889 100644 --- a/src/daemon/spawn.rs +++ b/src/daemon/spawn.rs @@ -20,6 +20,7 @@ use std::process::{Command, Stdio}; use std::time::{Duration, Instant}; use crate::core::config; +use crate::daemon::protocol::{ClientMsg, DaemonMsg, DaemonVersion, PROTOCOL_VERSION}; use crate::daemon::{pidfile, transport}; /// How long to wait for a freshly spawned daemon to start listening before we @@ -29,6 +30,10 @@ use crate::daemon::{pidfile, transport}; const STARTUP_TIMEOUT: Duration = Duration::from_secs(3); /// Poll interval while waiting for the socket to come up. const POLL_INTERVAL: Duration = Duration::from_millis(50); +/// How long the version handshake with an already-running daemon may take. +/// Local socket, tiny reply — a daemon that can't answer within this is wedged +/// (or so old it dropped the connection), and gets replaced either way. +const HANDSHAKE_TIMEOUT: Duration = Duration::from_secs(2); /// How long to wait for the old daemon to exit after we ask it to shut down. /// Generous on purpose: the daemon hangs up every pane's child (a ~200 ms SIGHUP /// grace each) before it exits, so a session with several panes needs a moment. @@ -46,25 +51,45 @@ const REAP_KILL_TIMEOUT: Duration = Duration::from_secs(2); /// endpoint can't be resolved or the daemon never came up within /// [`STARTUP_TIMEOUT`]. pub fn ensure_running() -> anyhow::Result<()> { - // Fast path: a live daemon answers `connect` immediately. We only want to - // probe — drop the connection right away so we don't hold a pane open. - if transport::connect().is_ok() { - return Ok(()); - } + // Fast path: a live daemon answers `connect` — but only reuse it if it + // speaks our protocol version. The daemon outlives the GUI binary, so after + // an app upgrade the running daemon may be an older build; reusing one + // whose wire dialect differs would misdecode frames mid-session. A mismatch + // (or a daemon too old to know `Version` at all) means restart, not reuse. + if let Ok(mut stream) = transport::connect() { + match query_daemon_version(&mut stream) { + Some(v) if v.protocol == PROTOCOL_VERSION => return Ok(()), + Some(v) => log::info!( + "daemon (build {}) speaks protocol {}, this build needs {}; restarting it", + v.build, + v.protocol, + PROTOCOL_VERSION + ), + None => log::info!( + "daemon predates protocol versioning or is unresponsive; restarting it" + ), + } + drop(stream); + // `stop` shuts the old daemon down gracefully (`Shutdown` predates + // versioning, so even the oldest daemon honors it), escalating to a + // pid-based reap if it won't go, and clears the endpoint marker. + stop(); + } else { + // Nobody answers — but "unreachable" is not "gone". If the pidfile + // records a daemon that is still alive (wedged, or one whose endpoint + // was lost), its panes are already beyond reach; reap it before + // claiming the endpoint so it can't linger forever holding every + // pane's PTY and children. + reap_recorded_daemon(); - // Nobody answers — but "unreachable" is not "gone". If the pidfile records - // a daemon that is still alive (wedged, or one whose endpoint was lost), - // its panes are already beyond reach; reap it before claiming the endpoint - // so it can't linger forever holding every pane's PTY and children. - reap_recorded_daemon(); - - // If an endpoint marker is sitting there, it's a stale leftover from a - // crashed daemon (a *live* one would have answered the connect above), - // so clear it. The daemon's own `run()` clears stale endpoints too, but doing - // it here means our post-spawn polling connects on the first try instead of - // racing the daemon's cleanup. - if transport::endpoint_exists() { - transport::remove_stale_endpoint(); + // If an endpoint marker is sitting there, it's a stale leftover from a + // crashed daemon (a *live* one would have answered the connect above), + // so clear it. The daemon's own `run()` clears stale endpoints too, but + // doing it here means our post-spawn polling connects on the first try + // instead of racing the daemon's cleanup. + if transport::endpoint_exists() { + transport::remove_stale_endpoint(); + } } spawn_detached()?; @@ -88,6 +113,23 @@ pub fn ensure_running() -> anyhow::Result<()> { } } +/// Ask a freshly connected daemon which protocol version it speaks. `None` +/// covers every non-answer the same way: a daemon that predates +/// `ClientMsg::Version` (unknown kind → it drops the connection without +/// replying), a wedged daemon (read timeout), or a garbled reply. Callers +/// treat `None` as "must be replaced". +fn query_daemon_version(stream: &mut transport::Stream) -> Option { + use std::io::Write as _; + + let _ = stream.set_read_timeout(Some(HANDSHAKE_TIMEOUT)); + ClientMsg::Version.encode(stream).ok()?; + stream.flush().ok()?; + match DaemonMsg::read(stream) { + Ok(DaemonMsg::Version(v)) => Some(v), + _ => None, + } +} + /// Restart the daemon: ask the running one to shut down — which hangs up every /// live shell — wait for it to exit, then spawn a fresh one. Returns once the new /// daemon is listening. @@ -114,7 +156,6 @@ pub fn restart() -> anyhow::Result<()> { /// that same file, so Windows locks it until the daemon exits. Stopping it here /// releases the lock so the install/uninstall can overwrite/remove the binary. pub fn stop() { - use crate::daemon::protocol::ClientMsg; use std::io::Write as _; // Ask a running daemon to stop. Best effort: a failed connect/write means @@ -480,6 +521,49 @@ mod tests { assert!(!process_alive(pid)); } + /// The handshake against a current daemon: the peer answers `Version` and + /// the client reads it back. Driven over a socketpair so no real daemon is + /// needed — `query_daemon_version` only sees a `Stream`. + #[test] + fn version_handshake_reads_a_matching_reply() { + use crate::daemon::protocol::{ClientMsg, DaemonMsg, DaemonVersion, PROTOCOL_VERSION}; + + let (mut client, mut daemon) = UnixStream::pair().unwrap(); + let server = std::thread::spawn(move || { + let msg = ClientMsg::read(&mut daemon).unwrap(); + assert_eq!(msg, ClientMsg::Version); + DaemonMsg::Version(DaemonVersion { + protocol: PROTOCOL_VERSION, + build: "test".into(), + }) + .encode(&mut daemon) + .unwrap(); + }); + + let got = query_daemon_version(&mut client).expect("a live daemon must answer"); + assert_eq!(got.protocol, PROTOCOL_VERSION); + assert_eq!(got.build, "test"); + server.join().unwrap(); + } + + /// The handshake against a pre-versioning daemon: it reads an unknown kind + /// and drops the connection without replying. That must come back as + /// `None` — the signal to replace the daemon — not hang or panic. + #[test] + fn version_handshake_treats_a_hangup_as_no_version() { + use crate::daemon::protocol::ClientMsg; + + let (mut client, mut daemon) = UnixStream::pair().unwrap(); + let server = std::thread::spawn(move || { + // An old daemon errors on the unknown kind and closes the socket. + let _ = ClientMsg::read(&mut daemon); + drop(daemon); + }); + + assert_eq!(query_daemon_version(&mut client), None); + server.join().unwrap(); + } + /// A stale socket file (one nothing is listening on) must be treated as "not /// running": connecting to it fails, which is our trigger to clean up + spawn. /// We assert the failure kind so the stale-cleanup branch stays exercised even From 611b671a5d7d7fb7f826c5c68a5c46efe644a51a Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:45:59 +0800 Subject: [PATCH 2/6] fix(palette): stop offering QuickConnect rows for bare words A bare word like "java" parses as a valid hostname, so every command search got Connect/Save rows pinned above the real matches. Require the query to look like a connect target (contain '@', ':' or '.') before injecting QuickConnect rows. --- src/ui/palette.rs | 58 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/src/ui/palette.rs b/src/ui/palette.rs index c3b2e796..486f4bba 100644 --- a/src/ui/palette.rs +++ b/src/ui/palette.rs @@ -334,7 +334,17 @@ impl PaletteDelegate { } /// The QuickConnect rows for a query at the root, if it parses as a target. + /// + /// Beyond parsing, the query must *look like* a connect target — contain + /// `@`, `:` or `.` (`user@host`, `host:port`, an FQDN/IP; `ssh://` and + /// bracketed IPv6 both carry a `:`). A bare word like "java" parses as a + /// valid hostname too, but injecting these rows for every word would pin + /// them above all command searches; bare short names keep the SSH Connect + /// input as their path. fn quick_connect_commands(query: &str) -> Vec { + if !query.contains(['@', ':', '.']) { + return Vec::new(); + } match parse_quick_connect(query) { Some(_) => { let target = query.trim().to_string(); @@ -723,3 +733,51 @@ impl Render for PaletteView { .child(div().occlude().child(card)) } } + +#[cfg(test)] +mod tests { + use super::*; + + /// Titles of the QuickConnect rows injected for a root-palette query. + fn row_titles(query: &str) -> Vec { + PaletteDelegate::quick_connect_commands(query) + .into_iter() + .map(|c| c.title) + .collect() + } + + #[test] + fn bare_word_gets_no_quick_connect_rows() { + assert!(row_titles("java").is_empty()); + assert!(row_titles("split").is_empty()); + assert!(row_titles("").is_empty()); + } + + #[test] + fn host_like_queries_get_connect_and_save_rows() { + for q in [ + "deploy@10.0.0.5", + "host.example.com", + "java:2222", + "ssh://java", + "[::1]:2222", + ] { + let titles = row_titles(q); + assert_eq!( + titles, + vec![ + format!("Connect to \"{q}\""), + format!("Save \"{q}\" as profile…"), + ], + "query {q:?}" + ); + } + } + + #[test] + fn host_like_but_unparsable_gets_no_rows() { + // Contains ':' but the port segment is invalid → parse fails. + assert!(row_titles("java:99999").is_empty()); + assert!(row_titles("@").is_empty()); + } +} From 2029f2ccf769b95377ab4c6bfa2f4878c26fc744 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Wed, 15 Jul 2026 17:45:59 +0800 Subject: [PATCH 3/6] fix(ui): show SSH state as a corner status dot in semantic colors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SSH avatar drew a 2px border ring in theme tokens, which read as a second avatar shape next to the flat shell badge — and "connected" used theme.accent, the list-selection grey in this app, so the ring showed no state at all. Reuse the agent status_dot on the badge corner instead, colored from the same hardcoded palette as agent dots (amber connecting, green connected, red failed/disconnected, neutral for a foreground ssh); the tab strip's inline 6px dot picks up the same RGB values. --- src/ui/app.rs | 29 +++++++++++++++++------------ src/ui/tab_strip.rs | 30 +++++++++++++++++++++--------- 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/src/ui/app.rs b/src/ui/app.rs index 4b27799e..34638ca1 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -2793,29 +2793,34 @@ impl Tty7App { } /// The status-dot colour for a tab whose representative pane is an SSH - /// session (PRD FR-E2): native panes are phase-coloured (connecting = warning, - /// connected = accent, failed/disconnected = red); a foreground `ssh` typed - /// into a shell gets a plain neutral dot. `None` for non-SSH tabs (no dot). - pub(crate) fn tab_ssh_dot(&self, tab: &Tab, cx: &App) -> Option { + /// session (PRD FR-E2), as an RGB value from the same hardcoded semantic + /// palette as [`AgentStatus::dot_rgb`] — not the theme's UI tokens, which + /// in this app are soft neutral fills (accent is the list-selection grey) + /// and read as no state at all. Native panes are phase-coloured + /// (connecting = amber, connected = green, failed/disconnected = red); a + /// foreground `ssh` typed into a shell gets a plain neutral dot. `None` + /// for non-SSH tabs (no dot). + /// + /// [`AgentStatus::dot_rgb`]: crate::core::cli_agent::AgentStatus::dot_rgb + pub(crate) fn tab_ssh_dot(&self, tab: &Tab, cx: &App) -> Option { use crate::daemon::protocol::SshPhase; let leaf = tab.pane.first_leaf()?; let v = leaf.read(cx); - let theme = cx.theme(); if let Some(phase) = v.ssh_phase() { // Native pane. - let color = if v.ssh_disconnected() { - theme.danger + let rgb = if v.ssh_disconnected() { + 0xEF4444 // red: link lost } else { match phase { - SshPhase::Connecting | SshPhase::Authenticating => theme.warning, - SshPhase::Connected => theme.accent, - SshPhase::Failed { .. } => theme.danger, + SshPhase::Connecting | SshPhase::Authenticating => 0xF59E0B, // amber: in flight + SshPhase::Connected => 0x22C55E, // green: link up + SshPhase::Failed { .. } => 0xEF4444, // red: never made it } }; - Some(color) + Some(rgb) } else if v.remote_context().is_some() { // A foreground `ssh` typed into a shell: a plain neutral dot. - Some(theme.muted_foreground) + Some(0x9CA3AF) } else { None } diff --git a/src/ui/tab_strip.rs b/src/ui/tab_strip.rs index 7b09ac13..d10522ed 100644 --- a/src/ui/tab_strip.rs +++ b/src/ui/tab_strip.rs @@ -147,7 +147,8 @@ impl Render for DragTab { } impl Tty7App { - /// The status dot pinned to an agent avatar's bottom-right corner: a solid + /// The status dot pinned to a tab avatar's bottom-right corner (an agent's + /// live status, or an SSH pane's connection phase): a solid /// `rgb` disc with a surface-colored separator ring so it reads as sitting /// on the badge. When `unread` (a finished turn you haven't looked at), the /// dot gains a crisp outer ring of the same hue — the dot's separator ring @@ -204,16 +205,17 @@ impl Tty7App { /// The leading avatar for a tab row/chip: a rounded badge that brands the /// tab by what's running in it — each session fronted with an icon. A /// recognized coding agent gets its brand mark — a white silhouette - /// (gpui tints SVGs as an alpha mask) on the vendor accent; an SSH pane gets - /// a terminal glyph ringed in its connection-status colour; a plain shell - /// gets a neutral terminal glyph. An agent's live status rides the corner as - /// a [`status_dot`](Self::status_dot). `size` is the badge's edge in px. + /// (gpui tints SVGs as an alpha mask) on the vendor accent; a plain shell + /// gets a neutral terminal glyph. Live status rides the corner as a + /// [`status_dot`](Self::status_dot) — the agent's working/waiting/done, or + /// an SSH pane's connection phase (`ssh`) — one corner-dot language for + /// the whole avatar column. `size` is the badge's edge in px. pub(crate) fn tab_avatar( &self, agent: Option, status: Option, unread: bool, - ssh: Option, + ssh: Option, size: f32, cx: &App, ) -> gpui::AnyElement { @@ -251,11 +253,11 @@ impl Tty7App { .into_any_element() } None => base + .relative() .rounded_full() // A clearly-visible neutral disc (a neutral grey shell badge), not a // near-transparent tint — so the avatar column reads as a column. .bg(cx.theme().muted) - .when_some(ssh, |d, c| d.border_2().border_color(c)) .child( // A flush `>_` prompt (not the boxed `square-terminal`) so it // fills the badge at the same visual weight as a brand mark. @@ -264,6 +266,10 @@ impl Tty7App { .size(px(size * 0.56)) .text_color(cx.theme().foreground.opacity(0.65)), ) + // SSH connection phase as a corner status dot — the same + // element as an agent's, not a border ring around the badge + // (a ring read as a second, differently-shaped avatar style). + .when_some(ssh, |b, rgb| b.child(Self::status_dot(rgb, false, size, cx))) .into_any_element(), } } @@ -560,8 +566,14 @@ impl Tty7App { }), ) // Leading SSH status dot when this tab hosts an SSH session. - .when_some(ssh_dot, |c, color| { - c.child(div().flex_shrink_0().size(px(6.)).rounded_full().bg(color)) + .when_some(ssh_dot, |c, rgb| { + c.child( + div() + .flex_shrink_0() + .size(px(6.)) + .rounded_full() + .bg(gpui::rgb(rgb)), + ) }) // Leading agent brand avatar, when a coding agent runs in this // tab — the vendor mark on its accent. Only agents get an avatar From 35a16638f1fcdc6c40c78e9095c83e22ce46c808 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Wed, 15 Jul 2026 18:09:22 +0800 Subject: [PATCH 4/6] feat(daemon): ask before restarting a version-mismatched daemon MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Startup used to silently stop a daemon speaking a different protocol, killing every persisted session without warning. The old daemon is still serving its panes fine — the mismatch may be benign for the messages actually exercised — so the call is now the user's: keep it and reuse the sessions, record the mismatch, and have the first window raise a Keep Sessions / Restart Daemon prompt (restart reuses the confirmed half of the existing Restart Daemon flow). Only a daemon that cannot answer the handshake at all (wedged, timeout) is still replaced outright, since it cannot serve its sessions either way. --- src/daemon/protocol.rs | 11 ++- src/daemon/spawn.rs | 164 +++++++++++++++++++++++++++++++---------- src/ui/app.rs | 67 ++++++++++++++++- 3 files changed, 200 insertions(+), 42 deletions(-) diff --git a/src/daemon/protocol.rs b/src/daemon/protocol.rs index 2b7f1fa7..281553fd 100644 --- a/src/daemon/protocol.rs +++ b/src/daemon/protocol.rs @@ -34,8 +34,10 @@ pub const MAX_FRAME: usize = 64 * 1024 * 1024; /// Version of this wire protocol. The daemon outlives the GUI binary, so after /// an app upgrade the two can be different builds; the GUI asks a running -/// daemon for its version (`ClientMsg::Version`) before reusing it and -/// restarts it on a mismatch (see `spawn::ensure_running`). +/// daemon for its version (`ClientMsg::Version`) before reusing it and, on a +/// mismatch, keeps it alive but asks the user whether to keep their sessions +/// on the old dialect or restart the service clean (see +/// `spawn::ensure_running`). /// /// Bump this on any change an old peer would *misread*: a repurposed kind /// byte, a changed payload shape, altered framing. Purely additive changes — @@ -774,8 +776,9 @@ pub enum ClientMsg { /// Ask which protocol version the daemon speaks (control connection); the /// daemon replies `Version`. A daemon that predates versioning doesn't know /// this kind and drops the connection instead of replying — the client - /// reads any non-answer as "older than every versioned daemon" and - /// restarts it (see `spawn::ensure_running`). + /// reads that hangup as "older than every versioned daemon" and treats it + /// like any other mismatch: keep it, ask the user (see + /// `spawn::ensure_running`). Version, } diff --git a/src/daemon/spawn.rs b/src/daemon/spawn.rs index 8b185889..c768b296 100644 --- a/src/daemon/spawn.rs +++ b/src/daemon/spawn.rs @@ -15,6 +15,7 @@ //! Then we poll the endpoint until it's connectable, so the caller can immediately //! proceed to connect. +use std::io; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use std::time::{Duration, Instant}; @@ -46,34 +47,89 @@ const REAP_TERM_TIMEOUT: Duration = Duration::from_secs(6); #[cfg(any(target_os = "macos", target_os = "linux"))] const REAP_KILL_TIMEOUT: Duration = Duration::from_secs(2); +/// A live daemon `ensure_running` reused *despite* a protocol mismatch: killing +/// it would end every persisted session, and the mismatch may well be benign +/// for the messages actually exercised — so that call is the user's to make, +/// not startup's. Recorded here and consumed by the first window +/// ([`take_mismatched_daemon`]), which raises a keep-or-restart prompt. +pub struct MismatchedDaemon { + /// What the daemon answered, or `None` for one so old it predates the + /// `Version` request entirely. + pub version: Option, +} + +static MISMATCHED_DAEMON: std::sync::Mutex> = + std::sync::Mutex::new(None); + +/// The protocol mismatch recorded by [`ensure_running`] this launch, if any. +/// Take-semantics so the prompt fires once per launch, not per window. +pub fn take_mismatched_daemon() -> Option { + MISMATCHED_DAEMON.lock().ok()?.take() +} + +/// How a live daemon answered the version handshake. +#[derive(Debug, PartialEq, Eq)] +enum VersionProbe { + /// It replied: it knows the handshake, at this dialect. + Speaks(DaemonVersion), + /// It hung up (or answered garbage) — a daemon from before the `Version` + /// request existed errors on the unknown kind and drops the connection. + /// Alive and serving its panes fine; just an older dialect. + Legacy, + /// It kept the connection open but never answered within + /// [`HANDSHAKE_TIMEOUT`] (or the write itself failed): wedged. Unlike + /// `Legacy`, this daemon can't serve anything — replace it outright. + Unresponsive, +} + /// Ensure a daemon is running for this process's config dir, spawning a detached /// one if needed. Returns `Ok(())` once the endpoint is connectable; `Err` if the /// endpoint can't be resolved or the daemon never came up within /// [`STARTUP_TIMEOUT`]. pub fn ensure_running() -> anyhow::Result<()> { - // Fast path: a live daemon answers `connect` — but only reuse it if it - // speaks our protocol version. The daemon outlives the GUI binary, so after - // an app upgrade the running daemon may be an older build; reusing one - // whose wire dialect differs would misdecode frames mid-session. A mismatch - // (or a daemon too old to know `Version` at all) means restart, not reuse. + // Fast path: a live daemon answers `connect`. The daemon outlives the GUI + // binary, so after an app upgrade the running daemon may be an older build + // whose wire dialect differs. That daemon still holds every persisted + // session, so we don't kill it here: reuse it, record the mismatch, and let + // the first window ask the user whether to keep it or restart clean + // (`take_mismatched_daemon`). Only a daemon that can't answer at all — + // wedged mid-handshake — is replaced outright, since it can't serve its + // panes either way. if let Ok(mut stream) = transport::connect() { match query_daemon_version(&mut stream) { - Some(v) if v.protocol == PROTOCOL_VERSION => return Ok(()), - Some(v) => log::info!( - "daemon (build {}) speaks protocol {}, this build needs {}; restarting it", - v.build, - v.protocol, - PROTOCOL_VERSION - ), - None => log::info!( - "daemon predates protocol versioning or is unresponsive; restarting it" - ), + VersionProbe::Speaks(v) if v.protocol == PROTOCOL_VERSION => return Ok(()), + VersionProbe::Speaks(v) => { + log::warn!( + "daemon (build {}) speaks protocol {}, this build needs {}; \ + keeping it and deferring to the user", + v.build, + v.protocol, + PROTOCOL_VERSION + ); + if let Ok(mut slot) = MISMATCHED_DAEMON.lock() { + *slot = Some(MismatchedDaemon { version: Some(v) }); + } + return Ok(()); + } + VersionProbe::Legacy => { + log::warn!( + "daemon predates protocol versioning; keeping it and deferring to the user" + ); + if let Ok(mut slot) = MISMATCHED_DAEMON.lock() { + *slot = Some(MismatchedDaemon { version: None }); + } + return Ok(()); + } + VersionProbe::Unresponsive => { + log::info!("daemon did not answer the version handshake; restarting it"); + drop(stream); + // `stop` shuts the old daemon down gracefully (`Shutdown` + // predates versioning, so even the oldest daemon honors it), + // escalating to a pid-based reap if it won't go, and clears the + // endpoint marker. + stop(); + } } - drop(stream); - // `stop` shuts the old daemon down gracefully (`Shutdown` predates - // versioning, so even the oldest daemon honors it), escalating to a - // pid-based reap if it won't go, and clears the endpoint marker. - stop(); } else { // Nobody answers — but "unreachable" is not "gone". If the pidfile // records a daemon that is still alive (wedged, or one whose endpoint @@ -113,20 +169,31 @@ pub fn ensure_running() -> anyhow::Result<()> { } } -/// Ask a freshly connected daemon which protocol version it speaks. `None` -/// covers every non-answer the same way: a daemon that predates -/// `ClientMsg::Version` (unknown kind → it drops the connection without -/// replying), a wedged daemon (read timeout), or a garbled reply. Callers -/// treat `None` as "must be replaced". -fn query_daemon_version(stream: &mut transport::Stream) -> Option { +/// Ask a freshly connected daemon which protocol version it speaks, and +/// classify every way that can go (see [`VersionProbe`]). The split that +/// matters: a *hangup* is how a pre-versioning daemon reacts to the unknown +/// kind — it's healthy, keep it; a *timeout* is a daemon that can't process +/// messages at all — replace it. +fn query_daemon_version(stream: &mut transport::Stream) -> VersionProbe { use std::io::Write as _; let _ = stream.set_read_timeout(Some(HANDSHAKE_TIMEOUT)); - ClientMsg::Version.encode(stream).ok()?; - stream.flush().ok()?; + if ClientMsg::Version + .encode(stream) + .and_then(|()| stream.flush()) + .is_err() + { + return VersionProbe::Unresponsive; + } match DaemonMsg::read(stream) { - Ok(DaemonMsg::Version(v)) => Some(v), - _ => None, + Ok(DaemonMsg::Version(v)) => VersionProbe::Speaks(v), + Err(e) if matches!(e.kind(), io::ErrorKind::TimedOut | io::ErrorKind::WouldBlock) => { + VersionProbe::Unresponsive + } + // EOF/reset (the pre-versioning hangup) — and, conservatively, any + // other well-formed-but-unexpected reply: the daemon is alive enough + // to answer, so it stays the user's call. + _ => VersionProbe::Legacy, } } @@ -540,17 +607,22 @@ mod tests { .unwrap(); }); - let got = query_daemon_version(&mut client).expect("a live daemon must answer"); - assert_eq!(got.protocol, PROTOCOL_VERSION); - assert_eq!(got.build, "test"); + match query_daemon_version(&mut client) { + VersionProbe::Speaks(got) => { + assert_eq!(got.protocol, PROTOCOL_VERSION); + assert_eq!(got.build, "test"); + } + other => panic!("a live daemon must answer, got {other:?}"), + } server.join().unwrap(); } /// The handshake against a pre-versioning daemon: it reads an unknown kind - /// and drops the connection without replying. That must come back as - /// `None` — the signal to replace the daemon — not hang or panic. + /// and drops the connection without replying. That must classify as + /// `Legacy` — a healthy daemon on an older dialect, the user's call to + /// keep or replace — not hang, panic, or read as wedged. #[test] - fn version_handshake_treats_a_hangup_as_no_version() { + fn version_handshake_treats_a_hangup_as_legacy() { use crate::daemon::protocol::ClientMsg; let (mut client, mut daemon) = UnixStream::pair().unwrap(); @@ -560,10 +632,28 @@ mod tests { drop(daemon); }); - assert_eq!(query_daemon_version(&mut client), None); + assert_eq!(query_daemon_version(&mut client), VersionProbe::Legacy); server.join().unwrap(); } + /// The handshake against a wedged daemon: the peer accepts the request but + /// never answers. The read must time out ([`HANDSHAKE_TIMEOUT`]) and + /// classify as `Unresponsive` — the one case `ensure_running` replaces the + /// daemon without asking, since it can't serve its panes anyway. + #[test] + fn version_handshake_treats_silence_as_unresponsive() { + let (mut client, daemon) = UnixStream::pair().unwrap(); + // Keep the daemon end open (no reply, no hangup) until the client + // gives up. + let start = Instant::now(); + assert_eq!( + query_daemon_version(&mut client), + VersionProbe::Unresponsive + ); + assert!(start.elapsed() >= HANDSHAKE_TIMEOUT); + drop(daemon); + } + /// A stale socket file (one nothing is listening on) must be treated as "not /// running": connecting to it fails, which is our trigger to clean up + spawn. /// We assert the failure kind so the stale-cleanup branch stays exercised even diff --git a/src/ui/app.rs b/src/ui/app.rs index 34638ca1..1fdef968 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -349,7 +349,61 @@ impl Tty7App { } else { None }; - Self::with_session(session, window, cx) + let app = Self::with_session(session, window, cx); + // If startup reused a daemon that speaks a different wire protocol + // (an app upgrade while the old service kept running), the sessions + // just restored above are living on that old dialect. Surface the + // keep-or-restart choice now that there's a window to ask in. + Self::prompt_daemon_version_mismatch(window, cx); + app + } + + /// Ask what to do about a protocol-mismatched daemon that + /// `spawn::ensure_running` deliberately left running (rather than silently + /// killing every persisted session at startup): keep using it — sessions + /// survive, features whose wire shape changed may misbehave — or restart + /// the service clean via the shared + /// [`restart_daemon_confirmed`](Self::restart_daemon_confirmed) path + /// (tabs reopen with fresh shells). Keeping is the default: dismissing + /// the prompt changes nothing. + fn prompt_daemon_version_mismatch(window: &mut Window, cx: &mut Context) { + let Some(mismatch) = crate::daemon::spawn::take_mismatched_daemon() else { + return; + }; + let ours = crate::daemon::protocol::PROTOCOL_VERSION; + let detail = match mismatch.version { + Some(v) => format!( + "The daemon holding your sessions is from another build \ + (v{}, protocol {} — this app speaks {}). You can keep using it and \ + your sessions stay, but features whose wire format changed may \ + misbehave until it's restarted. Restarting starts a clean daemon: \ + tabs reopen with fresh shells and anything running in them is \ + terminated.", + v.build, v.protocol, ours + ), + None => "The daemon holding your sessions is from an older \ + version of the app. You can keep using it and your sessions stay, \ + but newer features may misbehave until it's restarted. Restarting \ + starts a clean daemon: tabs reopen with fresh shells and anything \ + running in them is terminated." + .to_string(), + }; + let answer = window.prompt( + PromptLevel::Warning, + "Daemon Is From Another Version", + Some(&detail), + &["Keep Sessions", "Restart Daemon"], + cx, + ); + cx.spawn(async move |this, cx| { + // Index 1 == "Restart Daemon"; "Keep Sessions" or a dismissed + // prompt leave the old daemon (and every session) untouched. + if !matches!(answer.await, Ok(1)) { + return; + } + let _ = this.update_in(cx, |this, _window, cx| this.restart_daemon_confirmed(cx)); + }) + .detach(); } /// The whole constructor behind `new`, with the saved session injected @@ -631,6 +685,17 @@ impl Tty7App { if !matches!(answer.await, Ok(1)) { return; } + let _ = this.update_in(cx, |this, _window, cx| this.restart_daemon_confirmed(cx)); + }) + .detach(); + } + + /// The restart itself, past any confirmation — shared by + /// [`restart_daemon`](Self::restart_daemon)'s prompt and the startup + /// version-mismatch prompt + /// ([`prompt_daemon_version_mismatch`](Self::prompt_daemon_version_mismatch)). + fn restart_daemon_confirmed(&mut self, cx: &mut Context) { + cx.spawn(async move |this, cx| { // Persist the current layout + cwds, then tear the live terminals down // *before* the daemon dies: dropping each `RemoteTerminal` detaches its // socket, so no reader thread is mid-read when the daemon exits. The From 0182a072cc00b0cfba98341cf596e12752c88905 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Wed, 15 Jul 2026 18:45:25 +0800 Subject: [PATCH 5/6] fix(git-status): share one per-repo snapshot across panes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each pane used to compute and hold its own git branch/diff snapshot, refreshed only by its own events (cwd change, command end, agent turn end). Tabs sitting idle in the same repo kept whatever they last saw, so rows for one directory showed different +/− counts — or none at all when a tab's last probe landed on a clean tree. Snapshots now live in a process-wide GitStatusCache keyed by work-tree root: every pane whose cwd resolves into the same repo reads the same entry, refreshed by whichever pane probed last, and the sidebar observes the cache so all rows repaint together. In-flight probes are deduped per cwd (concurrent triggers fold into one git shell-out, with a rerun if re-triggered mid-flight), and a failed git diff keeps the previous counts instead of rendering the tree as suddenly clean. --- src/terminal/git_status.rs | 222 ++++++++++++++++++++++++++++++++----- src/terminal/view.rs | 79 +++++++------ src/ui/app.rs | 19 +++- 3 files changed, 257 insertions(+), 63 deletions(-) diff --git a/src/terminal/git_status.rs b/src/terminal/git_status.rs index 234f9efe..f10fdfed 100644 --- a/src/terminal/git_status.rs +++ b/src/terminal/git_status.rs @@ -3,17 +3,25 @@ //! line (`⎇ feat/x +6 −5`): each session fronted with its branch and change //! count. //! +//! Snapshots are shared through [`GitStatusCache`], a process-wide map keyed +//! by work-tree root: every pane whose cwd resolves into the same repo reads +//! the *same* entry, so ten tabs in one repo show one truth, refreshed by +//! whichever pane probed last — not ten drifting copies refreshed on ten +//! different schedules. Probes stay per-trigger (a pane's cwd change, command +//! end, or agent-turn end — see [`crate::terminal::view`]) but are deduped +//! in-flight, so simultaneous triggers from panes in the same directory cost +//! one `git` shell-out, not one per pane. +//! //! Deliberately shell-out simple: one `git` invocation per field, run on a -//! background thread by the caller (see [`crate::terminal::view`]) so the UI -//! never blocks on a slow repo. Read-only — `GIT_OPTIONAL_LOCKS=0` keeps status -//! polling from ever taking `index.lock` and fighting a real git command the -//! user is running. Returns `None` when the cwd isn't inside a git work tree, -//! so the sidebar simply omits the line. +//! background thread by the caller so the UI never blocks on a slow repo. +//! Read-only — `GIT_OPTIONAL_LOCKS=0` keeps status polling from ever taking +//! `index.lock` and fighting a real git command the user is running. -use std::path::Path; +use std::collections::{HashMap, HashSet}; +use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; -/// A pane's git snapshot: the branch it's on and how much the working tree has +/// A repo's git snapshot: the branch it's on and how much the working tree has /// changed against `HEAD`. `added`/`removed` sum the per-file line counts from /// `git diff --numstat HEAD` (tracked staged + unstaged changes); binary files /// and untracked files don't contribute a line count. @@ -28,24 +36,120 @@ pub struct GitStatus { pub removed: u32, } -/// Compute the git snapshot for `cwd`, or `None` when it isn't a git work tree -/// (or the path is gone). Blocking — call it on a background executor. -pub fn compute(cwd: &Path) -> Option { +/// One raw probe result, before it's folded into the cache: which work tree +/// `cwd` belongs to, plus the fields probed there. `counts` is `None` when the +/// `git diff` invocation itself failed (e.g. it raced a concurrent git write) — +/// distinct from a clean tree's `Some((0, 0))`, so the cache can keep the +/// previous numbers instead of pretending the tree went clean. +#[derive(Clone, PartialEq, Eq, Debug)] +pub struct RepoSnapshot { + /// The work tree root (`git rev-parse --show-toplevel`) — the cache key + /// every pane inside this repo shares. + pub root: PathBuf, + pub branch: String, + pub counts: Option<(u32, u32)>, +} + +/// Probe the git snapshot for `cwd`, or `None` when it isn't inside a git work +/// tree (or the path is gone). Blocking — call it on a background executor. +pub fn probe(cwd: &Path) -> Option { if !cwd.exists() { return None; } + // Doubles as the "is this a git repo" gate: fails outside a work tree. + let root = git(cwd, &["rev-parse", "--show-toplevel"])?; + let root = PathBuf::from(root.trim_end_matches(['\n', '\r'])); let branch = branch_name(cwd)?; - let (added, removed) = diff_numstat(cwd).unwrap_or((0, 0)); - Some(GitStatus { + Some(RepoSnapshot { + root, branch, - added, - removed, + counts: diff_numstat(cwd), }) } -/// The current branch name, or a short sha for a detached HEAD. Doubles as the -/// "is this a git repo" gate: both probes failing (not a work tree) yields -/// `None`. +/// The process-wide snapshot store (a gpui [`Global`](gpui::Global)): pane +/// cwds grouped by work-tree root, one [`GitStatus`] per root. Views read +/// through [`status_for`](Self::status_for); the probe loop in +/// [`crate::terminal::view`] brackets each background probe with +/// [`begin_probe`](Self::begin_probe) / [`finish_probe`](Self::finish_probe). +/// +/// In-flight dedup is keyed by cwd (the root isn't known until a first probe +/// answers), so two panes at the same directory share one probe; panes in +/// *different* subdirectories of one repo can still race a redundant probe — +/// rare, and both land the same answer. +#[derive(Default)] +pub struct GitStatusCache { + /// cwd → its work-tree root; `None` = probed and found not to be a repo. + roots: HashMap>, + /// root → the snapshot every pane in that tree shares. + status: HashMap, + /// cwds with a probe currently in flight, so concurrent triggers fold + /// into one shell-out. + in_flight: HashSet, + /// In-flight cwds re-triggered meanwhile — reprobed once their flight + /// lands, so the newest trigger's state is never skipped. + dirty: HashSet, +} + +impl gpui::Global for GitStatusCache {} + +impl GitStatusCache { + /// The snapshot for a pane at `cwd`: resolved through its work-tree root, + /// so every pane in the same repo answers identically. `None` before the + /// first probe lands or when `cwd` isn't in a repo. + pub fn status_for(&self, cwd: &Path) -> Option { + let root = self.roots.get(cwd)?.as_ref()?; + self.status.get(root).cloned() + } + + /// Claim a probe for `cwd`. `false` means one is already in flight — the + /// caller must *not* spawn another; the landed flight will reprobe once + /// (the cwd is marked dirty) so this trigger's state still gets observed. + pub fn begin_probe(&mut self, cwd: &Path) -> bool { + if self.in_flight.contains(cwd) { + self.dirty.insert(cwd.to_path_buf()); + false + } else { + self.in_flight.insert(cwd.to_path_buf()); + true + } + } + + /// Fold a landed probe for `cwd` into the cache. A failed diff inside a + /// live repo keeps the root's previous counts (a transient `git` error is + /// not "the tree went clean"). Returns whether the cwd was re-triggered + /// while this probe flew — the caller should start one more probe. + pub fn finish_probe(&mut self, cwd: &Path, snapshot: Option) -> bool { + self.in_flight.remove(cwd); + match snapshot { + Some(snap) => { + let (added, removed) = snap.counts.unwrap_or_else(|| { + self.status + .get(&snap.root) + .map(|g| (g.added, g.removed)) + .unwrap_or((0, 0)) + }); + self.status.insert( + snap.root.clone(), + GitStatus { + branch: snap.branch, + added, + removed, + }, + ); + self.roots.insert(cwd.to_path_buf(), Some(snap.root)); + } + // Not a repo (or the dir vanished). The root's entry stays for + // other cwds that still live in it. + None => { + self.roots.insert(cwd.to_path_buf(), None); + } + } + self.dirty.remove(cwd) + } +} + +/// The current branch name, or a short sha for a detached HEAD. fn branch_name(cwd: &Path) -> Option { // On a branch — even before the first commit — `symbolic-ref` names it. if let Some(out) = git(cwd, &["symbolic-ref", "--quiet", "--short", "HEAD"]) { @@ -62,6 +166,7 @@ fn branch_name(cwd: &Path) -> Option { /// Sum added/removed lines across the working tree vs `HEAD` from /// `git diff --numstat HEAD`. Binary files (`-\t-`) contribute nothing. +/// `None` when the invocation itself failed — the caller keeps old counts. fn diff_numstat(cwd: &Path) -> Option<(u32, u32)> { let out = git(cwd, &["diff", "--numstat", "HEAD"])?; let mut added = 0u32; @@ -102,29 +207,94 @@ fn git(cwd: &Path, args: &[&str]) -> Option { mod tests { use super::*; - /// A tmp path that is not a git repo yields no status (and never panics). + /// A tmp path that is not a git repo yields no snapshot (and never panics). #[test] fn non_repo_is_none() { let dir = std::env::temp_dir().join("tty7-git-status-not-a-repo-xyz"); let _ = std::fs::create_dir_all(&dir); - assert_eq!(compute(&dir), None); + assert_eq!(probe(&dir), None); } /// A path that doesn't exist is `None`, not a panic. #[test] fn missing_path_is_none() { - assert_eq!(compute(Path::new("/no/such/tty7/path/here")), None); + assert_eq!(probe(Path::new("/no/such/tty7/path/here")), None); } - /// This repo (the crate root is inside the tty7 work tree) reports a branch, - /// exercising the real `git` probe end-to-end. + /// This repo (the crate root is inside the tty7 work tree) reports a branch + /// and a root, exercising the real `git` probe end-to-end. #[test] - fn own_repo_has_a_branch() { + fn own_repo_has_a_branch_and_root() { let here = env!("CARGO_MANIFEST_DIR"); - if let Some(status) = compute(Path::new(here)) { - assert!(!status.branch.is_empty()); + if let Some(snap) = probe(Path::new(here)) { + assert!(!snap.branch.is_empty()); + assert!(Path::new(here).starts_with(&snap.root)); } // If the crate is built outside a work tree (e.g. a vendored tarball), - // `None` is the correct answer and the assertion above is skipped. + // `None` is the correct answer and the assertions above are skipped. + } + + fn snap(root: &str, branch: &str, counts: Option<(u32, u32)>) -> RepoSnapshot { + RepoSnapshot { + root: PathBuf::from(root), + branch: branch.into(), + counts, + } + } + + /// Two cwds landing in the same work tree share one entry: a probe from + /// either updates what both read (the group-by-root contract). + #[test] + fn cwds_in_one_repo_share_a_snapshot() { + let mut cache = GitStatusCache::default(); + let (a, b) = (Path::new("/repo/sub/a"), Path::new("/repo")); + cache.finish_probe(a, Some(snap("/repo", "main", Some((5, 2))))); + cache.finish_probe(b, Some(snap("/repo", "main", Some((5, 2))))); + // A later probe from `a` refreshes the numbers `b` reads too. + cache.finish_probe(a, Some(snap("/repo", "main", Some((200, 42))))); + for cwd in [a, b] { + let got = cache.status_for(cwd).unwrap(); + assert_eq!((got.added, got.removed), (200, 42), "cwd {cwd:?}"); + } + } + + /// A failed `git diff` (counts `None`) keeps the previous numbers rather + /// than rendering the tree as suddenly clean; the branch still updates. + #[test] + fn failed_diff_keeps_previous_counts() { + let mut cache = GitStatusCache::default(); + let cwd = Path::new("/repo"); + cache.finish_probe(cwd, Some(snap("/repo", "main", Some((200, 42))))); + cache.finish_probe(cwd, Some(snap("/repo", "feat/x", None))); + let got = cache.status_for(cwd).unwrap(); + assert_eq!(got.branch, "feat/x"); + assert_eq!((got.added, got.removed), (200, 42)); + } + + /// In-flight dedup: a second trigger while a probe flies doesn't claim a + /// new one, but marks the cwd dirty so the landing reports "go again". + #[test] + fn concurrent_triggers_fold_into_one_probe_then_rerun() { + let mut cache = GitStatusCache::default(); + let cwd = Path::new("/repo"); + assert!(cache.begin_probe(cwd)); + assert!(!cache.begin_probe(cwd)); // deduped, marked dirty + assert!(cache.finish_probe(cwd, Some(snap("/repo", "main", Some((1, 0)))))); + // The rerun claims cleanly and lands with nothing pending. + assert!(cache.begin_probe(cwd)); + assert!(!cache.finish_probe(cwd, Some(snap("/repo", "main", Some((1, 0)))))); + } + + /// A cwd that leaves the repo (dir deleted / not a work tree) stops + /// answering, without disturbing the root entry other cwds still use. + #[test] + fn non_repo_cwd_clears_only_itself() { + let mut cache = GitStatusCache::default(); + let (a, b) = (Path::new("/repo/a"), Path::new("/repo/b")); + cache.finish_probe(a, Some(snap("/repo", "main", Some((3, 1))))); + cache.finish_probe(b, Some(snap("/repo", "main", Some((3, 1))))); + cache.finish_probe(a, None); + assert_eq!(cache.status_for(a), None); + assert!(cache.status_for(b).is_some()); } } diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 8bcb27f4..74c90909 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -247,19 +247,13 @@ pub struct TerminalView { /// while this is true, so a result you've already seen stops nagging. Blue /// (working) / amber (waiting) are unaffected — they track live state. agent_result_unread: bool, - /// The pane's last-computed git snapshot (branch + working-tree diff size), - /// shown as the sidebar row's third line. Computed off-thread by - /// [`refresh_git_status`](Self::refresh_git_status) on a cwd change or a - /// command finishing; `None` outside a git work tree (or before the first - /// probe lands). - git_status: Option, - /// The cwd `git_status` was last computed (or scheduled) for, so the poll - /// loop only reprobes when the working directory actually changes. + /// The cwd this pane's git line reads from (and last scheduled a probe + /// for), so the poll loop only reprobes when the working directory + /// actually changes. The snapshot itself lives in the process-wide + /// [`GitStatusCache`](crate::terminal::git_status::GitStatusCache), keyed + /// by work-tree root — panes in one repo share one entry instead of each + /// computing (and staling) its own. git_status_cwd: Option, - /// Monotonic tag bumped on every git reprobe; a background result is dropped - /// unless it still matches, so a slow probe from a since-changed cwd can't - /// overwrite a fresher one (same guard as `completion_generation`). - git_status_gen: u64, /// The inline command line editor. Live only while the shell sits idle /// at its prompt (`input_active`): there the terminal keeps keyboard focus and /// we run our own line editor (so we own Tab / ↑ / ↓ for completion and @@ -950,9 +944,7 @@ impl TerminalView { agent_turn_started: None, agent_was_rich: false, agent_result_unread: false, - git_status: None, git_status_cwd: None, - git_status_gen: 0, cmd: CmdEditor::new(), typeahead: Typeahead::new(), hold: GapHold::new(), @@ -1035,11 +1027,15 @@ impl TerminalView { self.agent_result_unread } - /// The pane's last-computed git snapshot (branch + working-tree diff), for - /// the sidebar row's branch line. `None` outside a git work tree or before - /// the first background probe lands. - pub fn git_status(&self) -> Option { - self.git_status.clone() + /// The git snapshot for this pane's cwd (branch + working-tree diff), for + /// the sidebar row's branch line — read from the shared per-repo + /// [`GitStatusCache`](crate::terminal::git_status::GitStatusCache), so + /// every pane in one work tree reports the same numbers. `None` outside a + /// git work tree or before the repo's first background probe lands. + pub fn git_status(&self, cx: &App) -> Option { + let cwd = self.git_status_cwd.as_ref()?; + cx.try_global::()? + .status_for(cwd) } /// The current grid selection as text, if any non-blank one exists — the @@ -2456,33 +2452,48 @@ impl TerminalView { } } - /// Kick off an off-thread git probe for `cwd` and fold the result back on - /// the main thread, tagged with a generation so a stale probe (cwd changed - /// meanwhile) is dropped. Clears the status when there's no cwd (e.g. a - /// native-SSH pane pre-OSC-7, where a local `git` would be meaningless). + /// Kick off an off-thread git probe for `cwd` and fold the result into the + /// shared per-repo [`GitStatusCache`] on the main thread. The cache + /// brackets the flight (`begin_probe`/`finish_probe`): a probe already in + /// flight for the same cwd absorbs this trigger instead of spawning a + /// duplicate `git` shell-out, and reruns once when it lands. With no cwd + /// (e.g. a native-SSH pane pre-OSC-7, where a local `git` would be + /// meaningless) the pane simply stops reading a status. + /// + /// [`GitStatusCache`]: crate::terminal::git_status::GitStatusCache fn refresh_git_status(&mut self, cwd: Option, cx: &mut Context) { + use crate::terminal::git_status::GitStatusCache; + + let changed = self.git_status_cwd != cwd; self.git_status_cwd = cwd.clone(); let Some(cwd) = cwd else { - if self.git_status.take().is_some() { + if changed { cx.notify(); } return; }; - self.git_status_gen += 1; - let generation = self.git_status_gen; + cx.default_global::(); // first probe of the process creates it + if !cx.update_global::(|cache, _| cache.begin_probe(&cwd)) { + return; + } cx.spawn(async move |this, cx| { let result = cx .background_executor() - .spawn(async move { crate::terminal::git_status::compute(&cwd) }) + .spawn({ + let cwd = cwd.clone(); + async move { crate::terminal::git_status::probe(&cwd) } + }) .await; let _ = this.update(cx, |view, cx| { - // Drop a probe whose cwd has since been superseded. - if view.git_status_gen != generation { - return; - } - if view.git_status != result { - view.git_status = result; - cx.notify(); + // Landing through `update_global` wakes the sidebar's + // `observe_global`, so every pane in the repo repaints — not + // just this one. + let rerun = cx + .update_global::(|cache, _| cache.finish_probe(&cwd, result)); + // A trigger arrived while we flew; go once more so its state + // is observed — unless this pane has since left that cwd. + if rerun && view.git_status_cwd.as_deref() == Some(&cwd) { + view.refresh_git_status(Some(cwd), cx); } }); }) diff --git a/src/ui/app.rs b/src/ui/app.rs index 1fdef968..30b435d8 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -134,8 +134,9 @@ impl Tab { /// The git snapshot (branch + working-tree diff) of the tab's label-driving /// terminal — the focused leaf with a `window`, else the first — for the /// sidebar row's branch line (the branch and change count shown under the - /// title). `None` when that leaf isn't inside a git work tree, or before - /// its first probe lands. + /// title). Read through the shared per-repo cache, so tabs in one work + /// tree always agree. `None` when that leaf isn't inside a git work tree, + /// or before the repo's first probe lands. pub(crate) fn git_status( &self, window: Option<&Window>, @@ -145,7 +146,7 @@ impl Tab { Some(window) => self.pane.focused_or_first(window, cx), None => self.pane.first_leaf(), }?; - leaf.read(cx).git_status() + leaf.read(cx).git_status(cx) } /// The coding agent running in this tab, or `None`. Any leaf counts (a @@ -262,6 +263,11 @@ pub struct Tty7App { /// by then — this window never gets that `ModifiersChanged`, so without /// this the badges stuck on until some later keypress. Never read. _activation_watch: Subscription, + /// Keeps the `observe_global::` subscription alive: a git + /// probe landing (from *any* pane) repaints the sidebar, so every row in + /// the same repo shows the just-refreshed branch/diff line, not a stale + /// per-row copy. Never read. + _git_status_watch: Subscription, /// `Some` while the command palette overlay is open; `None` when closed. /// The view owns its search input, filtered list and keyboard handling and /// emits a `PaletteEvent`; we build the catalog and run the chosen command. @@ -436,6 +442,12 @@ impl Tty7App { // and colors are handled separately by `apply_theme`; here we cover the // font knobs that live on `Tty7App`/the panes. let config_watch = cx.observe_global::(|this, cx| this.reload_from_config(cx)); + // Repaint when any pane's git probe lands in the shared cache — the + // sidebar's branch/diff lines read from it, and the probing pane's own + // notify wouldn't re-render rows belonging to *other* panes. + cx.default_global::(); + let git_status_watch = cx + .observe_global::(|_, cx| cx.notify()); // Any real keypress means "chord, not a bare hold": cancel the held-⌘ // tab badges and whatever reveal is pending (see `ui::hints`). let this = cx.weak_entity(); @@ -497,6 +509,7 @@ impl Tty7App { _config_watch: config_watch, _keystroke_watch: keystroke_watch, _activation_watch: activation_watch, + _git_status_watch: git_status_watch, palette: None, palette_sub: None, closed: Vec::new(), From dc89af952d3ad021a2150590231fc8d109ab590d Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Wed, 15 Jul 2026 18:53:47 +0800 Subject: [PATCH 6/6] style(tabs): hide the close affordance until hover on active tabs too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The active tab row/chip used to keep its × always visible while the others faded in on hover. Treat every tab the same — opacity 0 until hover, space still reserved — so the sidebar and strip read clean. --- src/ui/tab_sidebar.rs | 14 ++++++-------- src/ui/tab_strip.rs | 20 +++++++++----------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/ui/tab_sidebar.rs b/src/ui/tab_sidebar.rs index a94d7e35..a34869dd 100644 --- a/src/ui/tab_sidebar.rs +++ b/src/ui/tab_sidebar.rs @@ -253,9 +253,9 @@ impl Tty7App { .child(self.tab_avatar(agent, agent_status, agent_unread, ssh_dot, 22., cx)) .child(label_region) // Trailing slot: while the shortcut hints are armed it shows the - // row's ⌘N switch digit; otherwise the close affordance — always - // shown on the active row, opacity-0-until-hover on the others so - // a column of tabs reads clean. Space is reserved either way. + // row's ⌘N switch digit; otherwise the close affordance — + // opacity-0-until-hover on every row, active or not, so a column + // of tabs reads clean. Space is reserved either way. .child(if show_badges && i < 9 { // Bare digit, no keycap box — matches the chip badge exactly. div() @@ -276,11 +276,9 @@ impl Tty7App { } else { div() .flex_shrink_0() - .when(!is_active, |s| { - s.opacity(0.) - .group_hover(SharedString::from(format!("tab-row-{i}")), |s| { - s.opacity(1.) - }) + .opacity(0.) + .group_hover(SharedString::from(format!("tab-row-{i}")), |s| { + s.opacity(1.) }) .child( Button::new(("sidebar-close", i)) diff --git a/src/ui/tab_strip.rs b/src/ui/tab_strip.rs index d10522ed..ed7e735f 100644 --- a/src/ui/tab_strip.rs +++ b/src/ui/tab_strip.rs @@ -591,12 +591,12 @@ impl Tty7App { }) // Clickable / editable label region. .child(label_region) - // Trailing slot: normally the close affordance — always shown on - // the active tab; on the others it stays out of the way - // (opacity 0) and fades in on chip hover, so a row of tabs reads - // clean instead of three-icons-per-chip busy. Space is reserved - // either way, so nothing shifts on hover. While the shortcut - // hints are armed, the same slot shows the tab's ⌘N badge instead. + // Trailing slot: normally the close affordance — kept out of the + // way (opacity 0) on every chip, active or not, and fades in on + // chip hover, so a row of tabs reads clean instead of + // three-icons-per-chip busy. Space is reserved either way, so + // nothing shifts on hover. While the shortcut hints are armed, + // the same slot shows the tab's ⌘N badge instead. .child(if show_badges && i < 9 { // Bare digit, no keycap box — the hint blends into the chip // rather than reading as another button. Sized to the exact @@ -621,11 +621,9 @@ impl Tty7App { } else { div() .flex_shrink_0() - .when(!is_active, |s| { - s.opacity(0.) - .group_hover(SharedString::from(format!("tab-chip-{i}")), |s| { - s.opacity(1.) - }) + .opacity(0.) + .group_hover(SharedString::from(format!("tab-chip-{i}")), |s| { + s.opacity(1.) }) .child( Button::new(("tab-close", i))