From 8955b1545f8deb5c36e0e1d7f53186127cf93072 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Thu, 23 Jul 2026 16:17:32 +0800 Subject: [PATCH 1/2] fix(git-status): refresh the sidebar counts on window focus and tool calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sidebar's `+N -N` only refreshed on three rare edges: the pane changing directory, a command ending, and an agent turn ending. Edits made anywhere else produced no signal at all, so the counts sat stale — a long agent turn showed nothing until it finished minutes later, and a file edited in another editor never registered until the user happened to run a command in the pane. Two new triggers close the gap: - Window activation re-probes every pane. Coming back to the window is the only cue we get that the tree moved while the user was elsewhere, and the sidebar lists every tab, so refreshing just the focused pane isn't enough. - An agent's tool completions re-probe mid-turn. `AgentSessionState` gains an `activity` counter because `ToolComplete` is deliberately a status no-op during normal work, leaving status-watchers unable to see it. Both go through a new throttled claim on `GitStatusCache` that drops triggers instead of queueing them, so a busy agent or a window full of panes collapses into one shell-out per repo per 1.5s rather than a `git` storm. Also: fold the probe's two `rev-parse` calls into one (it now asks for toplevel, git-dir and common-dir together), which makes `repo_home` a pure function and unit-testable; and land probe results in the shared cache independently of the pane entity, so a pane closed mid-probe can't wedge the cwd-keyed in-flight claim for every other pane in that directory. --- src/core/cli_agent.rs | 57 +++++++++++++++ src/daemon/pane.rs | 1 + src/daemon/protocol.rs | 1 + src/terminal/git_status.rs | 144 ++++++++++++++++++++++++++++++------- src/terminal/remote.rs | 1 + src/terminal/view.rs | 118 +++++++++++++++++++++++++----- src/ui/app.rs | 26 ++++++- 7 files changed, 304 insertions(+), 44 deletions(-) diff --git a/src/core/cli_agent.rs b/src/core/cli_agent.rs index 5c0d9c04..f58fb9a5 100644 --- a/src/core/cli_agent.rs +++ b/src/core/cli_agent.rs @@ -631,6 +631,16 @@ pub struct AgentSessionState { /// a stale path; while absent, consumers fall back to the pane's proc cwd. #[serde(default)] pub cwd: Option, + /// Tool completions seen in this session, counted only so consumers can + /// spot *that* the agent did something — a turn's edits land tool by tool, + /// and the status alone can't say so (`ToolComplete` is a no-op transition + /// during normal work, by design). The sidebar's git probe watches this to + /// refresh mid-turn instead of waiting for `stop`; see + /// [`TerminalView::refresh_git_status`](crate::terminal::view::TerminalView). + /// Monotonic within a session and never reset — consumers compare against + /// the value they last saw, so only the *change* means anything. + #[serde(default)] + pub activity: u64, } impl AgentStatus { @@ -698,6 +708,10 @@ impl AgentSessionState { // stream of completions during normal work is a no-op and can // never overwrite Done between turns. AgentEventKind::ToolComplete => { + // The count moves even when the status doesn't: a tool call is + // the one signal that the working tree may have just changed + // under a turn that won't end for minutes. + self.activity = self.activity.wrapping_add(1); if self.status == AgentStatus::Waiting { self.status = AgentStatus::Working; self.message = None; @@ -1103,6 +1117,49 @@ mod tests { assert_eq!(s.session_id.as_deref(), Some("sid-1")); } + /// Tool completions are deliberately a *status* no-op during normal work + /// (the assertions above), which leaves consumers watching the status with + /// no way to tell that an agent mid-turn just wrote a file. `activity` is + /// what makes them observable: it moves on every completion, in every + /// status, and never rewinds — the sidebar's git probe compares it against + /// the value it last saw. + #[test] + fn tool_completions_count_even_when_the_status_holds_still() { + let ev = |kind| AgentEvent { + agent: Some(CLIAgent::Claude), + kind, + session_id: None, + message: None, + cwd: None, + }; + + let mut s = AgentSessionState::default(); + s.apply_event(&ev(AgentEventKind::PromptSubmit)); + assert_eq!(s.activity, 0, "a turn starting is not tool activity"); + + for n in 1..=3 { + s.apply_event(&ev(AgentEventKind::ToolComplete)); + assert_eq!(s.status, AgentStatus::Working, "the status holds still…"); + assert_eq!(s.activity, n, "…while the counter is what moves"); + } + + // A straggler after the turn ended still counts: it may well have + // written a file, and it must not be mistaken for "nothing happened". + s.apply_event(&ev(AgentEventKind::Stop)); + s.apply_event(&ev(AgentEventKind::ToolComplete)); + assert_eq!( + s.status, + AgentStatus::Done, + "and still doesn't resurrect the turn" + ); + assert_eq!(s.activity, 4); + + // Session end resets plenty of state but not this — a rewind to 0 would + // read to a delta-comparing consumer as one more tool call. + s.apply_event(&ev(AgentEventKind::SessionEnd)); + assert_eq!(s.activity, 4); + } + /// The agent's cwd claim: any event carrying one sets it, later events /// without one leave it alone (mid-turn events keep the worktree path /// alive), and session end drops it — an exited agent must not pin the diff --git a/src/daemon/pane.rs b/src/daemon/pane.rs index 9c290dfe..23e50b15 100644 --- a/src/daemon/pane.rs +++ b/src/daemon/pane.rs @@ -3322,6 +3322,7 @@ mod tests { launch_argv: None, rich: true, cwd: None, + activity: 0, }); apply_signals(&mut st, sniffer.feed(b"\x1b]9;noise\x07")); assert_eq!( diff --git a/src/daemon/protocol.rs b/src/daemon/protocol.rs index 43f10771..071fb24a 100644 --- a/src/daemon/protocol.rs +++ b/src/daemon/protocol.rs @@ -1581,6 +1581,7 @@ mod tests { ]), rich: true, cwd: Some("/repo/.claude/worktrees/fix-x".into()), + activity: 12, })), DaemonMsg::AgentStatus(None), DaemonMsg::LoopbackForward(LoopbackForward { local_port: 49152 }), diff --git a/src/terminal/git_status.rs b/src/terminal/git_status.rs index 8bca6ed5..8dd92b59 100644 --- a/src/terminal/git_status.rs +++ b/src/terminal/git_status.rs @@ -20,6 +20,7 @@ use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; +use std::time::{Duration, Instant}; /// 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 @@ -62,39 +63,46 @@ 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'])); + // One `rev-parse` answers every path question at once: the work-tree root + // (which doubles as the "is this a git repo" gate — it fails outside a + // work tree) plus the git-dir/common-dir pair that tells a linked worktree + // from a main checkout. Asking separately cost two process spawns per + // probe, which mattered once probes stopped being rare: they now also fire + // on window activation and on an agent's tool calls, across every pane. + let paths = git( + cwd, + &[ + "rev-parse", + "--path-format=absolute", + "--show-toplevel", + "--git-dir", + "--git-common-dir", + ], + )?; + let mut lines = paths.lines().map(|l| l.trim_end_matches(['\n', '\r'])); + let root = PathBuf::from(lines.next()?); + // A git old enough to reject `--path-format` fails the whole invocation + // above, so reaching here means the two dirs are present — but degrade to + // "main checkout" rather than trusting that, same as the old code did. + let home = repo_home(&root, lines.next(), lines.next()); let branch = branch_name(cwd)?; Some(RepoSnapshot { - home: repo_home(cwd, &root), + home, root, branch, counts: diff_numstat(cwd), }) } -/// The repository "home" every checkout of one repo shares: for a linked -/// worktree (its git dir differs from the common git dir) the main work -/// tree's root — the parent of `
/.git`; for the main checkout itself, -/// a submodule, or any failure to tell, the work-tree root unchanged. A bare -/// common dir (no trailing `.git` component, the bare-repo-plus-worktrees -/// layout) anchors on the bare directory itself — still one shared key. -fn repo_home(cwd: &Path, root: &Path) -> PathBuf { - let both = git( - cwd, - &[ - "rev-parse", - "--path-format=absolute", - "--git-dir", - "--git-common-dir", - ], - ); - let Some(both) = both else { - return root.to_path_buf(); - }; - let mut lines = both.lines(); - let (Some(git_dir), Some(common)) = (lines.next(), lines.next()) else { +/// The repository "home" every checkout of one repo shares, from the work-tree +/// `root` and the `--git-dir` / `--git-common-dir` pair: for a linked worktree +/// (its git dir differs from the common git dir) the main work tree's root — +/// the parent of `
/.git`; for the main checkout itself, a submodule, or +/// any failure to tell, the work-tree root unchanged. A bare common dir (no +/// trailing `.git` component, the bare-repo-plus-worktrees layout) anchors on +/// the bare directory itself — still one shared key. +fn repo_home(root: &Path, git_dir: Option<&str>, common_dir: Option<&str>) -> PathBuf { + let (Some(git_dir), Some(common)) = (git_dir, common_dir) else { return root.to_path_buf(); }; if git_dir == common { @@ -133,6 +141,9 @@ pub struct GitStatusCache { /// In-flight cwds re-triggered meanwhile — reprobed once their flight /// lands, so the newest trigger's state is never skipped. dirty: HashSet, + /// When each cwd's last probe *landed*, for the throttle that opportunistic + /// triggers go through ([`begin_probe_throttled`](Self::begin_probe_throttled)). + last_probe: HashMap, } impl gpui::Global for GitStatusCache {} @@ -177,12 +188,39 @@ impl GitStatusCache { } } + /// Claim an *opportunistic* probe for `cwd`: one triggered by a cheap, + /// frequent signal — the window regaining focus, an agent finishing a tool + /// call — rather than by a rare edge like a command ending. + /// + /// Unlike [`begin_probe`](Self::begin_probe) this declines instead of + /// queueing: a probe already in flight, or one that landed less than + /// `min_interval` ago, drops the trigger entirely (no dirty mark, no + /// rerun). That's the whole point of the two entry points — the rare edges + /// must never be missed, while these signals repeat on their own, so a + /// count that's a second stale beats a `git` storm across every pane of a + /// repo the moment the user alt-tabs back. + pub fn begin_probe_throttled(&mut self, cwd: &Path, min_interval: Duration) -> bool { + if self.in_flight.contains(cwd) { + return false; + } + if self + .last_probe + .get(cwd) + .is_some_and(|at| at.elapsed() < min_interval) + { + return false; + } + 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); + self.last_probe.insert(cwd.to_path_buf(), Instant::now()); match snapshot { Some(snap) => { let (added, removed) = snap.counts.unwrap_or_else(|| { @@ -422,4 +460,60 @@ mod tests { assert_eq!(cache.status_for(main).unwrap().branch, "main"); assert_eq!(cache.status_for(wt).unwrap().branch, "feat/x"); } + + /// The four shapes `repo_home` has to tell apart, straight from the + /// `--git-dir` / `--git-common-dir` pair the merged `rev-parse` returns. + #[test] + fn repo_home_resolves_worktree_layouts() { + let root = Path::new("/repo/.wt/feat"); + + // A main checkout: the two dirs agree, so the work tree is its own home. + assert_eq!( + repo_home(Path::new("/repo"), Some("/repo/.git"), Some("/repo/.git")), + PathBuf::from("/repo") + ); + // A linked worktree: the common dir is the main checkout's `.git`, so + // the home is that `.git`'s parent — the main work tree. + assert_eq!( + repo_home(root, Some("/repo/.git/worktrees/feat"), Some("/repo/.git")), + PathBuf::from("/repo") + ); + // A bare repo with worktrees hanging off it: no `.git` component to + // strip, so the bare dir itself is the shared key. + assert_eq!( + repo_home(root, Some("/bare.git/worktrees/feat"), Some("/bare.git")), + PathBuf::from("/bare.git") + ); + // A git too old (or too odd) to answer both: degrade to the work tree + // rather than guessing a grouping key. + assert_eq!( + repo_home(root, Some("/repo/.git"), None), + root.to_path_buf() + ); + assert_eq!(repo_home(root, None, None), root.to_path_buf()); + } + + /// The opportunistic path declines where the edge path queues: an in-flight + /// probe drops the trigger (and leaves nothing dirty, so no rerun), and a + /// probe that just landed rate-limits the next one. + #[test] + fn throttled_probes_decline_instead_of_queueing() { + let mut cache = GitStatusCache::default(); + let cwd = Path::new("/repo"); + let gap = Duration::from_secs(60); + + assert!(cache.begin_probe_throttled(cwd, gap)); + // In flight: declined, and unlike `begin_probe` it doesn't mark dirty — + // the landing reports "nothing pending" rather than asking for a rerun. + assert!(!cache.begin_probe_throttled(cwd, gap)); + assert!(!cache.finish_probe(cwd, Some(snap("/repo", "main", Some((1, 0)))))); + + // Landed just now: still inside the gap, so the next trigger is dropped. + assert!(!cache.begin_probe_throttled(cwd, gap)); + // …but a zero gap always lets one through, and edge triggers never + // consult the throttle at all. + assert!(cache.begin_probe_throttled(cwd, Duration::ZERO)); + assert!(!cache.finish_probe(cwd, Some(snap("/repo", "main", Some((1, 0)))))); + assert!(cache.begin_probe(cwd)); + } } diff --git a/src/terminal/remote.rs b/src/terminal/remote.rs index 1ed0799b..d2a8627f 100644 --- a/src/terminal/remote.rs +++ b/src/terminal/remote.rs @@ -2360,6 +2360,7 @@ mod tests { launch_argv: None, rich: true, cwd: None, + activity: 0, })) .encode(&mut daemon_side) .unwrap(); diff --git a/src/terminal/view.rs b/src/terminal/view.rs index b59a1fd8..c9e953c0 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -261,6 +261,11 @@ pub struct TerminalView { /// by work-tree root — panes in one repo share one entry instead of each /// computing (and staling) its own. git_status_cwd: Option, + /// The agent session's tool-completion count as of the last poll, so a + /// change means "the agent ran a tool since we looked" — the cue to refresh + /// the git line mid-turn rather than at the end of one. Reset to 0 when no + /// session is present, so a new session's first tool call reads as activity. + last_agent_activity: 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 @@ -460,6 +465,27 @@ const INTEGRATION_GRACE: std::time::Duration = std::time::Duration::from_secs(8) /// How long the integration notice stays up when no keystroke dismisses it. const INTEGRATION_NOTICE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(15); +/// Floor on how often an *opportunistic* git probe may run for one cwd (see +/// [`GitRefresh::Opportunistic`]). Short enough that the sidebar's counts feel +/// live while an agent works, long enough that a burst of tool calls — or an +/// alt-tab into a window holding a dozen panes — collapses into one `git` +/// shell-out per repo instead of a dozen. +const OPPORTUNISTIC_GIT_GAP: std::time::Duration = std::time::Duration::from_millis(1500); + +/// Why a git-status probe is being asked for — the two classes get opposite +/// treatment when one is already in flight. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +enum GitRefresh { + /// A rare state change that must not be missed: the pane changed + /// directory, a command finished, an agent turn ended. Queues behind an + /// in-flight probe (which then reruns) rather than being dropped. + Edge, + /// A cheap signal that repeats on its own: the window regained focus, the + /// agent finished a tool call. Dropped outright when a probe is in flight + /// or one ran within [`OPPORTUNISTIC_GIT_GAP`] — the next one will come. + Opportunistic, +} + /// Fig-descended PTY shims known to exec over the shell we spawned and re-host /// it on a nested PTY without forwarding OSC 133 — which starves shell /// integration and silently kills the whole command-editor overlay (#46). @@ -1022,6 +1048,7 @@ impl TerminalView { agent_result_unread: false, keep_unread_on_focus: false, git_status_cwd: None, + last_agent_activity: 0, cmd: CmdEditor::new(), typeahead: Typeahead::new(), hold: GapHold::new(), @@ -1157,6 +1184,24 @@ impl TerminalView { self.git_status_cwd.as_deref() } + /// Re-probe this pane's git status opportunistically — for callers holding + /// a reason to suspect the tree moved without the pane seeing it. The one + /// that matters is the window regaining focus: edits made in an editor, or + /// by a `git` command run in another app entirely, produce no event here at + /// all, so without this the counts would sit stale until the user happened + /// to run something in the pane. + /// + /// Throttled and in-flight-deduped (see [`GitRefresh::Opportunistic`]), so + /// calling it for every pane on every activation is cheap. A pane with no + /// resolved cwd yet is skipped rather than being pinned to `None` — its + /// first real probe is the poll loop's job. + pub fn refresh_git_status_now(&mut self, cx: &mut Context) { + let cwd = self.git_status_cwd.clone(); + if cwd.is_some() { + self.refresh_git_status(cwd, GitRefresh::Opportunistic, cx); + } + } + /// The current grid selection as text, if any non-blank one exists — the /// source for "Agent: Send Selection". pub fn selection_text(&self) -> Option { @@ -2627,6 +2672,13 @@ impl TerminalView { // otherwise stay invisible until it exits. All rare edges, so the // off-thread `git` shell-out runs seldom, not every 300ms tick. // + // Those edges alone left the counts badly stale during the case they + // matter most: a long agent turn writes file after file for minutes + // with nothing to show for it. A tool completion is the one signal that + // the tree may have just moved mid-turn, so it refreshes too — through + // the throttled path, since a busy agent emits them several a second + // and each one would otherwise cost a `git diff` across the repo. + // // An agent that reports its own cwd through the hook channel wins over // the proc probe: it tracks internal chdirs the PTY can't observe // (Claude Code's EnterWorktree) and works where the proc fallback @@ -2639,18 +2691,31 @@ impl TerminalView { // a remote path — and being first in the chain it would win over // `local_cwd` unconditionally and hand that path straight to the local // `git`, which is the collision `local_cwd` exists to prevent. + let session = self.terminal.agent_session(); + // A count that moved means at least one tool finished since the last + // tick. With no session the counter resets, so a fresh agent's very + // first tool call still reads as activity. + let tool_activity = match session.as_ref().map(|s| s.activity) { + Some(n) => std::mem::replace(&mut self.last_agent_activity, n) != n, + None => { + self.last_agent_activity = 0; + false + } + }; let cwd_now = self .remote_context() .is_none() .then(|| { - self.terminal - .agent_session() - .and_then(|s| s.cwd) + session + .as_ref() + .and_then(|s| s.cwd.clone()) .or_else(|| self.cwd()) }) .flatten(); if cwd_now.as_ref() != self.git_status_cwd.as_ref() || cmd_finished || turn_finished { - self.refresh_git_status(cwd_now, cx); + self.refresh_git_status(cwd_now, GitRefresh::Edge, cx); + } else if tool_activity { + self.refresh_git_status(cwd_now, GitRefresh::Opportunistic, cx); } } @@ -2666,7 +2731,12 @@ impl TerminalView { /// cannot be what keeps the local probe away from a remote path. /// /// [`GitStatusCache`]: crate::terminal::git_status::GitStatusCache - fn refresh_git_status(&mut self, cwd: Option, cx: &mut Context) { + fn refresh_git_status( + &mut self, + cwd: Option, + trigger: GitRefresh, + cx: &mut Context, + ) { use crate::terminal::git_status::GitStatusCache; let changed = self.git_status_cwd != cwd; @@ -2678,7 +2748,11 @@ impl TerminalView { return; }; cx.default_global::(); // first probe of the process creates it - if !cx.update_global::(|cache, _| cache.begin_probe(&cwd)) { + let claimed = cx.update_global::(|cache, _| match trigger { + GitRefresh::Edge => cache.begin_probe(&cwd), + GitRefresh::Opportunistic => cache.begin_probe_throttled(&cwd, OPPORTUNISTIC_GIT_GAP), + }); + if !claimed { return; } cx.spawn(async move |this, cx| { @@ -2689,19 +2763,27 @@ impl TerminalView { async move { crate::terminal::git_status::probe(&cwd) } }) .await; - let _ = this.update(cx, |view, cx| { - // 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) + // Land the result in the shared cache before touching the pane, + // and whether or not the pane still exists: the in-flight claim is + // keyed by *cwd*, so a pane closed mid-probe that never released + // its claim would wedge the git line of every other pane in that + // directory — permanently, since nothing else ever clears it. + // + // 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. Only edge + // triggers set that flag, so the rerun is an edge too. + if rerun { + let _ = this.update(cx, |view, cx| { + if view.git_status_cwd.as_deref() == Some(&cwd) { + view.refresh_git_status(Some(cwd), GitRefresh::Edge, cx); + } }); - // 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); - } - }); + } }) .detach(); } diff --git a/src/ui/app.rs b/src/ui/app.rs index 8aed4b89..a2d21aa9 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -530,13 +530,22 @@ impl Tty7App { // so treat it like a release. Dismissing on *both* flips also keeps a // reveal scheduled just before the switch from popping the badges up // in a window the user already left. - let activation_watch = cx.observe_window_activation(window, |this, _window, cx| { + let activation_watch = cx.observe_window_activation(window, |this, window, cx| { this.dismiss_mod_hint(cx); // The panes' link-modifier tracking loses the release the same // way, and a stale "⌘ held" is worse than missing badges: a // plain unmodified click would open links. Treat the flip as a // release; holding ⌘ again re-arms it via `on_modifiers_changed`. this.set_link_modifier(false, cx); + // Coming back is the only cue we get that the working tree may + // have moved while the user was elsewhere: an edit in another + // editor, a `git` command in another app, an agent in another + // window. None of those reach a pane's poll loop, so without this + // the sidebar's `+N −N` would keep showing pre-alt-tab numbers + // until the user happened to run a command in the pane. + if window.is_window_active() { + this.refresh_git_status_all(cx); + } }); // Follow OS light/dark flips live: while "sync with system" is on, an // appearance change re-resolves the theme slot and repaints. While it's @@ -2067,6 +2076,21 @@ impl Tty7App { window.focus(&handle, cx); } + /// Ask every pane in the window to re-probe its git status. Called when the + /// window regains focus: the sidebar shows a git line for *every* tab, not + /// just the active one, so refreshing only the focused pane would leave the + /// rest of the list stale — which is exactly the list the user is scanning + /// right after switching back. + /// + /// Panes sharing a cwd fold into one probe in the shared cache, and the + /// throttle there drops anything probed in the last moment, so the cost of + /// a window with many panes is bounded by the number of distinct repos. + fn refresh_git_status_all(&mut self, cx: &mut Context) { + for leaf in self.tabs.iter().flat_map(|tab| tab.pane.leaves()) { + leaf.update(cx, |view, cx| view.refresh_git_status_now(cx)); + } + } + /// Where a freshly opened tab should be inserted, per `new_tab_position`: /// right after the active tab, or appended at the end. Clamped to the tab /// count so the zero-tab home state (active 0, no tabs) inserts at 0. From 7ab19f14f65988175609eff822d6cbce2ab65dac Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Thu, 23 Jul 2026 17:09:20 +0800 Subject: [PATCH 2/2] fix(git-status): throttle opportunistic probes per repo, not per cwd MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The counts a probe produces are repo-wide — `git diff --numstat HEAD` ignores the subdirectory it ran in — so panes at `repo/`, `repo/src` and `repo/docs` were three ways of asking one question, and a window activation spent one full-repo diff on each. Count the throttle against the work-tree root once a probe has resolved one for the cwd, and stamp the clock when the probe is *claimed* rather than when it lands: without that, panes claiming in the same instant all pass a throttle nothing has answered yet, which is exactly the shape a window activation has. In-flight dedup stays keyed by cwd, since it brackets a specific spawn that finish_probe has to release. --- Cargo.lock | 140 +++++-------------------------------- src/terminal/git_status.rs | 81 +++++++++++++++++++-- src/ui/app.rs | 7 +- 3 files changed, 98 insertions(+), 130 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index df99a38b..69c3f731 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2500,7 +2500,7 @@ version = "0.5.8" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bbc773e24e02d4ddd8395fd30dc147524273a83e54e0f312d986ea30de5f5646" dependencies = [ - "roxmltree 0.20.0", + "roxmltree", ] [[package]] @@ -3151,7 +3151,7 @@ dependencies = [ "raw-window-handle", "refineable", "regex", - "resvg 0.45.1", + "resvg", "scheduler", "schemars", "seahash", @@ -3167,7 +3167,7 @@ dependencies = [ "thiserror 2.0.18", "ttf-parser", "url", - "usvg 0.45.1", + "usvg", "util_macros", "uuid", "waker-fn", @@ -3209,7 +3209,7 @@ dependencies = [ "paste", "raw-window-handle", "regex", - "resvg 0.45.1", + "resvg", "ropey", "rust-i18n", "schemars", @@ -4056,12 +4056,6 @@ version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "edcd27d72f2f071c64249075f42e205ff93c9a4c5f6c6da53e79ed9f9832c285" -[[package]] -name = "imagesize" -version = "0.14.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "09e54e57b4c48b40f7aec75635392b12b3421fa26fe8b4332e63138ed278459c" - [[package]] name = "imgref" version = "1.12.2" @@ -4494,18 +4488,6 @@ dependencies = [ "smallvec", ] -[[package]] -name = "kurbo" -version = "0.13.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4b60dfc32f652b926df6192e55525b16d186c69d47876c3ead4da5cc9f8450e2" -dependencies = [ - "arrayvec", - "euclid", - "polycool", - "smallvec", -] - [[package]] name = "kv-log-macro" version = "1.0.7" @@ -6307,15 +6289,6 @@ dependencies = [ "zeroize", ] -[[package]] -name = "polycool" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "50596ddc09eb5ad5f75cacd40209568e66df71baf86e1499a0e99c4cff12a5a6" -dependencies = [ - "arrayvec", -] - [[package]] name = "polyval" version = "0.7.2" @@ -7084,26 +7057,12 @@ dependencies = [ "log", "pico-args", "rgb", - "svgtypes 0.15.3", - "tiny-skia 0.11.4", - "usvg 0.45.1", + "svgtypes", + "tiny-skia", + "usvg", "zune-jpeg 0.4.21", ] -[[package]] -name = "resvg" -version = "0.47.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9be183ad6a216aa96f33e4c8033b0988b8b3ea6fd2359d19af5bac4643fd8e81" -dependencies = [ - "log", - "pico-args", - "rgb", - "svgtypes 0.16.1", - "tiny-skia 0.12.0", - "usvg 0.47.0", -] - [[package]] name = "rfc6979" version = "0.6.0" @@ -7158,15 +7117,6 @@ version = "0.20.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "6c20b6793b5c2fa6553b250154b78d6d0db37e72700ae35fad9387a46f487c97" -[[package]] -name = "roxmltree" -version = "0.21.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f1964b10c76125c36f8afe190065a4bf9a87bf324842c05701330bba9f1cacbb" -dependencies = [ - "memchr", -] - [[package]] name = "rsa" version = "0.10.0-rc.18" @@ -8512,17 +8462,7 @@ version = "0.15.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "68c7541fff44b35860c1a7a47a7cadf3e4a304c457b58f9870d9706ece028afc" dependencies = [ - "kurbo 0.11.3", - "siphasher", -] - -[[package]] -name = "svgtypes" -version = "0.16.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "695b5790b3131dafa99b3bbfd25a216edb3d216dad9ca208d4657bfb8f2abc3d" -dependencies = [ - "kurbo 0.13.1", + "kurbo", "siphasher", ] @@ -8859,22 +8799,7 @@ dependencies = [ "cfg-if", "log", "png 0.17.16", - "tiny-skia-path 0.11.4", -] - -[[package]] -name = "tiny-skia" -version = "0.12.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "47ffee5eaaf5527f630fb0e356b90ebdec84d5d18d937c5e440350f88c5a91ea" -dependencies = [ - "arrayref", - "arrayvec", - "bytemuck", - "cfg-if", - "log", - "png 0.18.1", - "tiny-skia-path 0.12.0", + "tiny-skia-path", ] [[package]] @@ -8888,17 +8813,6 @@ dependencies = [ "strict-num", ] -[[package]] -name = "tiny-skia-path" -version = "0.12.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "edca365c3faccca67d06593c5980fa6c57687de727a03131735bb85f01fdeeb9" -dependencies = [ - "arrayref", - "bytemuck", - "strict-num", -] - [[package]] name = "tinystr" version = "0.8.3" @@ -9292,13 +9206,13 @@ dependencies = [ "portable-pty", "regex", "reqwest_client", - "resvg 0.47.0", + "resvg", "russh", "russh-sftp", "serde", "serde_json", "serde_yaml", - "sha2 0.11.0", + "sha2 0.10.9", "smallvec", "smol", "tokio", @@ -9465,45 +9379,23 @@ dependencies = [ "data-url", "flate2", "fontdb", - "imagesize 0.13.0", - "kurbo 0.11.3", + "imagesize", + "kurbo", "log", "pico-args", - "roxmltree 0.20.0", + "roxmltree", "rustybuzz", "simplecss", "siphasher", "strict-num", - "svgtypes 0.15.3", - "tiny-skia-path 0.11.4", + "svgtypes", + "tiny-skia-path", "unicode-bidi", "unicode-script", "unicode-vo", "xmlwriter", ] -[[package]] -name = "usvg" -version = "0.47.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d46cf96c5f498d36b7a9693bc6a7075c0bb9303189d61b2249b0dc3d309c07de" -dependencies = [ - "base64", - "data-url", - "flate2", - "imagesize 0.14.0", - "kurbo 0.13.1", - "log", - "pico-args", - "roxmltree 0.21.1", - "simplecss", - "siphasher", - "strict-num", - "svgtypes 0.16.1", - "tiny-skia-path 0.12.0", - "xmlwriter", -] - [[package]] name = "utf-8" version = "0.7.6" diff --git a/src/terminal/git_status.rs b/src/terminal/git_status.rs index 8dd92b59..39d8d3ff 100644 --- a/src/terminal/git_status.rs +++ b/src/terminal/git_status.rs @@ -193,34 +193,70 @@ impl GitStatusCache { /// call — rather than by a rare edge like a command ending. /// /// Unlike [`begin_probe`](Self::begin_probe) this declines instead of - /// queueing: a probe already in flight, or one that landed less than - /// `min_interval` ago, drops the trigger entirely (no dirty mark, no + /// queueing: a probe already in flight, or one against a repo probed less + /// than `min_interval` ago, drops the trigger entirely (no dirty mark, no /// rerun). That's the whole point of the two entry points — the rare edges /// must never be missed, while these signals repeat on their own, so a /// count that's a second stale beats a `git` storm across every pane of a /// repo the moment the user alt-tabs back. + /// + /// The throttle counts per *repo*, not per cwd (see + /// [`throttle_key`](Self::throttle_key)), and the claim stamps the clock + /// rather than waiting for the landing: without that, a dozen panes + /// scattered over one repo's subdirectories would all claim in the same + /// instant — each of them passing a throttle no probe had answered yet — + /// and produce a dozen identical full-repo diffs. pub fn begin_probe_throttled(&mut self, cwd: &Path, min_interval: Duration) -> bool { if self.in_flight.contains(cwd) { return false; } + let key = self.throttle_key(cwd).to_path_buf(); if self .last_probe - .get(cwd) + .get(&key) .is_some_and(|at| at.elapsed() < min_interval) { return false; } + self.last_probe.insert(key, Instant::now()); self.in_flight.insert(cwd.to_path_buf()); true } + /// What the opportunistic throttle counts against: the work-tree root once + /// some probe has answered for `cwd`, and `cwd` itself before that. + /// + /// The counts a probe produces are repo-wide — `git diff --numstat HEAD` + /// ignores which subdirectory it ran in — so panes at `repo/`, `repo/src` + /// and `repo/docs` are three ways of asking one question, and want one + /// shared clock rather than one each. In-flight dedup stays keyed by cwd: + /// it brackets a specific spawn, and [`finish_probe`](Self::finish_probe) + /// has to be able to release exactly what was claimed. + /// + /// Before any probe has landed the root is simply unknown, so the first + /// sweep over a repo still costs one probe per distinct cwd; every sweep + /// after that collapses to one. + fn throttle_key<'a>(&'a self, cwd: &'a Path) -> &'a Path { + match self.roots.get(cwd) { + Some(Some(root)) => root, + _ => cwd, + } + } + /// 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); - self.last_probe.insert(cwd.to_path_buf(), Instant::now()); + // Re-stamp on landing so the gap is measured from fresh counts, and + // under the root this probe just resolved — which is how a cwd first + // learns to share its repo's clock (at claim time it had none). + let key = match &snapshot { + Some(snap) => snap.root.clone(), + None => self.throttle_key(cwd).to_path_buf(), + }; + self.last_probe.insert(key, Instant::now()); match snapshot { Some(snap) => { let (added, removed) = snap.counts.unwrap_or_else(|| { @@ -516,4 +552,41 @@ mod tests { assert!(!cache.finish_probe(cwd, Some(snap("/repo", "main", Some((1, 0)))))); assert!(cache.begin_probe(cwd)); } + + /// The throttle is per repo, not per cwd: panes sitting in different + /// subdirectories ask one question (the counts are repo-wide), so once the + /// cache knows where they live, a window activation costs one probe for + /// the repo rather than one per pane. + #[test] + fn throttle_collapses_subdirectories_of_one_repo() { + let mut cache = GitStatusCache::default(); + let (top, src, docs) = ( + Path::new("/repo"), + Path::new("/repo/src"), + Path::new("/repo/docs"), + ); + let gap = Duration::from_secs(60); + + // Nothing known yet, so each cwd is its own key and each gets a probe. + for cwd in [top, src, docs] { + assert!(cache.begin_probe_throttled(cwd, gap)); + assert!(!cache.finish_probe(cwd, Some(snap("/repo", "main", Some((3, 1)))))); + } + + // Now all three resolve to `/repo`, so the next sweep collapses: the + // first pane to ask spends the probe and the rest ride on it. + assert!(!cache.begin_probe_throttled(top, gap)); + assert!(!cache.begin_probe_throttled(src, gap)); + + // …and the claim itself is what stops the stampede — with the clock + // wound back far enough to let one through, the *others* still decline + // while it is in flight, even though nothing has landed yet. + assert!(cache.begin_probe_throttled(docs, Duration::ZERO)); + assert!(!cache.begin_probe_throttled(top, gap)); + assert!(!cache.begin_probe_throttled(src, gap)); + + // A pane elsewhere is untouched by any of it. + let other = Path::new("/other"); + assert!(cache.begin_probe_throttled(other, gap)); + } } diff --git a/src/ui/app.rs b/src/ui/app.rs index a2d21aa9..14b86051 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -2083,8 +2083,11 @@ impl Tty7App { /// right after switching back. /// /// Panes sharing a cwd fold into one probe in the shared cache, and the - /// throttle there drops anything probed in the last moment, so the cost of - /// a window with many panes is bounded by the number of distinct repos. + /// throttle there counts per repo rather than per cwd, so once the cache + /// knows where each pane lives the cost of a window with many panes is + /// bounded by the number of distinct repos — not by the number of + /// subdirectories they happen to sit in, which would be the same full-repo + /// `git diff` asked several times over. fn refresh_git_status_all(&mut self, cx: &mut Context) { for leaf in self.tabs.iter().flat_map(|tab| tab.pane.leaves()) { leaf.update(cx, |view, cx| view.refresh_git_status_now(cx));