diff --git a/src/daemon/protocol.rs b/src/daemon/protocol.rs index d59b68a6..281553fd 100644 --- a/src/daemon/protocol.rs +++ b/src/daemon/protocol.rs @@ -32,6 +32,29 @@ 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, 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 — +/// 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 +773,13 @@ 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 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, } /// Messages the daemon sends back to the GUI client. @@ -820,6 +850,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 +902,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 +937,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 +1084,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 +1160,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 +1224,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 +1271,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 +1489,7 @@ mod tests { forward_id: 3, }, ClientMsg::ListForwards { pane_id: 7 }, + ClientMsg::Version, ]; let mut buf = Vec::new(); for m in &msgs { @@ -1585,6 +1628,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..c768b296 100644 --- a/src/daemon/spawn.rs +++ b/src/daemon/spawn.rs @@ -15,11 +15,13 @@ //! 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}; 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 +31,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. @@ -41,30 +47,105 @@ 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` 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`. 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) { + 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(); + } + } + } 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 +169,34 @@ pub fn ensure_running() -> anyhow::Result<()> { } } +/// 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)); + if ClientMsg::Version + .encode(stream) + .and_then(|()| stream.flush()) + .is_err() + { + return VersionProbe::Unresponsive; + } + match DaemonMsg::read(stream) { + 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, + } +} + /// 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 +223,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 +588,72 @@ 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(); + }); + + 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 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_legacy() { + 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), 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/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 4b27799e..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. @@ -349,7 +355,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 @@ -382,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(); @@ -443,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(), @@ -631,6 +698,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 @@ -2793,29 +2871,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/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()); + } +} 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 7b09ac13..ed7e735f 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 @@ -579,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 @@ -609,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))