From 5c45d17513a80fd5f6428f58effc0b5488e2aab2 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Mon, 12 Oct 2026 23:17:14 +0800 Subject: [PATCH] fix(sidebar): retry negative repository probes for idle panes (#1157) --- src/terminal/git_status.rs | 59 +++++++++++++++++++++++++++++++++++++- src/terminal/view.rs | 19 +++++------- 2 files changed, 65 insertions(+), 13 deletions(-) diff --git a/src/terminal/git_status.rs b/src/terminal/git_status.rs index 04cf6db6..5bcb93fd 100644 --- a/src/terminal/git_status.rs +++ b/src/terminal/git_status.rs @@ -115,6 +115,13 @@ impl GitStatusCache { .is_some_and(|at| at.elapsed() < min_interval) } + /// Recheck unresolved and negative answers: repository metadata can be + /// temporarily unreadable, or a repository can be created in an idle cwd. + pub fn retry_due(&self, host: HostId, cwd: &Path, min_interval: Duration) -> bool { + !matches!(self.roots.get(host, &*key(host, cwd)), Some(Some(_))) + && self.probe_due(host, cwd, min_interval) + } + /// `cwd` is already in the cache's own spelling — every caller of this one /// has been past [`key`]. fn throttle_key<'a>(&'a self, host: HostId, cwd: &'a Path) -> &'a Path { @@ -195,7 +202,9 @@ impl GitStatusCache { let rerun = !self.probes.finish(&(host, cwd.to_path_buf())); let throttle = match &snapshot { Some(snap) => snap.root.clone(), - None => self.throttle_key(host, &cwd).to_path_buf(), + // A negative result removes the cwd-to-root mapping below, so + // subsequent retries are throttled by cwd, not the old root. + None => cwd.to_path_buf(), }; self.last_probe.insert(host, throttle, Instant::now()); match snapshot { @@ -430,6 +439,54 @@ mod tests { assert!(cache.status_for(L, b).is_some()); } + #[test] + fn a_negative_probe_retries_and_recovers_without_a_cwd_change() { + let mut cache = GitStatusCache::default(); + let cwd = Path::new("/repo/src"); + let gap = Duration::from_secs(10); + cache.finish_probe(L, cwd, Some(snap("/repo", "main", None))); + assert!(!cache.retry_due(L, cwd, Duration::ZERO)); + + // Discovery temporarily cannot recognize the repository. + cache.begin_probe(L, cwd); + cache.finish_probe(L, cwd, None); + assert_eq!(cache.known_repo_for(L, cwd), Some(None)); + assert!( + !cache.retry_due(L, cwd, gap), + "negative answers are throttled by cwd" + ); + cache + .last_probe + .insert(L, cwd.to_path_buf(), Instant::now() - gap); + assert!(cache.retry_due(L, cwd, gap)); + assert!(cache.begin_probe_throttled(L, cwd, gap)); + assert!( + !cache.retry_due(L, cwd, Duration::ZERO), + "only one retry in flight" + ); + + cache.finish_probe(L, cwd, Some(snap("/repo", "main", None))); + assert_eq!( + cache.known_repo_for(L, cwd), + Some(Some(PathBuf::from("/repo"))) + ); + assert!(!cache.retry_due(L, cwd, Duration::ZERO)); + } + + #[test] + fn a_plain_directory_is_rechecked_after_the_retry_interval() { + let mut cache = GitStatusCache::default(); + let cwd = Path::new("/plain"); + let gap = Duration::from_secs(10); + assert!(cache.retry_due(L, cwd, gap)); + cache.finish_probe(L, cwd, None); + assert!(!cache.retry_due(L, cwd, gap)); + cache + .last_probe + .insert(L, cwd.to_path_buf(), Instant::now() - gap); + assert!(cache.retry_due(L, cwd, gap)); + } + #[test] fn a_failed_probe_leaves_what_was_known() { let mut cache = GitStatusCache::default(); diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 6caffddf..d17c0cda 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -679,11 +679,9 @@ const INTEGRATION_NOTICE_TIMEOUT: std::time::Duration = std::time::Duration::fro const OPPORTUNISTIC_GIT_GAP: std::time::Duration = std::time::Duration::from_millis(1500); -/// How long a pane whose repository is still unanswered waits before asking -/// again. Only a probe that failed leaves it that way, and whatever broke it -/// (a remote link mid-reconnect, a `git` that would not start) is gone in -/// seconds, not milliseconds — nor worth a request every poll tick while it -/// lasts. +/// How long a pane with an unknown or negative repository answer waits +/// before asking again. Discovery can miss temporarily unavailable metadata; +/// retry without issuing a request on every poll tick. const GIT_RETRY_GAP: std::time::Duration = std::time::Duration::from_secs(10); /// How long a title the program set has to stand before the tab adopts it. @@ -733,7 +731,7 @@ enum GitRefresh { Edge, Opportunistic, /// Nothing happened; the cache just has no answer for this pane's cwd - /// yet, because the last probe failed. Throttled by [`GIT_RETRY_GAP`]. + /// yet, or the last probe found no repository. Throttled by [`GIT_RETRY_GAP`]. Retry, } @@ -2274,10 +2272,8 @@ impl TerminalView { self.git_status_cwd.as_deref() } - /// This pane sits somewhere the repo cache cannot place yet — neither a - /// repository nor "not one" — and a retry is due. Only a failed probe - /// leaves a cwd that way; without this it would wait for the cwd to change - /// or a command to finish before anyone asked again. + /// Retry unknown and negative repo answers. A transient discovery failure + /// must not leave an idle pane in Ungrouped until it moves or restarts. /// /// Read-only on purpose: this runs on every poll tick, and the cache is a /// global whose every mutable touch redraws the window — while a probe is @@ -2289,8 +2285,7 @@ impl TerminalView { let Some(cache) = cx.try_global::() else { return false; }; - cache.known_repo_for(self.host_id, cwd).is_none() - && cache.probe_due(self.host_id, cwd, GIT_RETRY_GAP) + cache.retry_due(self.host_id, cwd, GIT_RETRY_GAP) } /// Plant the cwd the git-status poll would have found. For tests that