From d4e0dd3d903c50d2edb8c3cec71952a83989b310 Mon Sep 17 00:00:00 2001 From: Can Celik Date: Sat, 25 Jul 2026 04:26:55 +0300 Subject: [PATCH] perf: eliminate repeated workspace git discovery (#1842) * perf: eliminate repeated workspace git discovery refs #1838 * fix: keep workspace labels current without extra redraws refs #1838 * fix: retain discovered workspace git metadata refs #1838 --- src/app/actions.rs | 65 ++++- src/app/api.rs | 58 ++-- src/app/git_refresh.rs | 480 +++++++++++++++++++++++++++++++++ src/app/mod.rs | 50 +++- src/app/runtime.rs | 301 ++------------------- src/persist/restore.rs | 7 +- src/server/headless.rs | 60 ++++- src/ui/sidebar.rs | 4 +- src/workspace.rs | 181 +++++++++++-- src/workspace/git/discovery.rs | 16 +- src/workspace/git/mod.rs | 10 +- src/workspace/git/status.rs | 212 +++++++++++++-- 12 files changed, 1080 insertions(+), 364 deletions(-) create mode 100644 src/app/git_refresh.rs diff --git a/src/app/actions.rs b/src/app/actions.rs index d3b85c07..0709ed4c 100644 --- a/src/app/actions.rs +++ b/src/app/actions.rs @@ -2607,11 +2607,21 @@ impl AppState { } let ws = &mut self.workspaces[ws_idx]; - if ws.cached_git_branch != result.branch { + if ws.cached_identity_cwd != result.resolved_identity_cwd { + ws.cached_identity_cwd = result.resolved_identity_cwd; + } + if ws.cached_auto_label != result.auto_label { + ws.cached_auto_label = result.auto_label; + changed |= ws.custom_name.is_none(); + } + if ws.cached_git_status_key != result.status_cache_key { + ws.cached_git_status_key = result.status_cache_key; + } + if result.demand.branch && ws.cached_git_branch != result.branch { ws.cached_git_branch = result.branch; changed = true; } - if ws.cached_git_ahead_behind != result.ahead_behind { + if result.demand.ahead_behind && ws.cached_git_ahead_behind != result.ahead_behind { ws.cached_git_ahead_behind = result.ahead_behind; changed = true; } @@ -3103,7 +3113,8 @@ impl AppState { let sound = sound_for_toast_kind(kind, suppress_active_tab_notifications) .filter(|_| self.sound.allows(known_agent)); let build_toast = || { - let workspace_label = self.workspaces[ws_idx].display_name(); + let workspace_label = + self.workspaces[ws_idx].display_name_from_terminals(&self.terminals); let context = notification_context(&self.workspaces[ws_idx], &workspace_label, ws_idx, pane_id); ToastNotification { @@ -3904,7 +3915,10 @@ mod tests { &terminal_runtimes, vec![WorkspaceGitStatus { workspace_id: first_id, - resolved_identity_cwd: first_cwd, + resolved_identity_cwd: first_cwd.clone(), + status_cache_key: first_cwd, + demand: crate::workspace::GitStatusRefreshDemand::ALL, + auto_label: "one".into(), branch: Some("main".into()), ahead_behind: Some((2, 1)), space: None, @@ -3931,6 +3945,9 @@ mod tests { vec![WorkspaceGitStatus { workspace_id, resolved_identity_cwd: std::path::PathBuf::from("/definitely/not/current"), + status_cache_key: std::path::PathBuf::from("/definitely/not/current"), + demand: crate::workspace::GitStatusRefreshDemand::ALL, + auto_label: "stale".into(), branch: Some("main".into()), ahead_behind: Some((0, 1)), space: None, @@ -3942,6 +3959,36 @@ mod tests { assert_eq!(state.workspaces[0].git_ahead_behind(), Some((1, 0))); } + #[test] + fn apply_workspace_git_statuses_ignores_unrequested_branch_changes() { + let mut state = app_with_workspaces(&["one"]); + let workspace_id = state.workspaces[0].id.clone(); + let cwd = state.workspaces[0].resolved_identity_cwd().unwrap(); + state.workspaces[0].cached_auto_label = "one".into(); + state.workspaces[0].cached_git_branch = Some("old".into()); + + let terminal_runtimes = crate::terminal::TerminalRuntimeRegistry::new(); + let changed = state.apply_workspace_git_statuses( + &terminal_runtimes, + vec![WorkspaceGitStatus { + workspace_id, + resolved_identity_cwd: cwd.clone(), + status_cache_key: cwd, + demand: crate::workspace::GitStatusRefreshDemand { + branch: false, + ahead_behind: true, + }, + auto_label: "one".into(), + branch: Some("new".into()), + ahead_behind: None, + space: None, + }], + ); + + assert!(!changed); + assert_eq!(state.workspaces[0].branch().as_deref(), Some("old")); + } + #[test] fn apply_workspace_git_statuses_clears_missing_git_status() { let mut state = app_with_workspaces(&["one"]); @@ -3955,7 +4002,10 @@ mod tests { &terminal_runtimes, vec![WorkspaceGitStatus { workspace_id, - resolved_identity_cwd: cwd, + resolved_identity_cwd: cwd.clone(), + status_cache_key: cwd, + demand: crate::workspace::GitStatusRefreshDemand::ALL, + auto_label: "one".into(), branch: None, ahead_behind: None, space: None, @@ -3980,7 +4030,10 @@ mod tests { &terminal_runtimes, vec![WorkspaceGitStatus { workspace_id, - resolved_identity_cwd: cwd, + resolved_identity_cwd: cwd.clone(), + status_cache_key: cwd, + demand: crate::workspace::GitStatusRefreshDemand::ALL, + auto_label: "other".into(), branch: Some("scratch".into()), ahead_behind: None, space: Some(crate::workspace::GitSpaceMetadata { diff --git a/src/app/api.rs b/src/app/api.rs index 02a61067..37b1f455 100644 --- a/src/app/api.rs +++ b/src/app/api.rs @@ -59,6 +59,44 @@ impl App { response_rx.try_recv().ok() } + pub(crate) fn handle_internal_event_with_render_impact(&mut self, ev: AppEvent) -> bool { + match ev { + AppEvent::GitStatusRefreshed { + results, + cache_updates, + } => self.handle_git_status_refreshed(results, cache_updates), + ev => { + self.handle_internal_event(ev); + true + } + } + } + + fn handle_git_status_refreshed( + &mut self, + results: Vec, + cache_updates: Vec<(std::path::PathBuf, crate::workspace::GitStatusCacheEntry)>, + ) -> bool { + self.git_refresh_in_flight = false; + for (key, entry) in cache_updates { + self.git_status_cache.insert(key, entry); + } + if self.git_refresh_due_after_in_flight { + self.mark_git_status_refresh_due(Instant::now()); + self.git_refresh_due_after_in_flight = false; + } else { + self.last_git_remote_status_refresh = Instant::now(); + } + let changed = self + .state + .apply_workspace_git_statuses(&self.terminal_runtimes, results); + if changed { + self.render_dirty.store(true, Ordering::Release); + self.render_notify.notify_one(); + } + changed + } + pub(crate) fn handle_internal_event(&mut self, ev: AppEvent) { if let AppEvent::ClipboardWrite { content } = ev { #[cfg(not(test))] @@ -90,23 +128,7 @@ impl App { cache_updates, } = ev { - self.git_refresh_in_flight = false; - for (key, entry) in cache_updates { - self.git_status_cache.insert(key, entry); - } - if self.git_refresh_due_after_in_flight { - self.mark_git_status_refresh_due(Instant::now()); - self.git_refresh_due_after_in_flight = false; - } else { - self.last_git_remote_status_refresh = Instant::now(); - } - if self - .state - .apply_workspace_git_statuses(&self.terminal_runtimes, results) - { - self.render_dirty.store(true, Ordering::Release); - self.render_notify.notify_one(); - } + self.handle_git_status_refreshed(results, cache_updates); return; } @@ -270,7 +292,7 @@ impl App { } self.sync_full_lifecycle_authority_detection_pauses(); if terminal_cwd_reported { - self.mark_git_status_refresh_due(Instant::now()); + self.request_git_identity_refresh(Instant::now()); self.render_dirty.store(true, Ordering::Release); self.render_notify.notify_one(); } diff --git a/src/app/git_refresh.rs b/src/app/git_refresh.rs new file mode 100644 index 00000000..a868a0ed --- /dev/null +++ b/src/app/git_refresh.rs @@ -0,0 +1,480 @@ +use std::collections::HashMap; +use std::path::PathBuf; +use std::time::Instant; + +use super::{App, GIT_REMOTE_STATUS_REFRESH_INTERVAL, GIT_REPO_DISCOVERY_REFRESH_INTERVAL}; +use crate::events::AppEvent; +use crate::workspace::{GitStatusCacheEntry, GitStatusRefreshDemand, WorkspaceGitStatus}; + +#[derive(Clone, Debug, PartialEq, Eq)] +struct WorkspaceGitRefreshItem { + workspace_id: String, + resolved_identity_cwd: PathBuf, + cache_key_hint: Option, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +struct WorkspaceGitRefreshTarget { + workspace_id: String, + resolved_identity_cwd: PathBuf, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +struct WorkspaceGitRefreshJob { + cache_key: PathBuf, + cached: Option, + targets: Vec, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +struct WorkspaceGitRefreshOutput { + results: Vec, + cache_updates: Vec<(PathBuf, GitStatusCacheEntry)>, +} + +impl App { + pub(crate) fn start_git_status_refresh_if_due(&mut self, now: Instant) { + let Some(deadline) = self.git_refresh_deadline() else { + return; + }; + + if now < deadline { + return; + } + + let refresh_repo_discovery = self.git_identity_refresh_requested + || now.saturating_duration_since(self.last_git_repo_discovery_refresh) + >= GIT_REPO_DISCOVERY_REFRESH_INTERVAL; + let workspaces = self.workspace_git_refresh_items(refresh_repo_discovery); + + if workspaces.is_empty() { + self.last_git_remote_status_refresh = now; + self.git_identity_refresh_requested = false; + return; + } + + self.git_refresh_in_flight = true; + let event_tx = self.event_tx.clone(); + let cache = self.git_status_cache.clone(); + let mut demand = self.git_refresh_demand(); + if self.git_identity_refresh_requested { + demand.branch = true; + } + self.git_identity_refresh_requested = false; + if refresh_repo_discovery { + self.last_git_repo_discovery_refresh = now; + } + std::thread::spawn(move || { + let output = + refresh_workspace_git_statuses_with_cache_and_demand(workspaces, &cache, demand); + let _ = event_tx.blocking_send(AppEvent::GitStatusRefreshed { + results: output.results, + cache_updates: output.cache_updates, + }); + }); + } + + pub(crate) fn request_git_identity_refresh(&mut self, now: Instant) { + self.git_identity_refresh_requested = true; + self.mark_git_status_refresh_due(now); + } + + pub(crate) fn mark_git_status_refresh_due(&mut self, now: Instant) { + self.git_status_cache + .retain(|_, entry| entry.fingerprint.is_some()); + if self.git_refresh_in_flight { + self.git_refresh_due_after_in_flight = true; + return; + } + self.last_git_remote_status_refresh = now + .checked_sub(GIT_REMOTE_STATUS_REFRESH_INTERVAL) + .unwrap_or(now); + self.git_refresh_due_after_in_flight = false; + } + + pub(crate) fn git_refresh_deadline(&self) -> Option { + (!self.git_refresh_in_flight + && !self.state.workspaces.is_empty() + && (self.git_identity_refresh_requested || !self.git_refresh_demand().is_empty())) + .then_some(self.last_git_remote_status_refresh + GIT_REMOTE_STATUS_REFRESH_INTERVAL) + } + + fn git_refresh_demand(&self) -> GitStatusRefreshDemand { + let mut demand = GitStatusRefreshDemand::default(); + for token in self.state.sidebar_spaces.rows.iter().flatten() { + match token.parts().0 { + crate::config::SpaceSidebarToken::Branch => demand.branch = true, + crate::config::SpaceSidebarToken::GitStatus => demand.ahead_behind = true, + _ => {} + } + } + demand + } + + fn workspace_git_refresh_items( + &self, + refresh_repo_discovery: bool, + ) -> Vec { + self.state + .workspaces + .iter() + .filter_map(|ws| { + let cwd = + ws.resolved_identity_cwd_from(&self.state.terminals, &self.terminal_runtimes)?; + let cache_key_hint = (!refresh_repo_discovery && ws.cached_identity_cwd == cwd) + .then(|| ws.cached_git_status_key.clone()); + Some(WorkspaceGitRefreshItem { + workspace_id: ws.id.clone(), + resolved_identity_cwd: cwd, + cache_key_hint, + }) + }) + .collect() + } +} + +fn deduplicate_git_refresh_items( + items: Vec, + cache: &HashMap, +) -> Vec { + let mut indexes = HashMap::::new(); + let mut jobs = Vec::::new(); + + for item in items { + let cache_key = item.cache_key_hint.unwrap_or_else(|| { + crate::workspace::git_status_cache_key(&item.resolved_identity_cwd) + .unwrap_or_else(|| item.resolved_identity_cwd.clone()) + }); + let target = WorkspaceGitRefreshTarget { + workspace_id: item.workspace_id, + resolved_identity_cwd: item.resolved_identity_cwd, + }; + if let Some(&index) = indexes.get(&cache_key) { + jobs[index].targets.push(target); + continue; + } + + let cached = cache.get(&cache_key).cloned(); + indexes.insert(cache_key.clone(), jobs.len()); + jobs.push(WorkspaceGitRefreshJob { + cache_key, + cached, + targets: vec![target], + }); + } + + jobs +} + +fn refresh_workspace_git_statuses_with_cache_and_demand( + items: Vec, + cache: &HashMap, + demand: GitStatusRefreshDemand, +) -> WorkspaceGitRefreshOutput { + let mut results = Vec::new(); + let mut cache_updates = Vec::new(); + + for job in deduplicate_git_refresh_items(items, cache) { + let (snapshot, cache_entry) = crate::workspace::git_status_snapshot_for_cwd_with_demand( + &job.cache_key, + job.cached.as_ref(), + demand, + ); + if let Some(cache_entry) = cache_entry { + cache_updates.push((job.cache_key.clone(), cache_entry)); + } + results.extend(job.targets.into_iter().map(move |target| { + snapshot.clone().into_workspace_status( + target.workspace_id, + target.resolved_identity_cwd, + job.cache_key.clone(), + demand, + ) + })); + } + + WorkspaceGitRefreshOutput { + results, + cache_updates, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::workspace::Workspace; + + #[test] + fn git_refresh_deduplicates_workspaces_with_same_cache_key() { + let repo = + std::env::temp_dir().join(format!("herdr-git-refresh-dedupe-{}", std::process::id())); + let nested = repo.join("nested"); + let other = repo.join("other"); + std::fs::create_dir_all(&nested).expect("create nested dir"); + std::fs::create_dir_all(&other).expect("create other dir"); + std::process::Command::new("git") + .arg("-C") + .arg(&repo) + .arg("init") + .output() + .expect("run git init"); + + let output = refresh_workspace_git_statuses_with_cache_and_demand( + vec![ + WorkspaceGitRefreshItem { + workspace_id: "one".into(), + resolved_identity_cwd: nested.clone(), + cache_key_hint: None, + }, + WorkspaceGitRefreshItem { + workspace_id: "two".into(), + resolved_identity_cwd: other.clone(), + cache_key_hint: None, + }, + ], + &HashMap::new(), + GitStatusRefreshDemand::ALL, + ); + + assert_eq!(output.cache_updates.len(), 1); + assert_eq!( + output.cache_updates[0].0, + std::fs::canonicalize(&repo).expect("canonical repo path") + ); + assert_eq!(output.results.len(), 2); + assert_eq!(output.results[0].workspace_id, "one"); + assert_eq!(output.results[0].resolved_identity_cwd, nested); + assert_eq!(output.results[1].workspace_id, "two"); + assert_eq!(output.results[1].resolved_identity_cwd, other); + + let _ = std::fs::remove_dir_all(repo); + } + + #[test] + fn git_refresh_item_collection_does_not_discover_uncached_cwd() { + let mut app = test_app(&crate::config::Config::default()); + let cwd = std::env::temp_dir().join(format!("herdr-uncached-cwd-{}", std::process::id())); + let mut ws = Workspace::test_new("test"); + ws.identity_cwd = cwd.clone(); + ws.tabs.clear(); + app.state.workspaces.push(ws); + + let items = app.workspace_git_refresh_items(false); + + assert_eq!(items.len(), 1); + assert_eq!(items[0].resolved_identity_cwd, cwd); + assert_eq!(items[0].cache_key_hint, None); + } + + #[test] + fn git_refresh_item_collection_reuses_matching_cached_key() { + let mut app = test_app(&crate::config::Config::default()); + let cwd = PathBuf::from("/repo/deep/nested"); + let cache_key = PathBuf::from("/repo"); + let mut ws = Workspace::test_new("test"); + ws.identity_cwd = cwd.clone(); + ws.cached_identity_cwd = cwd; + ws.cached_git_status_key = cache_key.clone(); + ws.tabs.clear(); + app.state.workspaces.push(ws); + + let items = app.workspace_git_refresh_items(false); + + assert_eq!(items.len(), 1); + assert_eq!(items[0].cache_key_hint, Some(cache_key)); + } + + #[test] + fn periodic_repo_discovery_ignores_cached_key_hints() { + let mut app = test_app(&crate::config::Config::default()); + let cwd = PathBuf::from("/repo/deep/nested"); + let mut ws = Workspace::test_new("test"); + ws.identity_cwd = cwd.clone(); + ws.cached_identity_cwd = cwd; + ws.cached_git_status_key = PathBuf::from("/repo"); + ws.tabs.clear(); + app.state.workspaces.push(ws); + + let items = app.workspace_git_refresh_items(true); + + assert_eq!(items.len(), 1); + assert_eq!(items[0].cache_key_hint, None); + } + + #[test] + fn cwd_identity_refresh_runs_once_without_sidebar_git_tokens() { + let mut config = crate::config::Config::default(); + config.ui.sidebar.spaces.rows = vec![vec![crate::config::SpaceSidebarToken::Workspace]]; + let mut app = test_app(&config); + app.state.workspaces.push(Workspace::test_new("test")); + let now = Instant::now(); + + app.request_git_identity_refresh(now); + + assert!(app.git_refresh_deadline().is_some()); + app.start_git_status_refresh_if_due(now); + assert!(app.git_refresh_in_flight); + assert!(!app.git_identity_refresh_requested); + } + + #[test] + fn due_git_refresh_does_not_start_without_sidebar_consumer() { + let mut config = crate::config::Config::default(); + config.ui.sidebar.spaces.rows = vec![vec![crate::config::SpaceSidebarToken::Workspace]]; + let mut app = test_app(&config); + app.state.workspaces.push(Workspace::test_new("test")); + let now = Instant::now(); + app.last_git_remote_status_refresh = now - GIT_REMOTE_STATUS_REFRESH_INTERVAL; + + app.start_git_status_refresh_if_due(now); + + assert!(!app.git_refresh_in_flight); + assert!(app.event_rx.try_recv().is_err()); + } + + #[test] + fn git_refresh_demand_matches_sidebar_rows() { + let cases = [ + ( + crate::config::SpaceSidebarToken::Workspace, + GitStatusRefreshDemand::default(), + ), + ( + crate::config::SpaceSidebarToken::Branch, + GitStatusRefreshDemand { + branch: true, + ahead_behind: false, + }, + ), + ( + crate::config::SpaceSidebarToken::GitStatus, + GitStatusRefreshDemand { + branch: false, + ahead_behind: true, + }, + ), + ]; + + for (token, expected) in cases { + let mut config = crate::config::Config::default(); + config.ui.sidebar.spaces.rows = vec![vec![token.clone()]]; + let mut app = test_app(&config); + app.state.workspaces.push(Workspace::test_new("test")); + + assert_eq!(app.git_refresh_demand(), expected, "token: {token:?}"); + assert_eq!( + app.git_refresh_deadline().is_some(), + !expected.is_empty(), + "token: {token:?}" + ); + } + } + + #[test] + fn unnamed_linked_worktree_does_not_force_periodic_branch_refresh() { + let mut config = crate::config::Config::default(); + config.ui.sidebar.spaces.rows = vec![vec![crate::config::SpaceSidebarToken::Workspace]]; + let mut app = test_app(&config); + let mut child = Workspace::test_new("test"); + child.custom_name = None; + child.worktree_space = Some(crate::workspace::WorktreeSpaceMembership { + key: "repo".into(), + label: "repo".into(), + repo_root: "/repo".into(), + checkout_path: "/repo-worktree".into(), + is_linked_worktree: true, + }); + app.state.workspaces.push(child); + + assert_eq!(app.git_refresh_deadline(), None); + } + + #[test] + fn custom_named_linked_worktree_does_not_require_branch_refresh() { + let mut config = crate::config::Config::default(); + config.ui.sidebar.spaces.rows = vec![vec![crate::config::SpaceSidebarToken::Workspace]]; + let mut app = test_app(&config); + let mut child = Workspace::test_new("custom"); + child.worktree_space = Some(crate::workspace::WorktreeSpaceMembership { + key: "repo".into(), + label: "repo".into(), + repo_root: "/repo".into(), + checkout_path: "/repo-worktree".into(), + is_linked_worktree: true, + }); + app.state.workspaces.push(child); + + assert_eq!(app.git_refresh_deadline(), None); + } + + #[test] + fn headless_deadline_can_suppress_git_refresh_timer() { + let mut app = test_app(&crate::config::Config::default()); + app.state.workspaces.push(Workspace::test_new("test")); + let now = Instant::now(); + app.last_git_remote_status_refresh = now - GIT_REMOTE_STATUS_REFRESH_INTERVAL; + + assert_eq!( + app.next_headless_loop_deadline_with_git_refresh(now, false, false), + None + ); + assert_eq!( + app.next_headless_loop_deadline_with_git_refresh(now, false, true), + Some(now) + ); + } + + #[test] + fn explicit_git_refresh_invalidates_cached_non_git_results() { + let mut app = test_app(&crate::config::Config::default()); + let cwd = std::env::temp_dir().join(format!("herdr-git-miss-{}", std::process::id())); + std::fs::create_dir_all(&cwd).unwrap(); + let (_, entry) = crate::workspace::git_status_snapshot_for_cwd_with_demand( + &cwd, + None, + GitStatusRefreshDemand::ALL, + ); + app.git_status_cache + .insert(cwd.clone(), entry.expect("non-Git cache entry")); + + app.mark_git_status_refresh_due(Instant::now()); + + assert!(app.git_status_cache.is_empty()); + std::fs::remove_dir_all(cwd).unwrap(); + } + + #[test] + fn git_refresh_due_request_survives_in_flight_refresh() { + let mut app = test_app(&crate::config::Config::default()); + let now = Instant::now(); + app.git_refresh_in_flight = true; + + app.mark_git_status_refresh_due(now); + assert!(app.git_refresh_due_after_in_flight); + + app.handle_internal_event(AppEvent::GitStatusRefreshed { + results: Vec::new(), + cache_updates: Vec::new(), + }); + + assert!(!app.git_refresh_in_flight); + assert!(!app.git_refresh_due_after_in_flight); + assert_eq!(app.git_refresh_deadline(), None); + + app.state.workspaces.push(Workspace::test_new("test")); + let deadline = app + .git_refresh_deadline() + .expect("refresh should be due once a workspace exists"); + assert!(deadline <= Instant::now()); + } + + fn test_app(config: &crate::config::Config) -> super::super::App { + super::super::App::new( + config, + true, + None, + tokio::sync::mpsc::unbounded_channel().1, + crate::api::EventHub::default(), + ) + } +} diff --git a/src/app/mod.rs b/src/app/mod.rs index 84f6d917..c2d479b3 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -12,6 +12,7 @@ mod api; mod api_helpers; mod config_io; mod creation; +mod git_refresh; mod ids; mod input; mod popup; @@ -38,6 +39,7 @@ pub(crate) const HEADLESS_ANIMATION_TICK_STEP: u32 = 8; pub(crate) const SELECTION_AUTOSCROLL_INTERVAL: Duration = Duration::from_millis(30); const RESIZE_POLL_INTERVAL: Duration = Duration::from_millis(100); const GIT_REMOTE_STATUS_REFRESH_INTERVAL: Duration = Duration::from_millis(1500); +const GIT_REPO_DISCOVERY_REFRESH_INTERVAL: Duration = Duration::from_secs(5 * 60); const AUTO_UPDATE_CHECK_INTERVAL: Duration = Duration::from_secs(30 * 60); const PENDING_AGENT_RESUME_THEME_WAIT: Duration = Duration::from_millis(750); const SESSION_SAVE_DEBOUNCE: Duration = Duration::from_secs(5); @@ -110,8 +112,10 @@ pub struct App { pub(crate) copy_feedback_deadline: Option, pub(crate) last_api_notification_at: Option, pub(crate) last_git_remote_status_refresh: Instant, + pub(crate) last_git_repo_discovery_refresh: Instant, pub(crate) git_refresh_in_flight: bool, pub(crate) git_refresh_due_after_in_flight: bool, + pub(crate) git_identity_refresh_requested: bool, pub(crate) git_status_cache: HashMap, pub(crate) pending_api_worktree_creates: HashMap, pub(crate) pending_api_worktree_removes: HashMap, @@ -736,8 +740,10 @@ impl App { event_tx, event_rx, last_git_remote_status_refresh: Instant::now() - GIT_REMOTE_STATUS_REFRESH_INTERVAL, + last_git_repo_discovery_refresh: Instant::now(), git_refresh_in_flight: false, git_refresh_due_after_in_flight: false, + git_identity_refresh_requested: false, git_status_cache: HashMap::new(), pending_api_worktree_creates: HashMap::new(), pending_api_worktree_removes: HashMap::new(), @@ -892,10 +898,11 @@ impl App { pub(crate) fn handle_internal_event_with_prefix_sync( &mut self, event: crate::events::AppEvent, - ) { + ) -> bool { let previous_mode = self.state.mode; - self.handle_internal_event(event); + let changed = self.handle_internal_event_with_render_impact(event); self.sync_prefix_input_source(previous_mode); + changed } #[cfg(test)] @@ -1126,8 +1133,9 @@ impl App { match event { LoopEvent::Timer => {} LoopEvent::Internal(ev) => { - self.handle_internal_event_with_prefix_sync(ev); - needs_render = true; + if self.handle_internal_event_with_prefix_sync(ev) { + needs_render = true; + } } LoopEvent::Api(msg) => { if self.handle_api_request_message(*msg) { @@ -2130,6 +2138,20 @@ mod tests { assert_eq!(app.git_refresh_deadline(), None); } + #[test] + fn unchanged_git_status_event_has_no_render_impact() { + let mut app = test_app(); + app.git_refresh_in_flight = true; + + let changed = app.handle_internal_event_with_prefix_sync(AppEvent::GitStatusRefreshed { + results: Vec::new(), + cache_updates: Vec::new(), + }); + + assert!(!changed); + assert!(!app.git_refresh_in_flight); + } + #[test] fn git_status_event_clears_in_flight_refresh() { let mut app = test_app(); @@ -2157,7 +2179,10 @@ mod tests { app.handle_internal_event(AppEvent::GitStatusRefreshed { results: vec![crate::workspace::WorkspaceGitStatus { workspace_id, - resolved_identity_cwd, + resolved_identity_cwd: resolved_identity_cwd.clone(), + status_cache_key: resolved_identity_cwd, + demand: crate::workspace::GitStatusRefreshDemand::ALL, + auto_label: "one".into(), branch: Some("render-dirty-test".into()), ahead_behind: Some((1, 0)), space: None, @@ -2387,6 +2412,21 @@ mod tests { assert!(app.state.toast.is_none()); } + #[test] + fn unchanged_git_status_drain_has_no_render_impact() { + let mut app = test_app(); + app.git_refresh_in_flight = true; + app.event_tx + .try_send(AppEvent::GitStatusRefreshed { + results: Vec::new(), + cache_updates: Vec::new(), + }) + .unwrap(); + + assert!(!app.drain_internal_events()); + assert!(!app.git_refresh_in_flight); + } + #[test] fn internal_event_drain_limits_work_per_tick() { let mut app = test_app(); diff --git a/src/app/runtime.rs b/src/app/runtime.rs index 428cd35b..180c3cef 100644 --- a/src/app/runtime.rs +++ b/src/app/runtime.rs @@ -4,40 +4,9 @@ use crossterm::terminal; use super::{ background_update_check_enabled, pressed_key_identity, App, ANIMATION_INTERVAL, - AUTO_UPDATE_CHECK_INTERVAL, GIT_REMOTE_STATUS_REFRESH_INTERVAL, MIN_RENDER_INTERVAL, - RESIZE_POLL_INTERVAL, SELECTION_AUTOSCROLL_INTERVAL, + AUTO_UPDATE_CHECK_INTERVAL, MIN_RENDER_INTERVAL, RESIZE_POLL_INTERVAL, + SELECTION_AUTOSCROLL_INTERVAL, }; -use crate::events::AppEvent; -use crate::workspace::{GitStatusCacheEntry, Workspace, WorkspaceGitStatus}; -use std::collections::HashMap; - -#[derive(Clone, Debug, PartialEq, Eq)] -pub(crate) struct WorkspaceGitRefreshItem { - pub(crate) workspace_id: String, - pub(crate) resolved_identity_cwd: std::path::PathBuf, - pub(crate) cache_key: std::path::PathBuf, -} - -#[derive(Clone, Debug, PartialEq, Eq)] -pub(crate) struct WorkspaceGitRefreshTarget { - pub(crate) workspace_id: String, - pub(crate) resolved_identity_cwd: std::path::PathBuf, -} - -#[derive(Clone, Debug, PartialEq, Eq)] -pub(crate) struct WorkspaceGitRefreshJob { - pub(crate) cache_key: std::path::PathBuf, - pub(crate) status_cwd: std::path::PathBuf, - pub(crate) cached: Option, - pub(crate) targets: Vec, -} - -#[derive(Clone, Debug, PartialEq, Eq)] -pub(crate) struct WorkspaceGitRefreshOutput { - pub(crate) results: Vec, - pub(crate) cache_updates: Vec<(std::path::PathBuf, GitStatusCacheEntry)>, -} - fn retain_custom_command_after_wait( pid: u32, result: std::io::Result>, @@ -562,50 +531,6 @@ impl App { std::thread::spawn(move || crate::detect::manifest_update::auto_update(manifest_update_tx)); } - pub(crate) fn start_git_status_refresh_if_due(&mut self, now: Instant) { - let Some(deadline) = self.git_refresh_deadline() else { - return; - }; - - if now < deadline { - return; - } - - let workspaces = self.workspace_git_refresh_items(); - - if workspaces.is_empty() { - self.last_git_remote_status_refresh = now; - return; - } - - self.git_refresh_in_flight = true; - let event_tx = self.event_tx.clone(); - let cache = self.git_status_cache.clone(); - std::thread::spawn(move || { - let output = refresh_workspace_git_statuses_with_cache(workspaces, &cache); - let _ = event_tx.blocking_send(AppEvent::GitStatusRefreshed { - results: output.results, - cache_updates: output.cache_updates, - }); - }); - } - - pub(crate) fn mark_git_status_refresh_due(&mut self, now: Instant) { - if self.git_refresh_in_flight { - self.git_refresh_due_after_in_flight = true; - return; - } - self.last_git_remote_status_refresh = now - .checked_sub(GIT_REMOTE_STATUS_REFRESH_INTERVAL) - .unwrap_or(now); - self.git_refresh_due_after_in_flight = false; - } - - pub(crate) fn git_refresh_deadline(&self) -> Option { - (!self.git_refresh_in_flight && !self.state.workspaces.is_empty()) - .then_some(self.last_git_remote_status_refresh + GIT_REMOTE_STATUS_REFRESH_INTERVAL) - } - pub(crate) fn next_loop_deadline(&self, now: Instant, needs_render: bool) -> Option { self.next_loop_deadline_with_resize_poll(now, needs_render, true, true) } @@ -659,103 +584,35 @@ impl App { .min() } - fn workspace_git_refresh_items(&self) -> Vec { - self.state - .workspaces - .iter() - .filter_map(|ws| { - let cwd = - ws.resolved_identity_cwd_from(&self.state.terminals, &self.terminal_runtimes)?; - let git_key = crate::workspace::git_status_cache_key(&cwd); - let cache_key = git_key.unwrap_or_else(|| cwd.clone()); - Some(WorkspaceGitRefreshItem { - workspace_id: ws.id.clone(), - resolved_identity_cwd: cwd, - cache_key, - }) - }) - .collect() - } - pub(crate) fn drain_internal_events(&mut self) -> bool { self.drain_internal_events_up_to(super::APP_EVENT_DRAIN_LIMIT) + .1 } pub(crate) fn drain_all_internal_events(&mut self) -> bool { - let mut had_event = false; - while self.drain_internal_events_up_to(super::APP_EVENT_DRAIN_LIMIT) { - had_event = true; + let mut changed = false; + loop { + let (had_event, batch_changed) = + self.drain_internal_events_up_to(super::APP_EVENT_DRAIN_LIMIT); + changed |= batch_changed; + if !had_event { + break; + } } - had_event + changed } - fn drain_internal_events_up_to(&mut self, limit: usize) -> bool { + fn drain_internal_events_up_to(&mut self, limit: usize) -> (bool, bool) { let mut had_event = false; + let mut changed = false; for _ in 0..limit { let Ok(ev) = self.event_rx.try_recv() else { break; }; had_event = true; - self.handle_internal_event_with_prefix_sync(ev); + changed |= self.handle_internal_event_with_prefix_sync(ev); } - had_event - } -} - -pub(crate) fn deduplicate_git_refresh_items( - items: Vec, - cache: &HashMap, -) -> Vec { - let mut indexes = HashMap::::new(); - let mut jobs = Vec::::new(); - - for item in items { - let target = WorkspaceGitRefreshTarget { - workspace_id: item.workspace_id, - resolved_identity_cwd: item.resolved_identity_cwd.clone(), - }; - if let Some(&index) = indexes.get(&item.cache_key) { - jobs[index].targets.push(target); - continue; - } - - let status_cwd = item.cache_key.clone(); - let cached = cache.get(&item.cache_key).cloned(); - indexes.insert(item.cache_key, jobs.len()); - jobs.push(WorkspaceGitRefreshJob { - cache_key: status_cwd.clone(), - status_cwd, - cached, - targets: vec![target], - }); - } - - jobs -} - -pub(crate) fn refresh_workspace_git_statuses_with_cache( - items: Vec, - cache: &HashMap, -) -> WorkspaceGitRefreshOutput { - let mut results = Vec::new(); - let mut cache_updates = Vec::new(); - - for job in deduplicate_git_refresh_items(items, cache) { - let (snapshot, cache_entry) = - Workspace::git_status_snapshot_for_cwd_with_cache(&job.status_cwd, job.cached.as_ref()); - if let Some(cache_entry) = cache_entry { - cache_updates.push((job.cache_key.clone(), cache_entry)); - } - results.extend(job.targets.into_iter().map(move |target| { - snapshot - .clone() - .into_workspace_status(target.workspace_id, target.resolved_identity_cwd) - })); - } - - WorkspaceGitRefreshOutput { - results, - cache_updates, + (had_event, changed) } } @@ -764,7 +621,6 @@ mod tests { use super::*; use crate::app::state; use crate::workspace::Workspace; - use std::path::PathBuf; #[test] fn interrupted_custom_command_wait_keeps_child_for_retry() { @@ -796,131 +652,6 @@ mod tests { (app, pane_id) } - #[test] - fn git_refresh_deduplicates_workspaces_with_same_cache_key() { - let repo = - std::env::temp_dir().join(format!("herdr-git-refresh-dedupe-{}", std::process::id())); - let nested = repo.join("nested"); - let other = repo.join("other"); - std::fs::create_dir_all(&nested).expect("create nested dir"); - std::fs::create_dir_all(&other).expect("create other dir"); - std::process::Command::new("git") - .arg("-C") - .arg(&repo) - .arg("init") - .output() - .expect("run git init"); - - let output = refresh_workspace_git_statuses_with_cache( - vec![ - WorkspaceGitRefreshItem { - workspace_id: "one".into(), - resolved_identity_cwd: nested.clone(), - cache_key: repo.clone(), - }, - WorkspaceGitRefreshItem { - workspace_id: "two".into(), - resolved_identity_cwd: other.clone(), - cache_key: repo.clone(), - }, - ], - &HashMap::new(), - ); - - assert_eq!(output.cache_updates.len(), 1); - assert_eq!(output.cache_updates[0].0, repo); - assert_eq!(output.results.len(), 2); - assert_eq!(output.results[0].workspace_id, "one"); - assert_eq!( - output.results[0].resolved_identity_cwd, - PathBuf::from(&nested) - ); - assert_eq!(output.results[1].workspace_id, "two"); - assert_eq!( - output.results[1].resolved_identity_cwd, - PathBuf::from(&other) - ); - - let _ = std::fs::remove_dir_all(repo); - } - - #[test] - fn git_refresh_items_use_cwd_cache_key_for_non_git_cwd() { - let mut app = super::super::App::new( - &crate::config::Config::default(), - true, - None, - tokio::sync::mpsc::unbounded_channel().1, - crate::api::EventHub::default(), - ); - let cwd = std::env::temp_dir().join(format!("herdr-non-git-cwd-{}", std::process::id())); - std::fs::create_dir_all(&cwd).expect("create temp cwd"); - let mut ws = Workspace::test_new("test"); - ws.identity_cwd = cwd.clone(); - ws.tabs.clear(); - app.state.workspaces.push(ws); - - let items = app.workspace_git_refresh_items(); - - assert_eq!(items.len(), 1); - assert_eq!(items[0].cache_key, cwd); - let _ = std::fs::remove_dir_all(&cwd); - } - - #[test] - fn headless_deadline_can_suppress_git_refresh_timer() { - let mut app = super::super::App::new( - &crate::config::Config::default(), - true, - None, - tokio::sync::mpsc::unbounded_channel().1, - crate::api::EventHub::default(), - ); - app.state.workspaces.push(Workspace::test_new("test")); - let now = Instant::now(); - app.last_git_remote_status_refresh = now - super::super::GIT_REMOTE_STATUS_REFRESH_INTERVAL; - - assert_eq!( - app.next_headless_loop_deadline_with_git_refresh(now, false, false), - None - ); - assert_eq!( - app.next_headless_loop_deadline_with_git_refresh(now, false, true), - Some(now) - ); - } - - #[test] - fn git_refresh_due_request_survives_in_flight_refresh() { - let mut app = super::super::App::new( - &crate::config::Config::default(), - true, - None, - tokio::sync::mpsc::unbounded_channel().1, - crate::api::EventHub::default(), - ); - let now = Instant::now(); - app.git_refresh_in_flight = true; - - app.mark_git_status_refresh_due(now); - assert!(app.git_refresh_due_after_in_flight); - - app.handle_internal_event(crate::events::AppEvent::GitStatusRefreshed { - results: Vec::new(), - cache_updates: Vec::new(), - }); - - assert!(!app.git_refresh_in_flight); - assert!(!app.git_refresh_due_after_in_flight); - assert_eq!(app.git_refresh_deadline(), None); - - app.state.workspaces.push(Workspace::test_new("test")); - let deadline = app - .git_refresh_deadline() - .expect("refresh should be due once a workspace exists"); - assert!(deadline <= Instant::now()); - } - #[test] fn tick_selection_autoscroll_stops_when_metrics_unavailable() { // Without a runtime, pane_scroll_metrics returns None. diff --git a/src/persist/restore.rs b/src/persist/restore.rs index e442ac17..a1b07710 100644 --- a/src/persist/restore.rs +++ b/src/persist/restore.rs @@ -403,15 +403,20 @@ fn restore_workspace( } let worktree_space = restored_worktree_space_membership(snap.worktree_space.clone()); + let (cached_git_space, cached_auto_label, cached_git_status_key) = + crate::workspace::discover_workspace_git_identity(&snap.identity_cwd); ( Some(Workspace { id: workspace_id, custom_name: snap.custom_name.clone(), identity_cwd: snap.identity_cwd.clone(), + cached_identity_cwd: snap.identity_cwd.clone(), + cached_auto_label, + cached_git_status_key, cached_git_branch: crate::workspace::git_branch(&snap.identity_cwd), cached_git_ahead_behind: None, - cached_git_space: crate::workspace::git_space_metadata(&snap.identity_cwd), + cached_git_space, worktree_space, metadata_tokens: crate::metadata_tokens::MetadataTokens::default(), metadata_token_sequences: HashMap::new(), diff --git a/src/server/headless.rs b/src/server/headless.rs index 942a17f1..99702512 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -2272,10 +2272,7 @@ impl HeadlessServer { true } - _ => { - self.app.handle_internal_event(ev); - true - } + _ => self.app.handle_internal_event_with_render_impact(ev), } } @@ -5583,6 +5580,61 @@ next_tab = "" app_client_marks_git_refresh_due_on_first_attach(RenderEncoding::SemanticFrame); } + #[test] + fn unchanged_git_refresh_does_not_request_headless_render() { + let mut server = test_headless_server(); + server.app.git_refresh_in_flight = true; + let mut workspace = crate::workspace::Workspace::test_new("one"); + let workspace_id = workspace.id.clone(); + let cwd = workspace.identity_cwd.clone(); + workspace.cached_auto_label = "cached".into(); + workspace.cached_git_status_key = cwd.clone(); + workspace.cached_git_branch = None; + server.app.state.workspaces.push(workspace); + + let changed = server.handle_internal_event_with_forwarding(AppEvent::GitStatusRefreshed { + results: vec![crate::workspace::WorkspaceGitStatus { + workspace_id, + resolved_identity_cwd: cwd.clone(), + status_cache_key: cwd, + demand: crate::workspace::GitStatusRefreshDemand::ALL, + auto_label: "cached".into(), + branch: None, + ahead_behind: None, + space: None, + }], + cache_updates: Vec::new(), + }); + + assert!(!changed); + assert!(!server.app.git_refresh_in_flight); + } + + #[test] + fn changed_git_refresh_requests_headless_render() { + let mut server = test_headless_server(); + let workspace = crate::workspace::Workspace::test_new("one"); + let workspace_id = workspace.id.clone(); + let cwd = workspace.identity_cwd.clone(); + server.app.state.workspaces.push(workspace); + + let changed = server.handle_internal_event_with_forwarding(AppEvent::GitStatusRefreshed { + results: vec![crate::workspace::WorkspaceGitStatus { + workspace_id, + resolved_identity_cwd: cwd.clone(), + status_cache_key: cwd, + demand: crate::workspace::GitStatusRefreshDemand::ALL, + auto_label: "one".into(), + branch: Some("changed".into()), + ahead_behind: None, + space: None, + }], + cache_updates: Vec::new(), + }); + + assert!(changed); + } + #[test] fn terminal_attach_client_exits_when_attached_pane_dies() { let mut server = test_headless_server(); diff --git a/src/ui/sidebar.rs b/src/ui/sidebar.rs index 0ecfb652..c7da990a 100644 --- a/src/ui/sidebar.rs +++ b/src/ui/sidebar.rs @@ -197,12 +197,12 @@ fn workspace_row_height(app: &AppState, ws: &crate::workspace::Workspace, indent let (state, seen) = ws.aggregate_state(&app.terminals); let label = if indented { grouped_child_display_label( - &ws.display_name(), + &ws.display_name_from_terminals(&app.terminals), ws.branch().as_deref(), ws.custom_name.is_some(), ) } else { - ws.display_name() + ws.display_name_from_terminals(&app.terminals) }; let token_values = ws.metadata_tokens.values(); tokens::space_rows( diff --git a/src/workspace.rs b/src/workspace.rs index e90bd359..73a46d25 100644 --- a/src/workspace.rs +++ b/src/workspace.rs @@ -20,11 +20,12 @@ mod tab; #[cfg(test)] use self::git::git_ahead_behind; -pub(crate) use self::tab::MovedPane; +use self::git::git_status_cache_key_for_space; +pub(crate) use self::{git::git_status_snapshot_for_cwd_with_demand, tab::MovedPane}; pub use self::{ git::{ - derive_label_from_cwd, git_branch, git_space_metadata, git_status_cache_key, - GitSpaceMetadata, GitStatusCacheEntry, + derive_label_from_cwd, fallback_label_from_cwd, git_branch, git_space_metadata, + git_status_cache_key, GitSpaceMetadata, GitStatusCacheEntry, GitStatusRefreshDemand, }, tab::{NewPane, Tab}, }; @@ -42,6 +43,9 @@ pub struct WorktreeSpaceMembership { pub struct WorkspaceGitStatus { pub workspace_id: String, pub resolved_identity_cwd: PathBuf, + pub status_cache_key: PathBuf, + pub demand: GitStatusRefreshDemand, + pub auto_label: String, pub branch: Option, pub ahead_behind: Option<(usize, usize)>, pub space: Option, @@ -49,20 +53,41 @@ pub struct WorkspaceGitStatus { #[derive(Debug, Clone, PartialEq, Eq)] pub struct WorkspaceGitStatusSnapshot { + pub auto_label: String, pub branch: Option, pub ahead_behind: Option<(usize, usize)>, pub space: Option, } +pub(crate) fn discover_workspace_git_identity( + cwd: &std::path::Path, +) -> (Option, String, PathBuf) { + let space = git_space_metadata(cwd); + let auto_label = space + .as_ref() + .map(|space| space.label.clone()) + .unwrap_or_else(|| fallback_label_from_cwd(cwd)); + let status_cache_key = space + .as_ref() + .map(git_status_cache_key_for_space) + .unwrap_or_else(|| cwd.to_path_buf()); + (space, auto_label, status_cache_key) +} + impl WorkspaceGitStatusSnapshot { pub fn into_workspace_status( self, workspace_id: String, resolved_identity_cwd: PathBuf, + status_cache_key: PathBuf, + demand: GitStatusRefreshDemand, ) -> WorkspaceGitStatus { WorkspaceGitStatus { workspace_id, resolved_identity_cwd, + status_cache_key, + demand, + auto_label: self.auto_label, branch: self.branch, ahead_behind: self.ahead_behind, space: self.space, @@ -149,6 +174,12 @@ pub struct Workspace { pub custom_name: Option, /// Fallback workspace identity source for tests, old snapshots, or missing runtimes. pub identity_cwd: PathBuf, + /// CWD from which the cached automatic label and Git metadata were derived. + pub(crate) cached_identity_cwd: PathBuf, + /// Automatic workspace label cached outside the render path. + pub(crate) cached_auto_label: String, + /// Cache key for periodic Git status associated with `cached_identity_cwd`. + pub(crate) cached_git_status_key: PathBuf, /// Cached current git branch for the workspace repo. pub(crate) cached_git_branch: Option, /// Cached ahead/behind counts for the workspace repo's current branch upstream. @@ -210,13 +241,18 @@ impl Workspace { let tab = Tab::from_existing_pane(1, tab_label, moved, events, render_notify, render_dirty); let mut public_pane_numbers = HashMap::new(); public_pane_numbers.insert(root_pane, 1); + let (cached_git_space, cached_auto_label, cached_git_status_key) = + discover_workspace_git_identity(&identity_cwd); Self { id, custom_name: label, identity_cwd: identity_cwd.clone(), + cached_identity_cwd: identity_cwd.clone(), + cached_auto_label, + cached_git_status_key, cached_git_branch: git_branch(&identity_cwd), cached_git_ahead_behind: None, - cached_git_space: git_space_metadata(&identity_cwd), + cached_git_space, worktree_space: None, metadata_tokens: crate::metadata_tokens::MetadataTokens::default(), metadata_token_sequences: HashMap::new(), @@ -392,14 +428,19 @@ impl Workspace { }; let mut public_pane_numbers = HashMap::new(); public_pane_numbers.insert(tab.root_pane, 1); + let (cached_git_space, cached_auto_label, cached_git_status_key) = + discover_workspace_git_identity(&initial_cwd); Ok(( Self { id, custom_name: None, identity_cwd: initial_cwd.clone(), + cached_identity_cwd: initial_cwd.clone(), + cached_auto_label, + cached_git_status_key, cached_git_branch: git_branch(&initial_cwd), cached_git_ahead_behind: None, - cached_git_space: None, + cached_git_space, worktree_space: None, metadata_tokens: crate::metadata_tokens::MetadataTokens::default(), metadata_token_sequences: HashMap::new(), @@ -1036,6 +1077,7 @@ impl Workspace { self.custom_name = Some(name); } + #[cfg(test)] pub fn resolved_identity_cwd(&self) -> Option { Some(self.identity_cwd.clone()) } @@ -1051,14 +1093,31 @@ impl Workspace { .or_else(|| Some(self.identity_cwd.clone())) } + #[cfg(test)] pub fn display_name(&self) -> String { if let Some(name) = &self.custom_name { return name.clone(); } - self.resolved_identity_cwd() - .map(|cwd| derive_label_from_cwd(&cwd)) - .unwrap_or_else(|| "workspace".into()) + self.automatic_display_name_for_cwd(&self.identity_cwd) + } + + pub(crate) fn display_name_from_terminals( + &self, + terminals: &HashMap, + ) -> String { + if let Some(name) = &self.custom_name { + return name.clone(); + } + + let cwd = self + .tabs + .first() + .and_then(|tab| tab.terminal_id(tab.root_pane)) + .and_then(|terminal_id| terminals.get(terminal_id)) + .map(|terminal| &terminal.cwd) + .unwrap_or(&self.identity_cwd); + self.automatic_display_name_for_cwd(cwd) } pub fn display_name_from( @@ -1071,10 +1130,18 @@ impl Workspace { } self.resolved_identity_cwd_from(terminals, terminal_runtimes) - .map(|cwd| derive_label_from_cwd(&cwd)) + .map(|cwd| self.automatic_display_name_for_cwd(&cwd)) .unwrap_or_else(|| "workspace".into()) } + fn automatic_display_name_for_cwd(&self, cwd: &std::path::Path) -> String { + if cwd == self.cached_identity_cwd { + self.cached_auto_label.clone() + } else { + fallback_label_from_cwd(cwd) + } + } + pub fn branch(&self) -> Option { self.cached_git_branch.clone() } @@ -1099,13 +1166,6 @@ impl Workspace { self.cached_git_space = cwd.as_deref().and_then(git_space_metadata); } - pub fn git_status_snapshot_for_cwd_with_cache( - resolved_identity_cwd: &std::path::Path, - cached: Option<&GitStatusCacheEntry>, - ) -> (WorkspaceGitStatusSnapshot, Option) { - self::git::git_status_snapshot_for_cwd(resolved_identity_cwd, cached) - } - pub fn find_tab_index_for_pane(&self, pane_id: PaneId) -> Option { self.tabs .iter() @@ -1210,6 +1270,9 @@ impl Workspace { id: generate_workspace_id(), custom_name: Some(name.to_string()), identity_cwd: identity_cwd.clone(), + cached_identity_cwd: identity_cwd.clone(), + cached_auto_label: fallback_label_from_cwd(&identity_cwd), + cached_git_status_key: identity_cwd.clone(), cached_git_branch: git_branch(&identity_cwd), cached_git_ahead_behind: None, cached_git_space: None, @@ -1571,6 +1634,92 @@ mod tests { assert!(!target.tabs[0].panes.contains_key(&source_pane)); } + #[tokio::test] + async fn new_workspace_retains_discovered_git_metadata() { + let stamp = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("clock should be after unix epoch") + .as_nanos(); + let root = std::env::temp_dir().join(format!( + "herdr-workspace-git-metadata-{}-{stamp}", + std::process::id() + )); + std::fs::create_dir_all(root.join(".git")).expect("create git directory"); + std::fs::write(root.join(".git/HEAD"), "ref: refs/heads/main\n").expect("write git head"); + #[cfg(windows)] + let command = "C:\\Windows\\System32\\whoami.exe"; + #[cfg(not(windows))] + let command = "/usr/bin/true"; + let argv = vec![command.to_string()]; + let (events, _) = mpsc::channel(64); + let render_notify = Arc::new(Notify::new()); + let render_dirty = Arc::new(AtomicBool::new(false)); + + let (workspace, _terminal, runtime) = Workspace::new_argv_command( + root.clone(), + 24, + 80, + &argv, + 1024, + crate::terminal_theme::TerminalTheme::default(), + events, + render_notify, + render_dirty, + ) + .expect("create workspace"); + + let space = workspace + .git_space() + .expect("workspace should retain discovered git metadata"); + assert_eq!(space.repo_root, root); + + runtime.shutdown(); + std::fs::remove_dir_all(root).expect("remove test repo"); + } + + #[test] + fn display_name_reads_cached_identity_without_rechecking_filesystem() { + let stamp = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("clock should be after unix epoch") + .as_nanos(); + let root = std::env::temp_dir().join(format!( + "herdr-workspace-label-cache-{}-{stamp}", + std::process::id() + )); + let cwd = root.join("deep/nested"); + std::fs::create_dir_all(&cwd).expect("create nested cwd"); + + let mut ws = Workspace::test_new("ignored"); + ws.custom_name = None; + ws.identity_cwd = cwd.clone(); + ws.tabs.clear(); + ws.cached_identity_cwd = cwd; + ws.cached_auto_label = "cached-repo".into(); + + std::fs::remove_dir_all(root).expect("remove cwd after cache admission"); + + assert_eq!(ws.display_name(), "cached-repo"); + } + + #[test] + fn terminal_aware_display_name_uses_latest_admitted_identity_cache() { + let mut ws = Workspace::test_new("ignored"); + let root_pane = ws.tabs[0].root_pane; + let terminal_id = ws.tabs[0].terminal_id(root_pane).unwrap().clone(); + ws.custom_name = None; + ws.identity_cwd = PathBuf::from("/old/workspace"); + ws.cached_identity_cwd = PathBuf::from("/new/repo/deep"); + ws.cached_auto_label = "repo".into(); + let terminals = HashMap::from([( + terminal_id.clone(), + TerminalState::new(terminal_id, PathBuf::from("/new/repo/deep")), + )]); + + assert_eq!(ws.display_name_from_terminals(&terminals), "repo"); + assert_eq!(ws.display_name(), "workspace"); + } + #[test] fn workspace_identity_follows_first_tab_root_pane_cwd() { let mut ws = Workspace::test_new("ignored"); diff --git a/src/workspace/git/discovery.rs b/src/workspace/git/discovery.rs index 4edfcbc2..9e1be74d 100644 --- a/src/workspace/git/discovery.rs +++ b/src/workspace/git/discovery.rs @@ -25,6 +25,10 @@ pub fn derive_label_from_cwd(cwd: &Path) -> String { } } + fallback_label_from_cwd(cwd) +} + +pub fn fallback_label_from_cwd(cwd: &Path) -> String { if let Ok(home) = std::env::var("HOME") { let home = Path::new(&home); if cwd == home { @@ -56,9 +60,11 @@ pub fn git_worktree_info(cwd: &Path) -> Option { } pub fn git_space_metadata(cwd: &Path) -> Option { - git_repo_root(cwd)?; - let info = git_worktree_info(cwd)?; + Some(git_space_metadata_from_info(&info)) +} + +pub(super) fn git_space_metadata_from_info(info: &GitWorktreeInfo) -> GitSpaceMetadata { let key = canonicalize_best_effort_path(&info.git_common_dir) .display() .to_string(); @@ -80,13 +86,13 @@ pub fn git_space_metadata(cwd: &Path) -> Option { .and_then(|name| name.to_str()) .unwrap_or("repo") .to_string(); - Some(GitSpaceMetadata { + GitSpaceMetadata { key, checkout_key, label, - repo_root: info.repo_root, + repo_root: info.repo_root.clone(), is_linked_worktree: info.is_linked_worktree, - }) + } } pub(super) fn canonicalize_best_effort_path(path: &Path) -> PathBuf { diff --git a/src/workspace/git/mod.rs b/src/workspace/git/mod.rs index 76dc4eb4..87a6b06e 100644 --- a/src/workspace/git/mod.rs +++ b/src/workspace/git/mod.rs @@ -7,8 +7,14 @@ mod status; mod test_support; pub use self::{ - discovery::{derive_label_from_cwd, git_branch, git_space_metadata, GitSpaceMetadata}, - status::{git_status_cache_key, git_status_snapshot_for_cwd, GitStatusCacheEntry}, + discovery::{ + derive_label_from_cwd, fallback_label_from_cwd, git_branch, git_space_metadata, + GitSpaceMetadata, + }, + status::{ + git_status_cache_key, git_status_cache_key_for_space, + git_status_snapshot_for_cwd_with_demand, GitStatusCacheEntry, GitStatusRefreshDemand, + }, }; #[cfg(test)] diff --git a/src/workspace/git/status.rs b/src/workspace/git/status.rs index d9c3123a..fe873e29 100644 --- a/src/workspace/git/status.rs +++ b/src/workspace/git/status.rs @@ -1,19 +1,39 @@ use std::path::{Path, PathBuf}; +use std::time::{Duration, Instant}; -use crate::workspace::WorkspaceGitStatusSnapshot; +use crate::workspace::{GitSpaceMetadata, WorkspaceGitStatusSnapshot}; use super::{ config::{read_branch_config, upstream_full_ref}, discovery::{ - canonicalize_best_effort_path, git_branch, git_ref_storage_is_reftable, - git_rev_parse_verify, git_space_metadata, git_symbolic_head_full, git_worktree_info, - read_ref_oid, GitWorktreeInfo, + canonicalize_best_effort_path, fallback_label_from_cwd, git_ref_storage_is_reftable, + git_rev_parse_verify, git_space_metadata_from_info, git_symbolic_head_full, + git_worktree_info, read_ref_oid, GitWorktreeInfo, }, }; +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct GitStatusRefreshDemand { + pub branch: bool, + pub ahead_behind: bool, +} + +impl GitStatusRefreshDemand { + #[cfg(test)] + pub const ALL: Self = Self { + branch: true, + ahead_behind: true, + }; + + pub fn is_empty(self) -> bool { + !self.branch && !self.ahead_behind + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct GitStatusCacheEntry { - pub fingerprint: GitStatusFingerprint, + pub fingerprint: Option, + pub retry_after: Option, pub snapshot: WorkspaceGitStatusSnapshot, } @@ -49,33 +69,97 @@ pub fn git_status_cache_key(cwd: &Path) -> Option { git_worktree_info(cwd).map(|info| canonicalize_best_effort_path(&info.repo_root)) } +pub fn git_status_cache_key_for_space(space: &GitSpaceMetadata) -> PathBuf { + canonicalize_best_effort_path(&space.repo_root) +} + +#[cfg(test)] pub fn git_status_snapshot_for_cwd( cwd: &Path, cached: Option<&GitStatusCacheEntry>, ) -> (WorkspaceGitStatusSnapshot, Option) { - let space = git_space_metadata(cwd); - let Some(fingerprint) = git_status_fingerprint(cwd) else { + git_status_snapshot_for_cwd_with_demand(cwd, cached, GitStatusRefreshDemand::ALL) +} + +pub fn git_status_snapshot_for_cwd_with_demand( + cwd: &Path, + cached: Option<&GitStatusCacheEntry>, + demand: GitStatusRefreshDemand, +) -> (WorkspaceGitStatusSnapshot, Option) { + if let Some(cached) = cached.filter(|entry| { + entry.fingerprint.is_none() + && entry + .retry_after + .is_some_and(|retry_after| retry_after > Instant::now()) + }) { + return (cached.snapshot.clone(), Some(cached.clone())); + } + + let Some(info) = git_worktree_info(cwd) else { + let snapshot = WorkspaceGitStatusSnapshot { + auto_label: fallback_label_from_cwd(cwd), + branch: None, + ahead_behind: None, + space: None, + }; + return ( + snapshot.clone(), + Some(GitStatusCacheEntry { + fingerprint: None, + retry_after: Some(Instant::now() + Duration::from_secs(30)), + snapshot, + }), + ); + }; + let space = git_space_metadata_from_info(&info); + let auto_label = space.label.clone(); + + if !demand.ahead_behind { + let branch = demand + .branch + .then(|| { + read_head_identity(&info).and_then(|head| match head { + GitHeadIdentity::Branch { short_name, .. } => Some(short_name), + GitHeadIdentity::Detached { .. } => None, + }) + }) + .flatten(); return ( WorkspaceGitStatusSnapshot { - branch: git_branch(cwd), + auto_label, + branch, ahead_behind: None, - space, + space: Some(space), + }, + None, + ); + } + + let Some(fingerprint) = git_status_fingerprint_from_info(&info) else { + return ( + WorkspaceGitStatusSnapshot { + auto_label, + branch: None, + ahead_behind: None, + space: Some(space), }, None, ); }; let branch = fingerprint.branch_name().map(str::to_string); - if let Some(cached) = cached.filter(|entry| entry.fingerprint == fingerprint) { + if let Some(cached) = cached.filter(|entry| entry.fingerprint.as_ref() == Some(&fingerprint)) { let snapshot = WorkspaceGitStatusSnapshot { + auto_label, branch, ahead_behind: cached.snapshot.ahead_behind, - space, + space: Some(space), }; return ( snapshot.clone(), Some(GitStatusCacheEntry { - fingerprint, + fingerprint: Some(fingerprint), + retry_after: None, snapshot, }), ); @@ -86,24 +170,31 @@ pub fn git_status_snapshot_for_cwd( .zip(fingerprint.upstream_oid()) .and_then(|(head_oid, upstream_oid)| git_ahead_behind_between(cwd, head_oid, upstream_oid)); let snapshot = WorkspaceGitStatusSnapshot { + auto_label, branch, ahead_behind, - space, + space: Some(space), }; ( snapshot.clone(), Some(GitStatusCacheEntry { - fingerprint, + fingerprint: Some(fingerprint), + retry_after: None, snapshot, }), ) } +#[cfg(test)] pub(super) fn git_status_fingerprint(cwd: &Path) -> Option { let info = git_worktree_info(cwd)?; - let head = read_head_identity(&info)?; + git_status_fingerprint_from_info(&info) +} + +fn git_status_fingerprint_from_info(info: &GitWorktreeInfo) -> Option { + let head = read_head_identity(info)?; let upstream = match &head { - GitHeadIdentity::Branch { short_name, .. } => read_upstream_identity(&info, short_name), + GitHeadIdentity::Branch { short_name, .. } => read_upstream_identity(info, short_name), GitHeadIdentity::Detached { .. } => None, }; @@ -243,7 +334,30 @@ fn parse_git_ahead_behind_output(stdout: &str) -> Option<(usize, usize)> { #[cfg(test)] mod tests { use super::*; - use crate::workspace::git::test_support::{run_git, temp_test_dir, write_fake_tracked_repo}; + use crate::workspace::git::{ + git_space_metadata, + test_support::{run_git, temp_test_dir, write_fake_tracked_repo}, + }; + + #[cfg(target_os = "linux")] + #[test] + fn cache_key_from_space_preserves_non_utf8_checkout_path() { + use std::os::unix::ffi::OsStringExt; + + let base = temp_test_dir("non-utf8-key"); + let root = base.join(std::ffi::OsString::from_vec(vec![ + b'r', b'e', b'p', b'o', 0x80, + ])); + write_fake_tracked_repo(&root); + let space = git_space_metadata(&root).expect("Git metadata"); + + assert_eq!( + git_status_cache_key_for_space(&space), + std::fs::canonicalize(&root).unwrap() + ); + + std::fs::remove_dir_all(base).unwrap(); + } #[test] fn git_status_cache_key_ignores_invalid_git_marker() { @@ -257,14 +371,68 @@ mod tests { std::fs::remove_dir_all(base).unwrap(); } + #[test] + fn non_git_refresh_reuses_cached_miss_without_rechecking_filesystem() { + let root = temp_test_dir("cached-miss"); + let cwd = root.join("deep/nested"); + std::fs::create_dir_all(&cwd).unwrap(); + + let (initial, cache_entry) = git_status_snapshot_for_cwd(&cwd, None); + let cache_entry = cache_entry.expect("non-Git result should be cached"); + std::fs::remove_dir_all(&root).unwrap(); + + let (cached, update) = git_status_snapshot_for_cwd(&cwd, Some(&cache_entry)); + + assert_eq!(cached, initial); + assert_eq!(update, Some(cache_entry)); + } + + #[test] + fn expired_non_git_cache_detects_repository_created_in_place() { + let root = temp_test_dir("expired-miss"); + let (_, cache_entry) = git_status_snapshot_for_cwd(&root, None); + let mut cache_entry = cache_entry.expect("non-Git result should be cached"); + cache_entry.retry_after = Some(Instant::now() - Duration::from_secs(1)); + write_fake_tracked_repo(&root); + + let (snapshot, update) = git_status_snapshot_for_cwd(&root, Some(&cache_entry)); + + assert_eq!(snapshot.branch.as_deref(), Some("main")); + assert!(update.is_some_and(|entry| entry.fingerprint.is_some())); + std::fs::remove_dir_all(root).unwrap(); + } + + #[test] + fn branch_only_refresh_skips_ahead_behind_cache_work() { + let root = temp_test_dir("branch-only"); + write_fake_tracked_repo(&root); + + let (snapshot, update) = git_status_snapshot_for_cwd_with_demand( + &root, + None, + GitStatusRefreshDemand { + branch: true, + ahead_behind: false, + }, + ); + + assert_eq!(snapshot.branch.as_deref(), Some("main")); + assert_eq!(snapshot.ahead_behind, None); + assert_eq!(update, None); + + std::fs::remove_dir_all(root).unwrap(); + } + #[test] fn git_status_reuses_cached_ahead_behind_when_fingerprint_matches() { let root = temp_test_dir("cache-hit"); write_fake_tracked_repo(&root); let fingerprint = git_status_fingerprint(&root).unwrap(); let cached = GitStatusCacheEntry { - fingerprint, + fingerprint: Some(fingerprint), + retry_after: None, snapshot: WorkspaceGitStatusSnapshot { + auto_label: "repo".into(), branch: Some("main".into()), ahead_behind: Some((2, 1)), space: git_space_metadata(&root), @@ -286,8 +454,10 @@ mod tests { write_fake_tracked_repo(&root); let fingerprint = git_status_fingerprint(&root).unwrap(); let cached = GitStatusCacheEntry { - fingerprint, + fingerprint: Some(fingerprint), + retry_after: None, snapshot: WorkspaceGitStatusSnapshot { + auto_label: "repo".into(), branch: Some("main".into()), ahead_behind: Some((4, 0)), space: git_space_metadata(&root), @@ -319,8 +489,10 @@ mod tests { write_fake_tracked_repo(&root); let fingerprint = git_status_fingerprint(&root).unwrap(); let cached = GitStatusCacheEntry { - fingerprint, + fingerprint: Some(fingerprint), + retry_after: None, snapshot: WorkspaceGitStatusSnapshot { + auto_label: "repo".into(), branch: Some("main".into()), ahead_behind: Some((0, 3)), space: git_space_metadata(&root),