diff --git a/src-tauri/crates/git/src/watch/event_emitter.rs b/src-tauri/crates/git/src/watch/event_emitter.rs index 20d7d8376..2d9a0adfe 100644 --- a/src-tauri/crates/git/src/watch/event_emitter.rs +++ b/src-tauri/crates/git/src/watch/event_emitter.rs @@ -43,13 +43,15 @@ impl EventEmitter { }); let payload = json!({ + "type": "repo:changed", "repo_id": repo_id, "change_type": change_type_str, "affected_count": affected_count, "timestamp": Self::current_timestamp_ms(), }); - let _ = self.app_handle.emit("repo:changed", payload); + let _ = self.app_handle.emit("repo:changed", payload.clone()); + crate::hooks::websocket_broadcast(payload.to_string()); } /// Emit file changed event (for Filesync channel - individual file changes) diff --git a/src-tauri/crates/search/src/file.rs b/src-tauri/crates/search/src/file.rs index 62279d27d..a8196fd40 100644 --- a/src-tauri/crates/search/src/file.rs +++ b/src-tauri/crates/search/src/file.rs @@ -15,12 +15,16 @@ use nucleo_matcher::pattern::{CaseMatching, Normalization, Pattern}; use nucleo_matcher::{Config, Matcher, Utf32Str}; use rayon::prelude::*; use serde::{Deserialize, Serialize}; -use std::collections::HashMap; use std::path::PathBuf; -use std::sync::{Arc, Mutex}; -use std::time::Instant; +use std::sync::{Arc, LazyLock}; +use std::time::{Duration, Instant}; use tracing::{debug, info, warn}; +#[path = "file/index_cache.rs"] +mod index_cache; + +use index_cache::FilePathIndexCache; + // ============================================ // Types // ============================================ @@ -62,86 +66,34 @@ struct FileEntry { is_dir: bool, } -struct FileIndex { - entries: Arc>, - _root_path: String, - indexed_at: std::time::SystemTime, - estimated_bytes: usize, -} - -/// Cache TTL — 5 minutes. The old 30 s TTL caused a cold re-walk every time -/// the user paused for half a minute between @ searches. -const CACHE_TTL_SECS: u64 = 300; +/// File changes invalidate indexes through the repository watcher. This slow +/// safety TTL only recovers from a missed watcher event; it is not a polling +/// cadence and does not create background work by itself. +const CACHE_SAFETY_TTL: Duration = Duration::from_secs(60 * 60); const MAX_CACHED_FILE_INDEXES: usize = 4; -const MAX_FILE_INDEX_BYTES: usize = 32 * 1024 * 1024; -const MAX_FILE_INDEX_CACHE_BYTES: usize = 64 * 1024 * 1024; - -static FILE_INDEX_CACHE: std::sync::LazyLock>>> = - std::sync::LazyLock::new(|| Arc::new(Mutex::new(HashMap::new()))); - -fn prune_file_index_cache(cache: &mut HashMap) { - cache.retain(|_, index| { - index - .indexed_at - .elapsed() - .map(|elapsed| elapsed.as_secs() < CACHE_TTL_SECS) - .unwrap_or(false) - }); - let mut total_bytes = cache - .values() - .map(|index| index.estimated_bytes) - .sum::(); - while cache.len() > MAX_CACHED_FILE_INDEXES || total_bytes > MAX_FILE_INDEX_CACHE_BYTES { - let Some(oldest_key) = cache - .iter() - .min_by_key(|(_, index)| index.indexed_at) - .map(|(root_path, _)| root_path.clone()) - else { - break; - }; - if let Some(removed) = cache.remove(&oldest_key) { - total_bytes = total_bytes.saturating_sub(removed.estimated_bytes); - } - } -} - -fn estimate_file_index_bytes(entries: &[FileEntry]) -> usize { - std::mem::size_of_val(entries) - + entries - .iter() - .map(|entry| entry.path.len() + entry.filename.len()) - .sum::() -} - -fn insert_file_index_cache_entry( - root_path: String, - entries: Arc>, - indexed_at: std::time::SystemTime, -) { - let estimated_bytes = estimate_file_index_bytes(&entries); - if estimated_bytes > MAX_FILE_INDEX_BYTES { - debug!( - root_path = %root_path, - entries = entries.len(), - estimated_bytes, - "search::file: index exceeds per-repository cache budget; using it for this request only" - ); - return; - } - - let mut cache = FILE_INDEX_CACHE.lock().unwrap(); - prune_file_index_cache(&mut cache); - cache.insert( - root_path.clone(), - FileIndex { - entries, - _root_path: root_path, - indexed_at, - estimated_bytes, - }, - ); - prune_file_index_cache(&mut cache); +static FILE_INDEX_CACHE: LazyLock = + LazyLock::new(|| FilePathIndexCache::new(CACHE_SAFETY_TTL, MAX_CACHED_FILE_INDEXES)); + +const DEFAULT_EXCLUDED_DIRS: &[&str] = &[ + "node_modules", + ".git", + "dist", + "build", + ".next", + "target", + ".cache", + "coverage", + "__pycache__", + ".venv", + "venv", +]; + +fn default_excluded_dirs() -> Vec { + DEFAULT_EXCLUDED_DIRS + .iter() + .map(|directory| (*directory).to_string()) + .collect() } // ============================================ @@ -173,17 +125,29 @@ fn build_file_index(root_path: &str, exclude_dirs: &[String]) -> Vec // Skip excluded directories at the walker level so we never descend // into node_modules, .git, etc. This is orders of magnitude faster // than post-filtering. + let root = std::path::PathBuf::from(root_path); + let filter_root = root.clone(); builder.filter_entry(move |entry| { if entry.file_type().is_some_and(|ft| ft.is_dir()) { let name = entry.file_name().to_string_lossy(); if exclude_set.contains(name.as_ref()) { return false; } + + // ORG2 runtime worktrees contain full repository copies and their + // generated artifacts. They are implementation state, not distinct + // user files, so descending into them multiplies every index walk. + if let Ok(relative) = entry.path().strip_prefix(&filter_root) { + if relative == std::path::Path::new(".worktrees") + || relative == std::path::Path::new(".orgii/worktrees") + { + return false; + } + } } true }); - let root = std::path::Path::new(root_path); let walker = builder.build(); let entries: Vec = walker @@ -224,42 +188,21 @@ fn build_file_index(root_path: &str, exclude_dirs: &[String]) -> Vec /// **never** during the expensive `build_file_index` walk. This means /// concurrent searches for different repos proceed in parallel, and a /// slow index build for repo A won't block a cached lookup for repo B. -fn get_file_index(root_path: &str, exclude_dirs: &[String]) -> Arc> { - // 1. Quick check under the lock — return cached entries if fresh. - { - let mut cache = FILE_INDEX_CACHE.lock().unwrap(); - prune_file_index_cache(&mut cache); - if let Some(index) = cache.get(root_path) { - if let Ok(elapsed) = index.indexed_at.elapsed() { - if elapsed.as_secs() < CACHE_TTL_SECS { - return Arc::clone(&index.entries); - } - } - } - } // ← lock released here - - // 2. Validate the path before spending time walking it. - // Protects against bad descriptors after rapid repo switches. +fn get_file_index(root_path: &str, exclude_dirs: &[String]) -> Result, String> { + // Validate the path before spending time walking it. Protects against bad + // descriptors after rapid repo switches. let root = std::path::Path::new(root_path); if !root.exists() || !root.is_dir() { warn!( root_path = %root_path, "search::file: root path invalid or gone; skipping index" ); - return Arc::new(Vec::new()); + return Ok(Arc::from(Vec::::new())); } - // 3. Build index WITHOUT holding the lock. - let entries = Arc::new(build_file_index(root_path, exclude_dirs)); - - // 4. Re-acquire lock to store. - insert_file_index_cache_entry( - root_path.to_string(), - Arc::clone(&entries), - std::time::SystemTime::now(), - ); - - entries + FILE_INDEX_CACHE.get_or_build(root_path, exclude_dirs, || { + build_file_index(root_path, exclude_dirs) + }) } // ============================================ @@ -271,29 +214,25 @@ fn score_entry( entry: &FileEntry, pattern: &Pattern, matcher: &mut Matcher, -) -> Option<(FileEntry, i64)> { - // Buffer for UTF-32 conversion - let mut buf = Vec::new(); + buf: &mut Vec, +) -> Option { + buf.clear(); // Convert filename to Utf32Str for nucleo - let filename_utf32 = Utf32Str::new(&entry.filename, &mut buf); + let filename_utf32 = Utf32Str::new(&entry.filename, buf); // Try matching against filename first (higher priority) if let Some(score) = pattern.score(filename_utf32, matcher) { // Boost filename matches significantly let boosted_score = (score as i64) * 2; - return Some((entry.clone(), boosted_score)); + return Some(boosted_score); } // Clear buffer and try matching against full path buf.clear(); - let path_utf32 = Utf32Str::new(&entry.path, &mut buf); - - if let Some(score) = pattern.score(path_utf32, matcher) { - return Some((entry.clone(), score as i64)); - } + let path_utf32 = Utf32Str::new(&entry.path, buf); - None + pattern.score(path_utf32, matcher).map(i64::from) } /// Perform fuzzy search on the file index @@ -325,9 +264,11 @@ fn fuzzy_search( ); // Use parallel processing for large indices - let results: Vec<(FileEntry, i64)> = entries + let mut scored_results: Vec<(usize, i64)> = entries .par_iter() + .enumerate() .filter(|entry| { + let entry = entry.1; // Filter by extension if specified if let Some(extensions) = file_extensions { if !entry.is_dir { @@ -339,19 +280,33 @@ fn fuzzy_search( } true }) - .filter_map(|entry| { - // Each thread gets its own matcher - let mut matcher = Matcher::new(Config::DEFAULT); - score_entry(entry, &pattern, &mut matcher) - }) + .map_init( + || (Matcher::new(Config::DEFAULT), Vec::new()), + |(matcher, buf), (index, entry)| { + score_entry(entry, &pattern, matcher, buf).map(|score| (index, score)) + }, + ) + .filter_map(|result| result) .collect(); - // Sort by score descending and take top results - let mut sorted_results = results; - sorted_results.sort_by_key(|result| std::cmp::Reverse(result.1)); - sorted_results.truncate(max_results); + if max_results == 0 { + return Vec::new(); + } + + // Partition first so only the requested top-K needs a full sort. + let compare_rank = |left: &(usize, i64), right: &(usize, i64)| { + right.1.cmp(&left.1).then_with(|| left.0.cmp(&right.0)) + }; + if scored_results.len() > max_results { + scored_results.select_nth_unstable_by(max_results, compare_rank); + scored_results.truncate(max_results); + } + scored_results.sort_unstable_by(compare_rank); - sorted_results + scored_results + .into_iter() + .map(|(index, score)| (entries[index].clone(), score)) + .collect() } // ============================================ @@ -371,30 +326,23 @@ pub async fn search_files_fuzzy(options: SearchOptions) -> Result = Vec::new(); @@ -450,33 +398,16 @@ pub async fn index_project_files( } // Default exclusions - let default_excludes = vec![ - "node_modules".to_string(), - ".git".to_string(), - "dist".to_string(), - "build".to_string(), - ".next".to_string(), - "target".to_string(), - ]; + let default_excludes = default_excluded_dirs(); let exclude_dirs = exclude_dirs.unwrap_or(default_excludes); - // Clear existing cache for this path - { - let mut cache = FILE_INDEX_CACHE.lock().unwrap(); - cache.remove(&root_path); - } - - // Build fresh index - let entries = Arc::new(build_file_index(&root_path, &exclude_dirs)); + // Invalidate every exclusion-policy variant for this root. A build + // that started before this force request cannot repopulate the cache. + FILE_INDEX_CACHE.invalidate_root(&root_path); + let entries = get_file_index(&root_path, &exclude_dirs)?; let count = entries.len(); - insert_file_index_cache_entry( - root_path, - Arc::clone(&entries), - std::time::SystemTime::now(), - ); - let duration = start.elapsed(); info!(entries = count, ?duration, "search::file: indexed entries"); @@ -502,50 +433,12 @@ pub async fn prewarm_file_index(root_path: String) -> Result { )); } - // Check if already cached and fresh — skip the walk entirely. - { - let mut cache = FILE_INDEX_CACHE.lock().unwrap(); - prune_file_index_cache(&mut cache); - if let Some(index) = cache.get(&root_path) { - if let Ok(elapsed) = index.indexed_at.elapsed() { - if elapsed.as_secs() < CACHE_TTL_SECS { - debug!( - entries = index.entries.len(), - age_secs = elapsed.as_secs_f64(), - "search::file: prewarm skipped; cache still fresh" - ); - return Ok(index.entries.len()); - } - } - } - } - debug!(root_path = %root_path, "search::file: prewarming index"); - let default_excludes = vec![ - "node_modules".to_string(), - ".git".to_string(), - "dist".to_string(), - "build".to_string(), - ".next".to_string(), - "target".to_string(), - ".cache".to_string(), - "coverage".to_string(), - "__pycache__".to_string(), - ".venv".to_string(), - "venv".to_string(), - ]; - - // Build WITHOUT holding the lock. - let entries = Arc::new(build_file_index(&root_path, &default_excludes)); + let default_excludes = default_excluded_dirs(); + let entries = get_file_index(&root_path, &default_excludes)?; let count = entries.len(); - insert_file_index_cache_entry( - root_path, - Arc::clone(&entries), - std::time::SystemTime::now(), - ); - info!(entries = count, "search::file: prewarm complete"); Ok(count) }) @@ -556,11 +449,20 @@ pub async fn prewarm_file_index(root_path: String) -> Result { /// Clear the file index cache #[tauri::command] pub fn clear_file_index_cache() { - let mut cache = FILE_INDEX_CACHE.lock().unwrap(); - cache.clear(); + FILE_INDEX_CACHE.clear(); info!("search::file: cache cleared"); } +/// Invalidate cached file indexes for one workspace root. +/// +/// This command performs no scan. The next foreground prewarm or search builds +/// a fresh index, and any older in-flight generation is discarded. +#[tauri::command] +pub fn invalidate_file_index_cache(root_path: String) { + FILE_INDEX_CACHE.invalidate_root(&root_path); + debug!(root_path = %root_path, "search::file: root cache invalidated"); +} + /// Find files by extension in a directory /// Returns list of file paths matching any of the given extensions #[tauri::command] @@ -584,21 +486,8 @@ pub async fn find_files_by_extension( } // Directories to skip entirely (the walker will NOT descend into them). - let exclude_set: std::collections::HashSet = [ - "node_modules", - ".git", - "dist", - "build", - ".next", - "target", - ".cache", - "__pycache__", - ".venv", - "venv", - ] - .iter() - .map(|s| s.to_string()) - .collect(); + let exclude_set: std::collections::HashSet = + default_excluded_dirs().into_iter().collect(); let mut builder = WalkBuilder::new(&directory); diff --git a/src-tauri/crates/search/src/file/index_cache.rs b/src-tauri/crates/search/src/file/index_cache.rs new file mode 100644 index 000000000..47bfd787c --- /dev/null +++ b/src-tauri/crates/search/src/file/index_cache.rs @@ -0,0 +1,392 @@ +use super::FileEntry; +use std::collections::HashMap; +use std::panic::{catch_unwind, AssertUnwindSafe}; +use std::sync::{Arc, Condvar, Mutex}; +use std::time::{Duration, Instant}; + +#[derive(Debug, Clone, Hash, PartialEq, Eq)] +struct FileIndexKey { + root_path: String, + exclude_dirs: Vec, +} + +impl FileIndexKey { + fn new(root_path: &str, exclude_dirs: &[String]) -> Self { + let mut exclude_dirs = exclude_dirs.to_vec(); + exclude_dirs.sort_unstable(); + exclude_dirs.dedup(); + Self { + root_path: root_path.to_string(), + exclude_dirs, + } + } +} + +struct CachedIndex { + entries: Arc<[FileEntry]>, + indexed_at: Instant, + last_accessed_at: Instant, +} + +#[derive(Default)] +struct BuildFlight { + result: Mutex>>, + completed: Condvar, +} + +impl BuildFlight { + fn wait(&self) -> Result<(), String> { + let mut result = self.result.lock().unwrap(); + while result.is_none() { + result = self.completed.wait(result).unwrap(); + } + result.clone().unwrap() + } + + fn finish(&self, result: Result<(), String>) { + *self.result.lock().unwrap() = Some(result); + self.completed.notify_all(); + } +} + +struct CacheSlot { + generation: u64, + cached: Option, + in_flight: Option>, + last_accessed_at: Instant, +} + +impl CacheSlot { + fn new() -> Self { + Self { + generation: 0, + cached: None, + in_flight: None, + last_accessed_at: Instant::now(), + } + } +} + +#[derive(Default)] +struct CacheState { + slots: HashMap, +} + +enum CacheAction { + Return(Arc<[FileEntry]>), + Wait(Arc), + Build { + flight: Arc, + generation: u64, + }, +} + +/// Coordinates file-path indexes for every open workspace. +/// +/// A slot is keyed by both workspace root and exclusion policy. Equivalent +/// callers share one build. Invalidating a root bumps its generation so a +/// build that started before a file change can never repopulate the cache. +pub(super) struct FilePathIndexCache { + state: Mutex, + safety_ttl: Duration, + max_cached_indexes: usize, +} + +impl FilePathIndexCache { + pub(super) fn new(safety_ttl: Duration, max_cached_indexes: usize) -> Self { + Self { + state: Mutex::new(CacheState::default()), + safety_ttl, + max_cached_indexes, + } + } + + pub(super) fn get_or_build( + &self, + root_path: &str, + exclude_dirs: &[String], + build: F, + ) -> Result, String> + where + F: Fn() -> Vec, + { + let key = FileIndexKey::new(root_path, exclude_dirs); + + loop { + let action = { + let mut state = self.state.lock().unwrap(); + self.prune_locked(&mut state); + + let slot = state + .slots + .entry(key.clone()) + .or_insert_with(CacheSlot::new); + slot.last_accessed_at = Instant::now(); + + if let Some(cached) = slot.cached.as_mut() { + if cached.indexed_at.elapsed() < self.safety_ttl { + cached.last_accessed_at = Instant::now(); + CacheAction::Return(Arc::clone(&cached.entries)) + } else if let Some(flight) = slot.in_flight.as_ref() { + CacheAction::Wait(Arc::clone(flight)) + } else { + slot.cached = None; + let flight = Arc::new(BuildFlight::default()); + slot.in_flight = Some(Arc::clone(&flight)); + CacheAction::Build { + flight, + generation: slot.generation, + } + } + } else if let Some(flight) = slot.in_flight.as_ref() { + CacheAction::Wait(Arc::clone(flight)) + } else { + let flight = Arc::new(BuildFlight::default()); + slot.in_flight = Some(Arc::clone(&flight)); + CacheAction::Build { + flight, + generation: slot.generation, + } + } + }; + + match action { + CacheAction::Return(entries) => return Ok(entries), + CacheAction::Wait(flight) => { + flight.wait()?; + } + CacheAction::Build { flight, generation } => { + let build_result = catch_unwind(AssertUnwindSafe(&build)); + let entries = match build_result { + Ok(entries) => Arc::<[FileEntry]>::from(entries), + Err(_) => { + let error = format!("File index build panicked for {root_path}"); + self.finish_failed_build(&key, &flight, error.clone()); + return Err(error); + } + }; + + let accepted = { + let mut state = self.state.lock().unwrap(); + let Some(slot) = state.slots.get_mut(&key) else { + flight.finish(Ok(())); + continue; + }; + + let owns_flight = slot + .in_flight + .as_ref() + .is_some_and(|current| Arc::ptr_eq(current, &flight)); + if owns_flight { + slot.in_flight = None; + } + + if owns_flight && slot.generation == generation { + let now = Instant::now(); + slot.cached = Some(CachedIndex { + entries: Arc::clone(&entries), + indexed_at: now, + last_accessed_at: now, + }); + slot.last_accessed_at = now; + self.prune_locked(&mut state); + true + } else { + false + } + }; + + flight.finish(Ok(())); + if accepted { + return Ok(entries); + } + // A file change or explicit clear superseded this build. + // Loop so the caller receives a generation-current index. + } + } + } + } + + pub(super) fn invalidate_root(&self, root_path: &str) { + let mut state = self.state.lock().unwrap(); + for (key, slot) in &mut state.slots { + if key.root_path == root_path { + slot.generation = slot.generation.wrapping_add(1); + slot.cached = None; + slot.last_accessed_at = Instant::now(); + } + } + state + .slots + .retain(|_, slot| slot.cached.is_some() || slot.in_flight.is_some()); + } + + pub(super) fn clear(&self) { + let mut state = self.state.lock().unwrap(); + for slot in state.slots.values_mut() { + slot.generation = slot.generation.wrapping_add(1); + slot.cached = None; + } + state.slots.retain(|_, slot| slot.in_flight.is_some()); + } + + fn finish_failed_build(&self, key: &FileIndexKey, flight: &Arc, error: String) { + let mut state = self.state.lock().unwrap(); + if let Some(slot) = state.slots.get_mut(key) { + if slot + .in_flight + .as_ref() + .is_some_and(|current| Arc::ptr_eq(current, flight)) + { + slot.in_flight = None; + } + } + state + .slots + .retain(|_, slot| slot.cached.is_some() || slot.in_flight.is_some()); + drop(state); + flight.finish(Err(error)); + } + + fn prune_locked(&self, state: &mut CacheState) { + state.slots.retain(|_, slot| { + slot.in_flight.is_some() + || slot + .cached + .as_ref() + .is_some_and(|cached| cached.indexed_at.elapsed() < self.safety_ttl) + }); + + while state + .slots + .values() + .filter(|slot| slot.cached.is_some()) + .count() + > self.max_cached_indexes + { + let Some(oldest_key) = state + .slots + .iter() + .filter(|(_, slot)| slot.in_flight.is_none() && slot.cached.is_some()) + .min_by_key(|(_, slot)| slot.last_accessed_at) + .map(|(key, _)| key.clone()) + else { + break; + }; + state.slots.remove(&oldest_key); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::atomic::{AtomicUsize, Ordering}; + use std::sync::Barrier; + use std::thread; + + fn entry(name: &str) -> FileEntry { + FileEntry { + path: format!("/repo/{name}"), + filename: name.to_string(), + is_dir: false, + } + } + + #[test] + fn equivalent_concurrent_requests_share_one_build() { + let cache = Arc::new(FilePathIndexCache::new(Duration::from_secs(60), 4)); + let build_count = Arc::new(AtomicUsize::new(0)); + let start = Arc::new(Barrier::new(8)); + + let threads: Vec<_> = (0..8) + .map(|_| { + let cache = Arc::clone(&cache); + let build_count = Arc::clone(&build_count); + let start = Arc::clone(&start); + thread::spawn(move || { + start.wait(); + cache + .get_or_build("/repo", &["target".to_string()], || { + build_count.fetch_add(1, Ordering::SeqCst); + thread::sleep(Duration::from_millis(40)); + vec![entry("main.rs")] + }) + .unwrap() + }) + }) + .collect(); + + for handle in threads { + assert_eq!(handle.join().unwrap().len(), 1); + } + assert_eq!(build_count.load(Ordering::SeqCst), 1); + } + + #[test] + fn invalidation_discards_an_in_flight_generation() { + let cache = Arc::new(FilePathIndexCache::new(Duration::from_secs(60), 4)); + let build_count = Arc::new(AtomicUsize::new(0)); + let first_started = Arc::new(Barrier::new(2)); + let resume_first = Arc::new(Barrier::new(2)); + + let worker = { + let cache = Arc::clone(&cache); + let build_count = Arc::clone(&build_count); + let first_started = Arc::clone(&first_started); + let resume_first = Arc::clone(&resume_first); + thread::spawn(move || { + cache + .get_or_build("/repo", &[], || { + let build_number = build_count.fetch_add(1, Ordering::SeqCst); + if build_number == 0 { + first_started.wait(); + resume_first.wait(); + } + vec![entry("main.rs")] + }) + .unwrap() + }) + }; + + first_started.wait(); + cache.invalidate_root("/repo"); + resume_first.wait(); + + assert_eq!(worker.join().unwrap().len(), 1); + assert_eq!(build_count.load(Ordering::SeqCst), 2); + } + + #[test] + fn exclusion_policy_is_part_of_the_cache_key() { + let cache = FilePathIndexCache::new(Duration::from_secs(60), 4); + let build_count = AtomicUsize::new(0); + + cache + .get_or_build("/repo", &["target".to_string()], || { + build_count.fetch_add(1, Ordering::SeqCst); + vec![entry("first")] + }) + .unwrap(); + cache + .get_or_build("/repo", &["node_modules".to_string()], || { + build_count.fetch_add(1, Ordering::SeqCst); + vec![entry("second")] + }) + .unwrap(); + + assert_eq!(build_count.load(Ordering::SeqCst), 2); + } + + #[test] + fn failed_owner_releases_waiters_and_allows_recovery() { + let cache = FilePathIndexCache::new(Duration::from_secs(60), 4); + let failed = cache.get_or_build("/repo", &[], || panic!("boom")); + assert!(failed.is_err()); + + let recovered = cache + .get_or_build("/repo", &[], || vec![entry("recovered")]) + .unwrap(); + assert_eq!(recovered[0].filename, "recovered"); + } +} diff --git a/src-tauri/crates/search/src/tests/file_tests.rs b/src-tauri/crates/search/src/tests/file_tests.rs index 991865e19..4e43596f7 100644 --- a/src-tauri/crates/search/src/tests/file_tests.rs +++ b/src-tauri/crates/search/src/tests/file_tests.rs @@ -1,47 +1,6 @@ -use std::collections::HashMap; -use std::sync::Arc; +use app_utils::testing::temp_dir_with_files; -use crate::file::{ - estimate_file_index_bytes, fuzzy_search, prune_file_index_cache, FileEntry, FileIndex, -}; - -#[test] -fn file_index_size_estimate_includes_paths_and_entry_storage() { - let entries = vec![FileEntry { - path: "C:/repo/src/main.rs".to_string(), - filename: "main.rs".to_string(), - is_dir: false, - }]; - - assert!( - estimate_file_index_bytes(&entries) - >= std::mem::size_of::() + entries[0].path.len() + entries[0].filename.len() - ); -} - -#[test] -fn file_index_cache_prunes_to_global_byte_budget() { - let now = std::time::SystemTime::now(); - let mut cache = HashMap::new(); - for index in 0..3 { - cache.insert( - format!("repo-{index}"), - FileIndex { - entries: Arc::new(Vec::new()), - _root_path: format!("repo-{index}"), - indexed_at: now - .checked_sub(std::time::Duration::from_secs(3 - index)) - .expect("test timestamp should be representable"), - estimated_bytes: 24 * 1024 * 1024, - }, - ); - } - - prune_file_index_cache(&mut cache); - - assert_eq!(cache.len(), 2); - assert!(!cache.contains_key("repo-0")); -} +use crate::file::{build_file_index, default_excluded_dirs, fuzzy_search, FileEntry}; #[test] fn test_fuzzy_matching() { @@ -70,3 +29,50 @@ fn test_fuzzy_matching() { // "btn" should match "Button" better than others assert_eq!(results[0].0.filename, "Button.tsx"); } + +#[test] +fn file_index_skips_runtime_worktrees_but_keeps_user_orgii_files() { + let (_dir, root) = temp_dir_with_files(&[ + ("src/main.rs", "fn main() {}"), + (".env", "SECRET=not-a-real-secret"), + (".orgii/skills/example/SKILL.md", "# Example"), + (".orgii/worktrees/session-a/generated.rs", "generated"), + (".worktrees/session-b/generated.rs", "generated"), + ]); + + let entries = build_file_index(root.to_str().unwrap(), &default_excluded_dirs()); + let paths: Vec<_> = entries + .iter() + .filter_map(|entry| { + std::path::Path::new(&entry.path) + .strip_prefix(&root) + .ok() + .map(|path| path.to_string_lossy().to_string()) + }) + .collect(); + + assert!(paths.contains(&"src/main.rs".to_string())); + assert!(paths.contains(&".env".to_string())); + assert!(paths.contains(&".orgii/skills/example/SKILL.md".to_string())); + assert!(!paths + .iter() + .any(|path| path.starts_with(".orgii/worktrees"))); + assert!(!paths.iter().any(|path| path.starts_with(".worktrees"))); +} + +#[test] +fn fuzzy_search_honors_zero_and_top_k_limits() { + let entries: Vec<_> = (0..100) + .map(|index| FileEntry { + path: format!("src/component-{index}.tsx"), + filename: format!("component-{index}.tsx"), + is_dir: false, + }) + .collect(); + + assert!(fuzzy_search(&entries, "component", 0, None).is_empty()); + + let results = fuzzy_search(&entries, "component", 5, None); + assert_eq!(results.len(), 5); + assert!(results.windows(2).all(|pair| pair[0].1 >= pair[1].1)); +} diff --git a/src-tauri/src/commands/handler_list.inc b/src-tauri/src/commands/handler_list.inc index bdc881402..49d3427d5 100644 --- a/src-tauri/src/commands/handler_list.inc +++ b/src-tauri/src/commands/handler_list.inc @@ -213,6 +213,7 @@ search::file::search_files_fuzzy, search::file::index_project_files, search::file::prewarm_file_index, search::file::clear_file_index_cache, +search::file::invalidate_file_index_cache, search::file::find_files_by_extension, // Code search commands search::code::commands::search_code_regex, diff --git a/src/api/http/project/cache.test.ts b/src/api/http/project/cache.test.ts index 78c38a444..f54402a08 100644 --- a/src/api/http/project/cache.test.ts +++ b/src/api/http/project/cache.test.ts @@ -27,6 +27,24 @@ describe("project read cache invalidation fencing", () => { ]); }); + it("can deduplicate only the active request without caching its result", async () => { + const fetcher = vi + .fn<() => Promise>() + .mockResolvedValueOnce("first") + .mockResolvedValueOnce("second"); + + const first = cachedRead("project:filtered", fetcher, { maxAgeMs: 0 }); + const joined = cachedRead("project:filtered", fetcher, { maxAgeMs: 0 }); + await expect(Promise.all([first, joined])).resolves.toEqual([ + "first", + "first", + ]); + await expect( + cachedRead("project:filtered", fetcher, { maxAgeMs: 0 }) + ).resolves.toBe("second"); + expect(fetcher).toHaveBeenCalledTimes(2); + }); + it("never lets a pre-invalidation Promise resurrect or return stale data", async () => { let resolveStale: ((value: string) => void) | undefined; const fetcher = vi diff --git a/src/api/http/project/cache.ts b/src/api/http/project/cache.ts index 6fa208980..29c9421fd 100644 --- a/src/api/http/project/cache.ts +++ b/src/api/http/project/cache.ts @@ -42,11 +42,13 @@ function evictIfNeeded(): void { export async function cachedRead( cacheKey: string, - fetcher: () => Promise + fetcher: () => Promise, + options?: { maxAgeMs?: number } ): Promise { + const maxAgeMs = options?.maxAgeMs ?? CACHE_TTL_MS; const now = Date.now(); const existing = cache.get(cacheKey); - if (existing && now - existing.timestamp < CACHE_TTL_MS) { + if (maxAgeMs > 0 && existing && now - existing.timestamp < maxAgeMs) { return existing.data as T; } @@ -63,10 +65,12 @@ export async function cachedRead( // stale snapshot; converge the original waiter onto the post-change // read (or its already-running shared Promise) instead. if (inflight.get(cacheKey) === promise) inflight.delete(cacheKey); - return cachedRead(cacheKey, fetcher); + return cachedRead(cacheKey, fetcher, options); + } + if (maxAgeMs > 0) { + evictIfNeeded(); + cache.set(cacheKey, { data: result, timestamp: Date.now() }); } - evictIfNeeded(); - cache.set(cacheKey, { data: result, timestamp: Date.now() }); if (inflight.get(cacheKey) === promise) inflight.delete(cacheKey); return result; }) diff --git a/src/api/http/project/client.purge.test.ts b/src/api/http/project/client.purge.test.ts new file mode 100644 index 000000000..eee106467 --- /dev/null +++ b/src/api/http/project/client.purge.test.ts @@ -0,0 +1,42 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { __TESTS_ONLY, purgeExpiredDeletedWorkItems } from "./client"; + +const { invokeMock } = vi.hoisted(() => ({ + invokeMock: vi.fn(), +})); + +vi.mock("@tauri-apps/api/core", () => ({ + invoke: invokeMock, +})); + +describe("expired work-item purge coordination", () => { + beforeEach(() => { + __TESTS_ONLY.resetPurgeCoordinator(); + invokeMock.mockReset(); + }); + + it("shares an active purge and throttles later filter refreshes", async () => { + invokeMock.mockResolvedValue(0); + + const first = purgeExpiredDeletedWorkItems("project-a"); + const joined = purgeExpiredDeletedWorkItems("project-a"); + await expect(Promise.all([first, joined])).resolves.toEqual([0, 0]); + await expect(purgeExpiredDeletedWorkItems("project-a")).resolves.toBe(0); + + expect(invokeMock).toHaveBeenCalledTimes(1); + }); + + it("releases a failed purge so the next request can retry", async () => { + invokeMock + .mockRejectedValueOnce(new Error("database busy")) + .mockResolvedValueOnce(0); + + await expect(purgeExpiredDeletedWorkItems("project-a")).rejects.toThrow( + "database busy" + ); + await expect(purgeExpiredDeletedWorkItems("project-a")).resolves.toBe(0); + + expect(invokeMock).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/api/http/project/client.ts b/src/api/http/project/client.ts index edd09d374..078a1db62 100644 --- a/src/api/http/project/client.ts +++ b/src/api/http/project/client.ts @@ -37,6 +37,28 @@ import type { WorkItemsViewData, } from "./types"; +const PURGE_DELETED_ITEMS_MIN_INTERVAL_MS = 5 * 60 * 1_000; +const MAX_PURGE_PROJECTS = 50; + +interface PurgeState { + inFlight?: Promise; + lastRunAt?: number; +} + +const purgeStateByProject = new Map(); + +function getPurgeState(projectSlug: string): PurgeState { + const existing = purgeStateByProject.get(projectSlug); + if (existing) return existing; + if (purgeStateByProject.size >= MAX_PURGE_PROJECTS) { + const oldestKey = purgeStateByProject.keys().next().value; + if (oldestKey) purgeStateByProject.delete(oldestKey); + } + const state: PurgeState = {}; + purgeStateByProject.set(projectSlug, state); + return state; +} + // ============================================ // Init / discovery // ============================================ @@ -378,17 +400,26 @@ export async function readWorkItemsViewData( const { statusFilter, searchQuery } = options ?? {}; const scopePayload = scopeInvokePayload(options); const scopeSegment = scopeCacheSegment(options); + const normalizedSearchQuery = searchQuery?.trim() || undefined; const hasFilters = - (statusFilter && statusFilter !== "all") || - (searchQuery && searchQuery.trim()); + (statusFilter && statusFilter !== "all") || normalizedSearchQuery; if (hasFilters) { - return invoke("project_read_work_items_view_data", { - projectSlug, - ...scopePayload, - statusFilter: statusFilter ?? null, - searchQuery: searchQuery ?? null, - }); + const filterSegment = JSON.stringify([ + statusFilter ?? null, + normalizedSearchQuery ?? null, + ]); + return cachedRead( + `${projectSlug}:workitems-view:${scopeSegment}:${filterSegment}`, + () => + invoke("project_read_work_items_view_data", { + projectSlug, + ...scopePayload, + statusFilter: statusFilter ?? null, + searchQuery: normalizedSearchQuery ?? null, + }), + { maxAgeMs: 0 } + ); } return cachedRead(`${projectSlug}:workitems-view:${scopeSegment}`, () => @@ -501,13 +532,35 @@ export async function restoreWorkItem( export async function purgeExpiredDeletedWorkItems( projectSlug: string ): Promise { - const result = await invoke( - "project_purge_expired_deleted_work_items", - { projectSlug } - ); - invalidateCache(projectSlug); - return result; -} + const state = getPurgeState(projectSlug); + if (state.inFlight) return state.inFlight; + if ( + state.lastRunAt !== undefined && + Date.now() - state.lastRunAt < PURGE_DELETED_ITEMS_MIN_INTERVAL_MS + ) { + return 0; + } + + const request = invoke("project_purge_expired_deleted_work_items", { + projectSlug, + }).then((result) => { + state.lastRunAt = Date.now(); + if (result > 0) invalidateCache(projectSlug); + return result; + }); + state.inFlight = request; + const release = () => { + if (state.inFlight === request) state.inFlight = undefined; + }; + void request.then(release, release); + return request; +} + +export const __TESTS_ONLY = { + resetPurgeCoordinator(): void { + purgeStateByProject.clear(); + }, +}; /** * Atomic partial update; the Rust handler holds an `IMMEDIATE` diff --git a/src/api/tauri/repo/__tests__/repoListCoordinator.test.ts b/src/api/tauri/repo/__tests__/repoListCoordinator.test.ts new file mode 100644 index 000000000..44aafec59 --- /dev/null +++ b/src/api/tauri/repo/__tests__/repoListCoordinator.test.ts @@ -0,0 +1,99 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { __TESTS_ONLY, deleteRepo, getRepos } from "@src/api/tauri/repo"; + +const { invokeMock } = vi.hoisted(() => ({ + invokeMock: vi.fn(), +})); + +vi.mock("@tauri-apps/api/core", () => ({ + invoke: invokeMock, +})); + +function backendRepo(id: string) { + return { + id, + repo_id: id, + name: id, + path: `/repos/${id}`, + }; +} + +describe("repository list coordinator", () => { + beforeEach(() => { + __TESTS_ONLY.resetRepoListCoordinator(); + invokeMock.mockReset(); + }); + + it("shares one list request between concurrent consumers", async () => { + invokeMock.mockResolvedValue([backendRepo("one")]); + + const [first, second] = await Promise.all([getRepos(), getRepos()]); + + expect(invokeMock).toHaveBeenCalledTimes(1); + expect(first).toEqual(second); + }); + + it("runs one trailing request when force refresh arrives in flight", async () => { + let releaseFirst!: (repos: ReturnType[]) => void; + invokeMock + .mockImplementationOnce( + () => + new Promise((resolve) => { + releaseFirst = resolve; + }) + ) + .mockResolvedValueOnce([backendRepo("fresh")]); + + const initial = getRepos(); + const forced = getRepos({ forceRefresh: true }); + releaseFirst([backendRepo("stale")]); + + const [initialResult, forcedResult] = await Promise.all([initial, forced]); + + expect(invokeMock).toHaveBeenCalledTimes(2); + expect(initialResult.data.repos[0]?.repo_id).toBe("fresh"); + expect(forcedResult.data.repos[0]?.repo_id).toBe("fresh"); + }); + + it("refreshes an active list after a repository mutation", async () => { + let releaseFirst!: (repos: ReturnType[]) => void; + invokeMock.mockImplementation((command: string) => { + if (command === "server_delete_repo") return Promise.resolve(true); + if ( + invokeMock.mock.calls.filter(([name]) => name === "server_list_repos") + .length === 1 + ) { + return new Promise((resolve) => { + releaseFirst = resolve; + }); + } + return Promise.resolve([backendRepo("remaining")]); + }); + + const listing = getRepos(); + await deleteRepo("removed"); + releaseFirst([backendRepo("removed"), backendRepo("remaining")]); + const result = await listing; + + expect( + invokeMock.mock.calls.filter(([name]) => name === "server_list_repos") + ).toHaveLength(2); + expect(result.data.repos.map((repo) => repo.repo_id)).toEqual([ + "remaining", + ]); + }); + + it("releases a failed list request so a later load can retry", async () => { + invokeMock + .mockRejectedValueOnce(new Error("backend unavailable")) + .mockResolvedValueOnce([backendRepo("recovered")]); + + await expect(getRepos()).rejects.toThrow("backend unavailable"); + await expect(getRepos()).resolves.toMatchObject({ + data: { repos: [{ repo_id: "recovered" }] }, + }); + + expect(invokeMock).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/api/tauri/repo/index.ts b/src/api/tauri/repo/index.ts index 07888b765..17690feed 100644 --- a/src/api/tauri/repo/index.ts +++ b/src/api/tauri/repo/index.ts @@ -36,12 +36,31 @@ function wrapResponse(data: T) { return { data, status: 0 }; } +interface RepoListResponse { + data: RepoList; + status: number; +} + +interface RepoListFlight { + forceRefresh: boolean; + generation: number; + promise: Promise; +} + +let repoListGeneration = 0; +let repoListFlight: RepoListFlight | undefined; +let repoListForcePending = false; + +function markRepoListChanged(forceRefresh = false): void { + repoListGeneration += 1; + repoListForcePending ||= forceRefresh; +} + // ============================================ // Repository CRUD (via Tauri commands) // ============================================ -/** Get current user's repository list */ -export async function getRepos() { +async function fetchRepos(): Promise { const repos = await invokeTauri< Array<{ id: string; @@ -66,6 +85,72 @@ export async function getRepos() { return wrapResponse({ repos: mapped }); } +/** + * Get the current repository list with one shared IPC request. + * + * A force request arriving behind a normal load, or a mutation completing + * during a load, advances the generation. The old response is awaited but not + * returned; all callers then share one trailing request. + */ +export async function getRepos(options?: { + forceRefresh?: boolean; +}): Promise { + const forceRefresh = options?.forceRefresh ?? false; + const current = repoListFlight; + + if ( + forceRefresh && + current && + !current.forceRefresh && + current.generation === repoListGeneration + ) { + markRepoListChanged(true); + } + + if (current) { + try { + const response = await current.promise; + if ( + current.generation === repoListGeneration && + (!forceRefresh || current.forceRefresh) + ) { + return response; + } + } catch (error) { + if (current.generation === repoListGeneration) throw error; + } + return getRepos({ forceRefresh }); + } + + const effectiveForceRefresh = forceRefresh || repoListForcePending; + repoListForcePending = false; + const generation = repoListGeneration; + const promise = fetchRepos(); + const flight: RepoListFlight = { + forceRefresh: effectiveForceRefresh, + generation, + promise, + }; + repoListFlight = flight; + const release = () => { + if (repoListFlight === flight) repoListFlight = undefined; + }; + void promise.then(release, release); + + try { + const response = await promise; + if (generation !== repoListGeneration) { + return getRepos(); + } + return response; + } catch (error) { + if (generation !== repoListGeneration) { + return getRepos(); + } + throw error; + } +} + /** Get repository by ID (path) */ export async function getRepoById(repoId: string) { const result = await invokeTauri<{ @@ -88,6 +173,7 @@ export async function getRepoById(repoId: string) { /** Delete / unwatch repository */ export async function deleteRepo(repoId: string) { await invokeTauri("server_delete_repo", { repoId }); + markRepoListChanged(); return wrapResponse(null); } @@ -97,6 +183,7 @@ export async function updateRepoVisibility( visibility: "public" | "private" ) { await invokeTauri("server_update_repo_visibility", { path, visibility }); + markRepoListChanged(); } /** Check GitHub repo visibility via backend (no CORS issues). Returns "public", "private", or null. */ @@ -124,6 +211,7 @@ export async function importLocalRepo(data: { fs_path: string }) { path: string; kind?: string; }>("server_import_repo", { path: data.fs_path }); + markRepoListChanged(); const repo: Repo = { repo_id: result.repo_id || result.id, user_id: "", @@ -149,6 +237,7 @@ export async function createFromGithub(data: { url: data.github_url, targetDir: data.fs_path, }); + markRepoListChanged(); const repo: Repo = { repo_id: result.repo_id || result.id, user_id: "", @@ -176,6 +265,7 @@ export async function createEmptyRepo(data: { path: data.fs_path, name: data.name, }); + markRepoListChanged(); const repo: Repo = { repo_id: result.repo_id || result.id, user_id: "", @@ -195,6 +285,7 @@ export async function importWorkFolder(data: { fs_path: string }) { path: string; kind: string; }>("server_import_folder", { path: data.fs_path }); + markRepoListChanged(); const repo: Repo = { repo_id: result.repo_id || result.id, user_id: "", @@ -220,6 +311,7 @@ export async function createWorkFolder(data: { path: data.fs_path, name: data.name, }); + markRepoListChanged(); const repo: Repo = { repo_id: result.repo_id || result.id, user_id: "", @@ -293,4 +385,12 @@ export const repoApi = { detectIDEs, }; +export const __TESTS_ONLY = { + resetRepoListCoordinator() { + repoListGeneration = 0; + repoListFlight = undefined; + repoListForcePending = false; + }, +}; + export default repoApi; diff --git a/src/contexts/git/GitStatusContext/GitStatusProvider.tsx b/src/contexts/git/GitStatusContext/GitStatusProvider.tsx index 091552808..e063a0304 100644 --- a/src/contexts/git/GitStatusContext/GitStatusProvider.tsx +++ b/src/contexts/git/GitStatusContext/GitStatusProvider.tsx @@ -141,6 +141,19 @@ export const GitStatusProvider: React.FC<{ children: React.ReactNode }> = ({ return currentRepo?.path || currentRepo?.fs_uri; }, [currentRepo]); + const resolveRepoPath = useCallback( + (repoId: string): string | undefined => { + const repo = repoMap.get(repoId); + if (repo?.path || repo?.fs_uri) return repo.path || repo.fs_uri; + + const folder = workspaceFolders.find( + (candidate) => candidate.id === repoId || candidate.repoId === repoId + ); + return folder?.path; + }, + [repoMap, workspaceFolders] + ); + // ============================================ // Watcher Registration Hook // ============================================ @@ -233,6 +246,7 @@ export const GitStatusProvider: React.FC<{ children: React.ReactNode }> = ({ setGitStatusAtom, setGitSuggestedActionAtom, setGitOperation, + resolveRepoPath, }); // ============================================ diff --git a/src/contexts/git/GitStatusContext/hooks/__tests__/TEST_CASES.md b/src/contexts/git/GitStatusContext/hooks/__tests__/TEST_CASES.md new file mode 100644 index 000000000..28ead01a5 --- /dev/null +++ b/src/contexts/git/GitStatusContext/hooks/__tests__/TEST_CASES.md @@ -0,0 +1,34 @@ +# File index invalidation test cases + +## Preconditions + +- Git watcher events identify a repository with `repo_id`. +- File-path index invalidation is a cheap state transition; it does not scan. +- Content-only modifications do not change the indexed path set. + +## Happy path + +- Created, deleted, renamed, or unknown file events schedule invalidation. +- Aggregate `repo:changed` events invalidate only when `change_type` is `files`. +- Multiple events for one repository inside 250 ms produce one invalidation. +- Simultaneous events for different repositories invalidate each root once. + +## Edge cases + +- `modified` events are ignored because filenames and paths are unchanged. +- Disposing the listener clears pending invalidations. +- Empty paths are not scheduled. + +## Error path + +- A rejected Tauri invalidation request is reported through the supplied error callback and does not create an unhandled rejection. + +## Accessibility + +- Not applicable: this lifecycle change has no rendered UI or input behavior. + +## Acceptance criteria + +- No timer causes indexing or repeated background scans. +- A watcher burst produces at most one cheap invalidation call per root. +- Listener teardown leaves no pending timer. diff --git a/src/contexts/git/GitStatusContext/hooks/__tests__/fileIndexInvalidation.test.ts b/src/contexts/git/GitStatusContext/hooks/__tests__/fileIndexInvalidation.test.ts new file mode 100644 index 000000000..b1c28a4c6 --- /dev/null +++ b/src/contexts/git/GitStatusContext/hooks/__tests__/fileIndexInvalidation.test.ts @@ -0,0 +1,75 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { + createFileIndexInvalidationScheduler, + fileChangeInvalidatesPathIndex, + repoChangeInvalidatesPathIndex, +} from "../fileIndexInvalidation"; + +describe("file index invalidation", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("coalesces a burst into one invalidation per workspace root", async () => { + const invalidate = vi.fn().mockResolvedValue(undefined); + const scheduler = createFileIndexInvalidationScheduler(invalidate, 250); + + scheduler.schedule("/repo-a"); + scheduler.schedule("/repo-a"); + scheduler.schedule("/repo-b"); + vi.advanceTimersByTime(249); + expect(invalidate).not.toHaveBeenCalled(); + + vi.advanceTimersByTime(1); + await Promise.resolve(); + expect(invalidate).toHaveBeenCalledTimes(2); + expect(invalidate).toHaveBeenCalledWith("/repo-a"); + expect(invalidate).toHaveBeenCalledWith("/repo-b"); + }); + + it("drops pending work after disposal", () => { + const invalidate = vi.fn().mockResolvedValue(undefined); + const scheduler = createFileIndexInvalidationScheduler(invalidate, 250); + + scheduler.schedule("/repo-a"); + scheduler.dispose(); + vi.advanceTimersByTime(500); + + expect(invalidate).not.toHaveBeenCalled(); + }); + + it("reports asynchronous invalidation failures", async () => { + const error = new Error("IPC unavailable"); + const onError = vi.fn(); + const scheduler = createFileIndexInvalidationScheduler( + vi.fn().mockRejectedValue(error), + 1, + onError + ); + + scheduler.schedule("/repo-a"); + await vi.advanceTimersByTimeAsync(1); + + expect(onError).toHaveBeenCalledWith(error); + }); + + it("ignores content-only modifications", () => { + expect(fileChangeInvalidatesPathIndex("modified")).toBe(false); + expect(fileChangeInvalidatesPathIndex("created")).toBe(true); + expect(fileChangeInvalidatesPathIndex("deleted")).toBe(true); + expect(fileChangeInvalidatesPathIndex("renamed")).toBe(true); + expect(fileChangeInvalidatesPathIndex(undefined)).toBe(true); + }); + + it("only invalidates for aggregate filesystem changes", () => { + expect(repoChangeInvalidatesPathIndex("files")).toBe(true); + expect(repoChangeInvalidatesPathIndex("git_meta")).toBe(false); + expect(repoChangeInvalidatesPathIndex("branch")).toBe(false); + expect(repoChangeInvalidatesPathIndex(undefined)).toBe(false); + }); +}); diff --git a/src/contexts/git/GitStatusContext/hooks/fileIndexInvalidation.ts b/src/contexts/git/GitStatusContext/hooks/fileIndexInvalidation.ts new file mode 100644 index 000000000..b15584fd4 --- /dev/null +++ b/src/contexts/git/GitStatusContext/hooks/fileIndexInvalidation.ts @@ -0,0 +1,52 @@ +export interface FileIndexInvalidationScheduler { + schedule(rootPath: string): void; + dispose(): void; +} + +/** + * Coalesces file-create/delete/rename bursts into one invalidation per root. + * Invalidating is deliberately cheap: it marks state stale but never scans. + */ +export function createFileIndexInvalidationScheduler( + invalidate: (rootPath: string) => Promise, + delayMs = 250, + onError: (error: unknown) => void = () => undefined +): FileIndexInvalidationScheduler { + const pendingRoots = new Set(); + let timer: ReturnType | null = null; + let disposed = false; + + const flush = () => { + timer = null; + const roots = [...pendingRoots]; + pendingRoots.clear(); + + for (const rootPath of roots) { + void invalidate(rootPath).catch(onError); + } + }; + + return { + schedule(rootPath) { + if (disposed || !rootPath) return; + pendingRoots.add(rootPath); + if (timer) return; + timer = setTimeout(flush, delayMs); + }, + dispose() { + disposed = true; + pendingRoots.clear(); + if (timer) clearTimeout(timer); + timer = null; + }, + }; +} + +/** Content-only modifications do not change a file-path index. */ +export function fileChangeInvalidatesPathIndex(kind: unknown): boolean { + return typeof kind !== "string" || kind !== "modified"; +} + +export function repoChangeInvalidatesPathIndex(changeType: unknown): boolean { + return changeType === "files"; +} diff --git a/src/contexts/git/GitStatusContext/hooks/useGitEventListeners.ts b/src/contexts/git/GitStatusContext/hooks/useGitEventListeners.ts index 45677b146..3650411d2 100644 --- a/src/contexts/git/GitStatusContext/hooks/useGitEventListeners.ts +++ b/src/contexts/git/GitStatusContext/hooks/useGitEventListeners.ts @@ -18,8 +18,14 @@ import type { } from "@src/types/session/steps"; import { decodeOctalPath } from "@src/util/file/pathUtils"; import { computeSuggestedAction } from "@src/util/git/computeSuggestedAction"; +import { invalidateFileIndexCache } from "@src/util/platform/tauri/fileSearch"; import type { GitStatusRefs } from "../types"; +import { + createFileIndexInvalidationScheduler, + fileChangeInvalidatesPathIndex, + repoChangeInvalidatesPathIndex, +} from "./fileIndexInvalidation"; const log = createLogger("GitStatusContext"); @@ -38,6 +44,7 @@ interface UseGitEventListenersOptions { details: string; timestamp: number; }) => void; + resolveRepoPath: (repoId: string) => string | undefined; } export function useGitEventListeners({ @@ -48,6 +55,7 @@ export function useGitEventListeners({ setGitStatusAtom, setGitSuggestedActionAtom, setGitOperation, + resolveRepoPath, }: UseGitEventListenersOptions): void { const { currentRepoIdRef, gitStatusRef } = refs; @@ -60,6 +68,7 @@ export function useGitEventListeners({ const setGitStatusAtomRef = useRef(setGitStatusAtom); const setGitSuggestedActionAtomRef = useRef(setGitSuggestedActionAtom); const setGitOperationRef = useRef(setGitOperation); + const resolveRepoPathRef = useRef(resolveRepoPath); useEffect(() => { setGitStatusRef.current = setGitStatus; }, [setGitStatus]); @@ -75,11 +84,20 @@ export function useGitEventListeners({ useEffect(() => { setGitOperationRef.current = setGitOperation; }, [setGitOperation]); + useEffect(() => { + resolveRepoPathRef.current = resolveRepoPath; + }, [resolveRepoPath]); useEffect(() => { if (!selectedRepoId) return; const cleanupFns: (() => void)[] = []; + const fileIndexInvalidation = createFileIndexInvalidationScheduler( + invalidateFileIndexCache, + 250, + (error) => + log.warn("[GitStatusContext] File index invalidation failed:", error) + ); const setupListeners = () => { try { @@ -209,9 +227,44 @@ export function useGitEventListeners({ }); cleanupFns.push(unsubscribeStatus); - // Listen to file:changed for file changes - const unsubscribeChanged = ws.on("file:changed", (_data) => { - // Status will arrive via repo:status_updated event from debouncer + // The repository watcher emits this aggregate event for every real + // filesystem burst. It is the canonical path-index invalidation source. + const unsubscribeRepoChanged = ws.on("repo:changed", (data) => { + const event = data as { + repo_id?: string; + change_type?: string; + }; + if ( + !event.repo_id || + !repoChangeInvalidatesPathIndex(event.change_type) + ) { + return; + } + + const rootPath = resolveRepoPathRef.current(event.repo_id); + if (rootPath) fileIndexInvalidation.schedule(rootPath); + }); + cleanupFns.push(unsubscribeRepoChanged); + + // Retain compatibility with granular file events from future or + // alternate watcher producers. + const unsubscribeChanged = ws.on("file:changed", (data) => { + const event = data as { + repo_id?: string; + kind?: string; + files?: Array<{ kind?: string }>; + }; + if (!event.repo_id) return; + + const containsPathChange = event.files + ? event.files.some((file) => + fileChangeInvalidatesPathIndex(file.kind) + ) + : fileChangeInvalidatesPathIndex(event.kind); + if (!containsPathChange) return; + + const rootPath = resolveRepoPathRef.current(event.repo_id); + if (rootPath) fileIndexInvalidation.schedule(rootPath); }); cleanupFns.push(unsubscribeChanged); @@ -247,6 +300,7 @@ export function useGitEventListeners({ setupListeners(); return () => { + fileIndexInvalidation.dispose(); cleanupFns.forEach((fn) => fn()); }; }, [ diff --git a/src/engines/BrowserCore/BrowserCore.webviewRetention.test.ts b/src/engines/BrowserCore/BrowserCore.webviewRetention.test.ts new file mode 100644 index 000000000..d828c189c --- /dev/null +++ b/src/engines/BrowserCore/BrowserCore.webviewRetention.test.ts @@ -0,0 +1,170 @@ +// @vitest-environment jsdom +import { act, createElement } from "react"; +import { type Root, createRoot } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import type { UseBrowserStateReturn } from "./hooks/useBrowserState"; +import { + BrowserCore, + MAX_RETAINED_BROWSER_WEBVIEWS, + selectRetainedBrowserSessionIds, +} from "./index"; +import type { BrowserSession } from "./types"; + +vi.mock("jotai", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + useAtomValue: () => false, + }; +}); + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); + +vi.mock("./BrowserSessionWebview", () => ({ + default: ({ + session, + isActive, + }: { + session: BrowserSession; + isActive: boolean; + }) => + createElement("div", { + "data-browser-webview-session": session.id, + "data-active": String(isActive), + }), +})); + +const reactActEnvironment = globalThis as typeof globalThis & { + IS_REACT_ACT_ENVIRONMENT?: boolean; +}; + +function session( + id: string, + url = `https://${id}.example.com` +): BrowserSession { + return { + id, + title: id, + url, + history: url ? [url] : [], + historyIndex: url ? 0 : -1, + historyEntries: [], + isLoading: false, + error: null, + incognito: false, + }; +} + +function browserState( + sessions: BrowserSession[], + activeSessionId: string +): UseBrowserStateReturn { + return { + sessions, + activeSessionId, + activeSession: sessions.find((item) => item.id === activeSessionId), + addSession: vi.fn(), + closeSession: vi.fn(), + setActiveSession: vi.fn(), + updateSession: vi.fn(), + }; +} + +describe("BrowserCore retained native WebViews", () => { + let container: HTMLDivElement; + let root: Root; + + beforeEach(() => { + reactActEnvironment.IS_REACT_ACT_ENVIRONMENT = true; + (window as unknown as Record).__TAURI_INTERNALS__ = {}; + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + }); + + afterEach(() => { + act(() => root.unmount()); + container.remove(); + Reflect.deleteProperty( + window as unknown as Record, + "__TAURI_INTERNALS__" + ); + Reflect.deleteProperty(reactActEnvironment, "IS_REACT_ACT_ENVIRONMENT"); + vi.restoreAllMocks(); + }); + + function renderWith( + sessions: BrowserSession[], + activeSessionId: string + ): void { + act(() => { + root.render( + createElement(BrowserCore, { + browserState: browserState(sessions, activeSessionId), + }) + ); + }); + } + + function mountedSessionIds(): string[] { + return Array.from( + container.querySelectorAll("[data-browser-webview-session]") + ).map((element) => element.getAttribute("data-browser-webview-session")!); + } + + it("keeps only the active and most recently active native WebViews mounted", () => { + const sessions = [session("a"), session("b"), session("c")]; + + renderWith(sessions, "a"); + expect(mountedSessionIds()).toEqual(["a"]); + + renderWith(sessions, "b"); + expect(mountedSessionIds()).toEqual(["a", "b"]); + + renderWith(sessions, "c"); + expect(mountedSessionIds()).toEqual(["b", "c"]); + + renderWith(sessions, "a"); + expect(mountedSessionIds()).toEqual(["a", "c"]); + expect(mountedSessionIds()).toHaveLength(MAX_RETAINED_BROWSER_WEBVIEWS); + }); + + it("does not mount restored background sessions or blank tabs eagerly", () => { + const sessions = [session("a"), session("blank", ""), session("c")]; + + renderWith(sessions, "blank"); + expect(mountedSessionIds()).toEqual([]); + + renderWith(sessions, "c"); + expect(mountedSessionIds()).toEqual(["c"]); + }); + + it("keeps the native mount count bounded across repeated session switches", () => { + const sessions = [session("a"), session("b"), session("c"), session("d")]; + + for (let index = 0; index < 50; index += 1) { + const activeSessionId = sessions[index % sessions.length].id; + renderWith(sessions, activeSessionId); + + expect(mountedSessionIds()).toContain(activeSessionId); + expect(mountedSessionIds().length).toBeLessThanOrEqual( + MAX_RETAINED_BROWSER_WEBVIEWS + ); + } + }); +}); + +describe("selectRetainedBrowserSessionIds", () => { + it("drops deleted and non-navigable sessions while preserving recency", () => { + expect( + selectRetainedBrowserSessionIds( + ["deleted", "a", "blank"], + [session("a"), session("blank", ""), session("b")], + "b" + ) + ).toEqual(["a", "b"]); + }); +}); diff --git a/src/engines/BrowserCore/index.tsx b/src/engines/BrowserCore/index.tsx index 4c1bc3b3f..985e6edcc 100644 --- a/src/engines/BrowserCore/index.tsx +++ b/src/engines/BrowserCore/index.tsx @@ -36,12 +36,14 @@ import BrowserSessionWebview from "./BrowserSessionWebview"; import type { UseBrowserStateReturn } from "./hooks/useBrowserState"; import "./index.scss"; import { BROWSER_WEBVIEW_FRAME_ANCHOR_ATTRIBUTE } from "./nativeFrameAnchor"; +import type { BrowserSession } from "./types"; const log = createLogger("BrowserCore"); const ABOUT_BLANK_URL = "about:blank"; const SHOW_WEBVIEW_FRAME_ANCHOR = false; const EMBEDDED_BROWSER_WARNING_DELAY_MS = 3000; +export const MAX_RETAINED_BROWSER_WEBVIEWS = 2; const EMBEDDED_BROWSER_SENSITIVE_HOSTS = new Set([ "github.com", "www.github.com", @@ -66,6 +68,37 @@ function shouldShowEmbeddedBrowserFallback(url?: string): boolean { } } +export function selectRetainedBrowserSessionIds( + previousIds: readonly string[], + sessions: readonly BrowserSession[], + activeSessionId: string, + maxRetained = MAX_RETAINED_BROWSER_WEBVIEWS +): string[] { + if (maxRetained <= 0) return []; + + const navigableIds = new Set( + sessions + .filter((session) => !isBlankBrowserUrl(session.url)) + .map((session) => session.id) + ); + const next = previousIds.filter( + (sessionId) => navigableIds.has(sessionId) && sessionId !== activeSessionId + ); + + if (navigableIds.has(activeSessionId)) { + next.push(activeSessionId); + } + + return next.slice(-maxRetained); +} + +function sameIds(left: readonly string[], right: readonly string[]): boolean { + return ( + left.length === right.length && + left.every((sessionId, index) => sessionId === right[index]) + ); +} + // ============================================ // Props // ============================================ @@ -112,6 +145,29 @@ export const BrowserCore: React.FC = ({ }) => { const { t } = useTranslation(); const { sessions, activeSessionId, updateSession, addSession } = browserState; + const [retainedWebviewSessionIds, setRetainedWebviewSessionIds] = + React.useState([]); + const nextRetainedWebviewSessionIds = useMemo( + () => + selectRetainedBrowserSessionIds( + retainedWebviewSessionIds, + sessions, + activeSessionId + ), + [activeSessionId, retainedWebviewSessionIds, sessions] + ); + const retainedWebviewSessionIdSet = useMemo( + () => new Set(nextRetainedWebviewSessionIds), + [nextRetainedWebviewSessionIds] + ); + + React.useLayoutEffect(() => { + setRetainedWebviewSessionIds((previousIds) => + sameIds(previousIds, nextRetainedWebviewSessionIds) + ? previousIds + : nextRetainedWebviewSessionIds + ); + }, [nextRetainedWebviewSessionIds]); // Check if webviews should be blocked by overlays or station ownership. const isWebviewBlocked = useAtomValue(webviewBlockedAtom); @@ -282,17 +338,19 @@ export const BrowserCore: React.FC = ({ {/* Only the owning instance renders BrowserSessionWebview. */} {manageWebviews && - sessions.map((session) => ( - - ))} + sessions + .filter((session) => retainedWebviewSessionIdSet.has(session.id)) + .map((session) => ( + + ))} {/* Desktop-only notice */} {!isWebviewAvailable && ( diff --git a/src/features/BenchmarkPanel/index.tsx b/src/features/BenchmarkPanel/index.tsx index 5df7b7a2e..bc5a2756f 100644 --- a/src/features/BenchmarkPanel/index.tsx +++ b/src/features/BenchmarkPanel/index.tsx @@ -21,6 +21,10 @@ import TabPill from "@src/components/TabPill"; import { SURFACE_TOKENS } from "@src/config/surfaceTokens"; import BenchmarkTaskSelector from "@src/features/BenchmarkPanel/BenchmarkTaskSelector"; import { CodeMirrorEditor } from "@src/features/CodeMirror"; +import { + listBenchmarkTasksShared, + setBenchmarkAgentBatchStatusShared, +} from "@src/hooks/benchmark/benchmarkRequestCoordinator"; import { useBenchmarkAgentBatchRun } from "@src/hooks/benchmark/useBenchmarkAgentBatchRun"; import { useBenchmarkTasks } from "@src/hooks/benchmark/useBenchmarkTasks"; import { usePublishWorkstationTabHeader } from "@src/hooks/workStation"; @@ -170,7 +174,7 @@ export const BenchmarkPanel: React.FC = ({ setAddTasksLoading(true); setAddTasksError(null); try { - const rows = await benchmarkApi.listTasks({ + const rows = await listBenchmarkTasksShared({ kind: status.benchmarkKind, sourcePath: status.sourcePath, limit: BENCHMARK_TASK_LIST_LIMIT, @@ -310,6 +314,7 @@ export const BenchmarkPanel: React.FC = ({ action, taskIds, }); + setBenchmarkAgentBatchStatusShared(status); setBenchmarkBatchStatus(status); void loadSessions({ forceRefresh: true }); return true; @@ -418,6 +423,7 @@ export const BenchmarkPanel: React.FC = ({ batchId: batchStatus.batchId, evaluationMode: BENCHMARK_EVALUATION_MODE.LOCAL_DOCKER, }); + setBenchmarkAgentBatchStatusShared(status); setBenchmarkBatchStatus(status); const evaluatedCount = status.items.filter( (item) => item.evaluationStatus diff --git a/src/hooks/benchmark/__tests__/TEST_CASES.md b/src/hooks/benchmark/__tests__/TEST_CASES.md new file mode 100644 index 000000000..ec027ba2d --- /dev/null +++ b/src/hooks/benchmark/__tests__/TEST_CASES.md @@ -0,0 +1,19 @@ +# Benchmark async coordination test cases + +## Task discovery + +- Multiple mounted benchmark consumers requesting the same kind, source path, + and limit share one backend request. +- A failed shared request releases its entry so a later retry can run. +- A changed kind or source path is a distinct scope and cannot reuse the old + result. + +## Status polling + +- Agent-batch and benchmark-run status requests share one in-flight request per + identifier. +- A completed status response is reused only inside the current two-second poll + period. +- Polling never overlaps a still-running request. +- Hidden pages keep no polling timer; becoming visible runs one catch-up pass. +- Cleanup during an active request prevents any subsequent timer. diff --git a/src/hooks/benchmark/__tests__/benchmarkRequestCoordinator.test.ts b/src/hooks/benchmark/__tests__/benchmarkRequestCoordinator.test.ts new file mode 100644 index 000000000..faeb872c8 --- /dev/null +++ b/src/hooks/benchmark/__tests__/benchmarkRequestCoordinator.test.ts @@ -0,0 +1,111 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { benchmarkApi } from "@src/api/tauri/benchmark"; + +import { + __TESTS_ONLY, + getBenchmarkAgentBatchStatusShared, + listBenchmarkTasksShared, + setBenchmarkAgentBatchStatusShared, +} from "../benchmarkRequestCoordinator"; + +vi.mock("@src/api/tauri/benchmark", async (importOriginal) => { + const original = + await importOriginal(); + return { + ...original, + benchmarkApi: { + ...original.benchmarkApi, + getAgentBatchStatus: vi.fn(), + listTasks: vi.fn(), + }, + }; +}); + +describe("benchmark request coordinator", () => { + beforeEach(() => { + __TESTS_ONLY.reset(); + vi.mocked(benchmarkApi.getAgentBatchStatus).mockReset(); + vi.mocked(benchmarkApi.listTasks).mockReset(); + }); + + it("shares task discovery across hook instances", async () => { + vi.mocked(benchmarkApi.listTasks).mockResolvedValue([]); + const request = { + kind: "swe_bench_pro" as const, + sourcePath: "/bench", + limit: 250, + }; + + await Promise.all([ + listBenchmarkTasksShared(request), + listBenchmarkTasksShared(request), + ]); + + expect(benchmarkApi.listTasks).toHaveBeenCalledTimes(1); + }); + + it("shares an active status request and reuses it within one poll period", async () => { + const status = { + batchId: "batch-1", + status: "running", + }; + vi.mocked(benchmarkApi.getAgentBatchStatus).mockResolvedValue( + status as Awaited> + ); + + await Promise.all([ + getBenchmarkAgentBatchStatusShared("batch-1"), + getBenchmarkAgentBatchStatusShared("batch-1"), + ]); + await getBenchmarkAgentBatchStatusShared("batch-1"); + + expect(benchmarkApi.getAgentBatchStatus).toHaveBeenCalledTimes(1); + }); + + it("releases a failed request for retry", async () => { + vi.mocked(benchmarkApi.getAgentBatchStatus) + .mockRejectedValueOnce(new Error("offline")) + .mockResolvedValueOnce({ + batchId: "batch-1", + status: "running", + } as Awaited>); + + await expect(getBenchmarkAgentBatchStatusShared("batch-1")).rejects.toThrow( + "offline" + ); + await expect( + getBenchmarkAgentBatchStatusShared("batch-1") + ).resolves.toBeTruthy(); + + expect(benchmarkApi.getAgentBatchStatus).toHaveBeenCalledTimes(2); + }); + + it("does not expose an older poll response after a mutation seeds status", async () => { + type AgentStatus = Awaited< + ReturnType + >; + let release!: (status: AgentStatus) => void; + vi.mocked(benchmarkApi.getAgentBatchStatus).mockImplementation( + () => + new Promise((resolve) => { + release = resolve; + }) + ); + const running = { batchId: "batch-1", status: "running" } as AgentStatus; + const cancelled = { + batchId: "batch-1", + status: "cancelled", + } as AgentStatus; + + const poll = getBenchmarkAgentBatchStatusShared("batch-1"); + setBenchmarkAgentBatchStatusShared(cancelled); + release(running); + + await expect(poll).resolves.toBe(cancelled); + await expect(getBenchmarkAgentBatchStatusShared("batch-1")).resolves.toBe( + cancelled + ); + expect(benchmarkApi.getAgentBatchStatus).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/hooks/benchmark/benchmarkRequestCoordinator.ts b/src/hooks/benchmark/benchmarkRequestCoordinator.ts new file mode 100644 index 000000000..77e9f5fd2 --- /dev/null +++ b/src/hooks/benchmark/benchmarkRequestCoordinator.ts @@ -0,0 +1,185 @@ +import { + type BenchmarkAgentBatchStatus, + type BenchmarkKind, + type BenchmarkRunStatus, + type BenchmarkTaskDetail, + type BenchmarkTaskIndexRow, + benchmarkApi, +} from "@src/api/tauri/benchmark"; + +const STATUS_CACHE_MS = 1_900; +const MAX_STATUS_ENTRIES = 32; + +interface SharedEntry { + fetchedAt?: number; + generation?: number; + inFlight?: Promise; + value?: T; +} + +const taskRequests = new Map>(); +const taskDetailRequests = new Map>(); +const agentBatchHistoryRequests = new Map< + string, + SharedEntry +>(); +const agentBatchStatusRequests = new Map< + string, + SharedEntry +>(); +const runStatusRequests = new Map>(); + +function prune(entries: Map>): void { + if (entries.size <= MAX_STATUS_ENTRIES) return; + const removable = [...entries.entries()] + .filter(([, entry]) => !entry.inFlight) + .sort( + ([, left], [, right]) => (left.fetchedAt ?? 0) - (right.fetchedAt ?? 0) + ); + for (const [key] of removable) { + if (entries.size <= MAX_STATUS_ENTRIES) break; + entries.delete(key); + } +} + +function sharedRequest( + entries: Map>, + key: string, + loader: () => Promise, + options?: { force?: boolean; maxAgeMs?: number } +): Promise { + const entry = entries.get(key) ?? {}; + if (entry.inFlight) return entry.inFlight; + if ( + !options?.force && + entry.value !== undefined && + entry.fetchedAt !== undefined && + Date.now() - entry.fetchedAt < (options?.maxAgeMs ?? 0) + ) { + return Promise.resolve(entry.value); + } + + const requestGeneration = entry.generation ?? 0; + const request = loader().then((value) => { + if ( + (entry.generation ?? 0) !== requestGeneration && + entry.value !== undefined + ) { + return entry.value; + } + return value; + }); + entry.inFlight = request; + entries.set(key, entry); + void request.then( + (value) => { + if (entry.inFlight !== request) return; + entry.value = value; + entry.fetchedAt = Date.now(); + entry.inFlight = undefined; + prune(entries); + }, + () => { + if (entry.inFlight === request) { + entry.inFlight = undefined; + if (entry.value === undefined) entries.delete(key); + } + } + ); + return request; +} + +function seedSharedEntry( + entries: Map>, + key: string, + value: T +): void { + const entry = entries.get(key) ?? {}; + entry.generation = (entry.generation ?? 0) + 1; + entry.inFlight = undefined; + entry.value = value; + entry.fetchedAt = Date.now(); + entries.set(key, entry); + prune(entries); +} + +export function listBenchmarkTasksShared(request: { + kind: BenchmarkKind; + limit: number; + sourcePath: string; +}): Promise { + const key = JSON.stringify([request.kind, request.sourcePath, request.limit]); + return sharedRequest(taskRequests, key, () => + benchmarkApi.listTasks(request) + ); +} + +export function getBenchmarkTaskShared(request: { + kind: BenchmarkKind; + sourcePath: string; + taskId: string; +}): Promise { + const key = JSON.stringify([ + request.kind, + request.sourcePath, + request.taskId, + ]); + return sharedRequest(taskDetailRequests, key, () => + benchmarkApi.getTask(request) + ); +} + +export function listBenchmarkAgentBatchHistoriesShared( + limit: number +): Promise { + return sharedRequest( + agentBatchHistoryRequests, + String(limit), + () => benchmarkApi.listAgentBatchHistories({ limit }), + { maxAgeMs: STATUS_CACHE_MS } + ); +} + +export function getBenchmarkAgentBatchStatusShared( + batchId: string, + options?: { force?: boolean } +): Promise { + return sharedRequest( + agentBatchStatusRequests, + batchId, + () => benchmarkApi.getAgentBatchStatus({ batchId }), + { force: options?.force, maxAgeMs: STATUS_CACHE_MS } + ); +} + +export function setBenchmarkAgentBatchStatusShared( + status: BenchmarkAgentBatchStatus +): void { + seedSharedEntry(agentBatchStatusRequests, status.batchId, status); +} + +export function getBenchmarkRunStatusShared( + runId: string, + options?: { force?: boolean } +): Promise { + return sharedRequest( + runStatusRequests, + runId, + () => benchmarkApi.getRunStatus({ runId }), + { force: options?.force, maxAgeMs: STATUS_CACHE_MS } + ); +} + +export function setBenchmarkRunStatusShared(status: BenchmarkRunStatus): void { + seedSharedEntry(runStatusRequests, status.runId, status); +} + +export const __TESTS_ONLY = { + reset() { + taskRequests.clear(); + taskDetailRequests.clear(); + agentBatchHistoryRequests.clear(); + agentBatchStatusRequests.clear(); + runStatusRequests.clear(); + }, +}; diff --git a/src/hooks/benchmark/useBenchmarkAgentBatchRun.ts b/src/hooks/benchmark/useBenchmarkAgentBatchRun.ts index cd4ebba3b..17d1494d7 100644 --- a/src/hooks/benchmark/useBenchmarkAgentBatchRun.ts +++ b/src/hooks/benchmark/useBenchmarkAgentBatchRun.ts @@ -37,6 +37,13 @@ import { chatPanelContentModeAtom, chatPanelMaximizedAtom, } from "@src/store/ui/chatPanelAtom"; +import { startVisibilityAwarePoll } from "@src/util/core/visibilityAwarePoll"; + +import { + getBenchmarkAgentBatchStatusShared, + listBenchmarkAgentBatchHistoriesShared, + setBenchmarkAgentBatchStatusShared, +} from "./benchmarkRequestCoordinator"; const AGENT_BATCH_STATUS_POLL_INTERVAL_MS = 2_000; @@ -139,9 +146,10 @@ export function useBenchmarkAgentBatchRun() { if (!batchStatus?.batchId) { return null; } - const nextStatus = await benchmarkApi.getAgentBatchStatus({ - batchId: batchStatus.batchId, - }); + const nextStatus = await getBenchmarkAgentBatchStatusShared( + batchStatus.batchId, + { force: true } + ); setBatchStatus(nextStatus); return nextStatus; }, [batchStatus?.batchId, setBatchStatus]); @@ -177,6 +185,7 @@ export function useBenchmarkAgentBatchRun() { launch, concurrency, }); + setBenchmarkAgentBatchStatusShared(status); setBatchStatus(status); setActiveBatchId(status.batchId); setActiveBatchTaskId(null); @@ -223,6 +232,7 @@ export function useBenchmarkAgentBatchRun() { const status = await benchmarkApi.cancelAgentBatch({ batchId: batchStatus.batchId, }); + setBenchmarkAgentBatchStatusShared(status); setBatchStatus(status); return status; } catch (error) { @@ -239,8 +249,7 @@ export function useBenchmarkAgentBatchRun() { return undefined; } let cancelled = false; - benchmarkApi - .listAgentBatchHistories({ limit: 1 }) + listBenchmarkAgentBatchHistoriesShared(1) .then((histories) => { if (cancelled || histories.length === 0) return; const [latestHistory] = histories; @@ -268,26 +277,27 @@ export function useBenchmarkAgentBatchRun() { } let cancelled = false; - const intervalId = window.setInterval(() => { - benchmarkApi - .getAgentBatchStatus({ batchId: batchStatus.batchId }) - .then((status) => { - if (!cancelled) { - setBatchStatus(status); - } - }) - .catch((error) => { - if (!cancelled) { - const message = - error instanceof Error ? error.message : String(error); - setBatchError(message); - } - }); - }, AGENT_BATCH_STATUS_POLL_INTERVAL_MS); + const batchId = batchStatus.batchId; + const poll = startVisibilityAwarePoll({ + intervalMs: AGENT_BATCH_STATUS_POLL_INTERVAL_MS, + task: async () => { + const status = await getBenchmarkAgentBatchStatusShared(batchId); + if (!cancelled) { + setBatchStatus(status); + } + }, + onError: (error) => { + if (!cancelled) { + const message = + error instanceof Error ? error.message : String(error); + setBatchError(message); + } + }, + }); return () => { cancelled = true; - window.clearInterval(intervalId); + poll.stop(); }; }, [ batchStatus?.batchId, diff --git a/src/hooks/benchmark/useBenchmarkRun.ts b/src/hooks/benchmark/useBenchmarkRun.ts index 94d44fc97..6eb75c20b 100644 --- a/src/hooks/benchmark/useBenchmarkRun.ts +++ b/src/hooks/benchmark/useBenchmarkRun.ts @@ -19,6 +19,12 @@ import { benchmarkSourcePathAtom, benchmarkTargetRepoPathAtom, } from "@src/store/benchmark"; +import { startVisibilityAwarePoll } from "@src/util/core/visibilityAwarePoll"; + +import { + getBenchmarkRunStatusShared, + setBenchmarkRunStatusShared, +} from "./benchmarkRequestCoordinator"; const RUN_STATUS_POLL_INTERVAL_MS = 2_000; @@ -124,6 +130,7 @@ export function useBenchmarkRun() { ? targetRepoPath : undefined, }); + setBenchmarkRunStatusShared(status); setRunStatus(status); return status; } catch (error) { @@ -151,6 +158,7 @@ export function useBenchmarkRun() { setRunError(null); try { const status = await benchmarkApi.cancelRun({ runId: runStatus.runId }); + setBenchmarkRunStatusShared(status); setRunStatus(status); } catch (error) { const message = error instanceof Error ? error.message : String(error); @@ -169,24 +177,25 @@ export function useBenchmarkRun() { } let cancelled = false; - const intervalId = window.setInterval(() => { - benchmarkApi - .getRunStatus({ runId: runStatus.runId }) - .then((status) => { - if (!cancelled) { - setRunStatus(status); - } - }) - .catch((error) => { - if (!cancelled) { - setRunError(error instanceof Error ? error.message : String(error)); - } - }); - }, RUN_STATUS_POLL_INTERVAL_MS); + const runId = runStatus.runId; + const poll = startVisibilityAwarePoll({ + intervalMs: RUN_STATUS_POLL_INTERVAL_MS, + task: async () => { + const status = await getBenchmarkRunStatusShared(runId); + if (!cancelled) { + setRunStatus(status); + } + }, + onError: (error) => { + if (!cancelled) { + setRunError(error instanceof Error ? error.message : String(error)); + } + }, + }); return () => { cancelled = true; - window.clearInterval(intervalId); + poll.stop(); }; }, [runStatus?.runId, runStatus?.status, setRunError, setRunStatus]); diff --git a/src/hooks/benchmark/useBenchmarkTasks.ts b/src/hooks/benchmark/useBenchmarkTasks.ts index 4d4c183af..af3615f7b 100644 --- a/src/hooks/benchmark/useBenchmarkTasks.ts +++ b/src/hooks/benchmark/useBenchmarkTasks.ts @@ -1,7 +1,6 @@ import { useAtom, useSetAtom } from "jotai"; -import { useCallback, useEffect } from "react"; +import { useCallback, useEffect, useMemo } from "react"; -import { benchmarkApi } from "@src/api/tauri/benchmark"; import { BENCHMARK_TASK_LIST_LIMIT, benchmarkErrorAtom, @@ -13,6 +12,12 @@ import { benchmarkTasksAtom, benchmarkTasksLoadingAtom, } from "@src/store/benchmark"; +import { LatestScopedTask } from "@src/util/core/latestScopedTask"; + +import { + getBenchmarkTaskShared, + listBenchmarkTasksShared, +} from "./benchmarkRequestCoordinator"; interface UseBenchmarkTasksOptions { loadDetail?: boolean; @@ -38,75 +43,32 @@ export function useBenchmarkTasks({ ); const [error, setError] = useAtom(benchmarkErrorAtom); const setSelectedTaskAtom = useSetAtom(benchmarkSelectedTaskAtom); + const taskListCoordinator = useMemo(() => new LatestScopedTask(), []); + const taskDetailCoordinator = useMemo(() => new LatestScopedTask(), []); const loadTasks = useCallback(async () => { const trimmedSourcePath = sourcePath.trim(); if (!trimmedSourcePath) { + taskListCoordinator.supersede(); setError(null); setTasks([]); setSelectedTaskId(null); setSelectedTaskAtom(null); - return; - } - - setIsLoadingTasks(true); - setError(null); - try { - const rows = await benchmarkApi.listTasks({ - kind, - sourcePath: trimmedSourcePath, - limit: BENCHMARK_TASK_LIST_LIMIT, - }); - setTasks(rows); - setSelectedTaskId((currentTaskId) => { - if (rows.some((row) => row.taskId === currentTaskId)) { - return currentTaskId; - } - return rows[0]?.taskId ?? null; - }); - } catch (loadError) { - setError( - loadError instanceof Error ? loadError.message : String(loadError) - ); - setTasks([]); - setSelectedTaskId(null); - setSelectedTaskAtom(null); - } finally { setIsLoadingTasks(false); - } - }, [ - kind, - setError, - setIsLoadingTasks, - setSelectedTaskAtom, - setSelectedTaskId, - setTasks, - sourcePath, - ]); - - useEffect(() => { - if (!loadOnMount) return; - - const trimmedSourcePath = sourcePath.trim(); - if (!trimmedSourcePath) { - setError(null); - setTasks([]); - setSelectedTaskId(null); - setSelectedTaskAtom(null); return; } - let cancelled = false; - async function loadInitialTasks() { + const scopeKey = JSON.stringify([kind, trimmedSourcePath]); + await taskListCoordinator.run(scopeKey, async (context) => { setIsLoadingTasks(true); setError(null); try { - const rows = await benchmarkApi.listTasks({ + const rows = await listBenchmarkTasksShared({ kind, sourcePath: trimmedSourcePath, limit: BENCHMARK_TASK_LIST_LIMIT, }); - if (cancelled) return; + if (!context.isCurrent()) return; setTasks(rows); setSelectedTaskId((currentTaskId) => { if (rows.some((row) => row.taskId === currentTaskId)) { @@ -115,7 +77,7 @@ export function useBenchmarkTasks({ return rows[0]?.taskId ?? null; }); } catch (loadError) { - if (cancelled) return; + if (!context.isCurrent()) return; setError( loadError instanceof Error ? loadError.message : String(loadError) ); @@ -123,67 +85,69 @@ export function useBenchmarkTasks({ setSelectedTaskId(null); setSelectedTaskAtom(null); } finally { - if (!cancelled) { + if (context.isCurrent()) { setIsLoadingTasks(false); } } - } - - loadInitialTasks(); - return () => { - cancelled = true; - }; + }); }, [ kind, - loadOnMount, setError, setIsLoadingTasks, setSelectedTaskAtom, setSelectedTaskId, setTasks, sourcePath, + taskListCoordinator, ]); + useEffect(() => { + if (!loadOnMount) return; + void loadTasks(); + return () => { + taskListCoordinator.supersede(); + }; + }, [loadOnMount, loadTasks, taskListCoordinator]); + useEffect(() => { if (!loadDetail) return; if (!selectedTaskId) { + taskDetailCoordinator.supersede(); setSelectedTask(null); + setIsLoadingDetail(false); return; } - let cancelled = false; const taskId = selectedTaskId; - - async function loadTaskDetail() { + const scopeKey = JSON.stringify([kind, sourcePath, taskId]); + void taskDetailCoordinator.run(scopeKey, async (context) => { setIsLoadingDetail(true); setError(null); try { - const detail = await benchmarkApi.getTask({ + const detail = await getBenchmarkTaskShared({ kind, sourcePath, taskId, }); - if (!cancelled) { + if (context.isCurrent()) { setSelectedTask(detail); } } catch (loadError) { - if (!cancelled) { + if (context.isCurrent()) { setError( loadError instanceof Error ? loadError.message : String(loadError) ); setSelectedTask(null); } } finally { - if (!cancelled) { + if (context.isCurrent()) { setIsLoadingDetail(false); } } - } - - loadTaskDetail(); + }); return () => { - cancelled = true; + taskDetailCoordinator.supersede(); }; }, [ kind, @@ -193,6 +157,7 @@ export function useBenchmarkTasks({ setIsLoadingDetail, setSelectedTask, sourcePath, + taskDetailCoordinator, ]); return { diff --git a/src/hooks/git/useRepoSelection/useRepoLoader.ts b/src/hooks/git/useRepoSelection/useRepoLoader.ts index 1aa1e69ef..6dce4b8bc 100644 --- a/src/hooks/git/useRepoSelection/useRepoLoader.ts +++ b/src/hooks/git/useRepoSelection/useRepoLoader.ts @@ -94,6 +94,7 @@ export function useRepoLoader(): UseRepoLoaderReturn { const isHotReloadRef = useRef(false); const selectedRepoIdRef = useRef(selectedRepoId); const loadGenerationRef = useRef(0); + const forceRefreshRequestedRef = useRef(false); // === HOT RELOAD FIX === if (repos.length > 0 && !loadedReposRef.current && !isHotReloadRef.current) { @@ -124,11 +125,13 @@ export function useRepoLoader(): UseRepoLoaderReturn { } const loadRepos = useCallback(async () => { - if (globalLoadInProgress) { + const forceRefresh = forceRefreshRequestedRef.current; + forceRefreshRequestedRef.current = false; + if (globalLoadInProgress && !forceRefresh) { return; } - if (globalReposLoaded && loadedReposRef.current) { + if (!forceRefresh && globalReposLoaded && loadedReposRef.current) { return; } @@ -139,7 +142,7 @@ export function useRepoLoader(): UseRepoLoaderReturn { let loadSucceeded = false; try { - const response = await getRepos(); + const response = await getRepos({ forceRefresh }); // Discard stale response when forceRefreshRepos() started a newer call. // The newer call already owns globalLoadInProgress and loadingReposRef, @@ -230,7 +233,7 @@ export function useRepoLoader(): UseRepoLoaderReturn { const forceRefreshRepos = useCallback(async () => { loadedReposRef.current = false; setGlobalReposLoaded(false); - setGlobalLoadInProgress(false); + forceRefreshRequestedRef.current = true; await loadRepos(); }, [loadRepos]); diff --git a/src/hooks/platform/useInlineWebview/__tests__/useInlineWebviewNativeVisibility.test.ts b/src/hooks/platform/useInlineWebview/__tests__/useInlineWebviewNativeVisibility.test.ts new file mode 100644 index 000000000..df6ed6446 --- /dev/null +++ b/src/hooks/platform/useInlineWebview/__tests__/useInlineWebviewNativeVisibility.test.ts @@ -0,0 +1,157 @@ +// @vitest-environment jsdom +import { act, createElement } from "react"; +import { type Root, createRoot } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { useInlineWebviewNativeVisibility } from "../useInlineWebviewNativeVisibility"; + +const invokeMock = vi.fn(); + +vi.mock("@tauri-apps/api/core", () => ({ + invoke: (...args: unknown[]) => invokeMock(...args), +})); + +const reactActEnvironment = globalThis as typeof globalThis & { + IS_REACT_ACT_ENVIRONMENT?: boolean; +}; +const labelRef = { current: "browser-session-test" }; + +function deferred(): { + promise: Promise; + resolve: () => void; +} { + let resolve!: () => void; + const promise = new Promise((done) => { + resolve = done; + }); + return { promise, resolve }; +} + +function VisibilityHarness({ + isVisible, + updatePosition, +}: { + isVisible: boolean; + updatePosition: (options?: { + force?: boolean; + show?: boolean; + }) => Promise; +}) { + useInlineWebviewNativeVisibility({ + isWebviewCreated: true, + isVisible, + isWebviewAvailable: true, + labelRef, + updatePosition, + log: vi.fn(), + }); + return null; +} + +describe("useInlineWebviewNativeVisibility", () => { + let container: HTMLDivElement; + let root: Root; + + beforeEach(() => { + reactActEnvironment.IS_REACT_ACT_ENVIRONMENT = true; + invokeMock.mockReset(); + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + }); + + afterEach(() => { + act(() => root.unmount()); + container.remove(); + Reflect.deleteProperty(reactActEnvironment, "IS_REACT_ACT_ENVIRONMENT"); + }); + + it("serializes native transitions and applies the latest visibility intent", async () => { + const hiddenTransition = deferred(); + const updatePosition = vi.fn().mockResolvedValue(undefined); + invokeMock.mockReturnValueOnce(hiddenTransition.promise); + + await act(async () => { + root.render( + createElement(VisibilityHarness, { + isVisible: false, + updatePosition, + }) + ); + await Promise.resolve(); + }); + + expect(invokeMock).toHaveBeenCalledWith("update_inline_webview_position", { + label: "browser-session-test", + x: -10000, + y: -10000, + width: 1, + height: 1, + }); + + await act(async () => { + root.render( + createElement(VisibilityHarness, { + isVisible: true, + updatePosition, + }) + ); + await Promise.resolve(); + }); + expect(updatePosition).not.toHaveBeenCalled(); + + await act(async () => { + hiddenTransition.resolve(); + await hiddenTransition.promise; + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(updatePosition).toHaveBeenCalledTimes(1); + expect(updatePosition).toHaveBeenCalledWith({ force: true, show: true }); + }); + + it("skips a queued show transition that a newer hide supersedes", async () => { + const firstHide = deferred(); + const updatePosition = vi.fn().mockResolvedValue(undefined); + invokeMock + .mockReturnValueOnce(firstHide.promise) + .mockResolvedValueOnce(undefined); + + await act(async () => { + root.render( + createElement(VisibilityHarness, { + isVisible: false, + updatePosition, + }) + ); + await Promise.resolve(); + }); + + act(() => { + root.render( + createElement(VisibilityHarness, { + isVisible: true, + updatePosition, + }) + ); + root.render( + createElement(VisibilityHarness, { + isVisible: false, + updatePosition, + }) + ); + }); + + await act(async () => { + firstHide.resolve(); + await firstHide.promise; + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(updatePosition).not.toHaveBeenCalled(); + expect(invokeMock).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/hooks/platform/useInlineWebview/__tests__/useWebviewLayout.test.ts b/src/hooks/platform/useInlineWebview/__tests__/useWebviewLayout.test.ts new file mode 100644 index 000000000..b96ea08e3 --- /dev/null +++ b/src/hooks/platform/useInlineWebview/__tests__/useWebviewLayout.test.ts @@ -0,0 +1,163 @@ +// @vitest-environment jsdom +import { act, createElement, useEffect } from "react"; +import { type Root, createRoot } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import type { UseWebviewLayoutReturn } from "../useWebviewLayout"; +import { useWebviewLayout } from "../useWebviewLayout"; +import { WEBVIEW_LAYOUT_CHANGED_EVENT } from "../webviewLayoutEvents"; + +const invokeMock = vi.fn().mockResolvedValue(undefined); + +vi.mock("@tauri-apps/api/core", () => ({ + invoke: (...args: unknown[]) => invokeMock(...args), +})); + +const reactActEnvironment = globalThis as typeof globalThis & { + IS_REACT_ACT_ENVIRONMENT?: boolean; +}; +const labelRef = { current: "browser-session-layout-test" }; +const layoutContainerRef = { current: null as HTMLDivElement | null }; + +class ResizeObserverMock { + static instances: ResizeObserverMock[] = []; + readonly disconnect = vi.fn(); + readonly observe = vi.fn(); + + constructor(readonly callback: ResizeObserverCallback) { + ResizeObserverMock.instances.push(this); + } + + unobserve(): void {} +} + +let latestLayout: UseWebviewLayoutReturn | null = null; + +function LayoutHarness({ isVisible }: { isVisible: boolean }) { + const layout = useWebviewLayout({ + containerRef: layoutContainerRef, + isWebviewCreated: true, + isWebviewAvailable: true, + isVisible, + labelRef, + log: vi.fn(), + }); + useEffect(() => { + latestLayout = layout; + return () => { + latestLayout = null; + }; + }, [layout]); + return null; +} + +describe("useWebviewLayout visibility lifecycle", () => { + let container: HTMLDivElement; + let root: Root; + let rectSpy: ReturnType; + + beforeEach(() => { + reactActEnvironment.IS_REACT_ACT_ENVIRONMENT = true; + invokeMock.mockClear(); + ResizeObserverMock.instances = []; + globalThis.ResizeObserver = + ResizeObserverMock as unknown as typeof ResizeObserver; + layoutContainerRef.current = document.createElement("div"); + rectSpy = vi + .spyOn(HTMLElement.prototype, "getBoundingClientRect") + .mockReturnValue({ + x: 10, + y: 20, + left: 10, + top: 20, + right: 310, + bottom: 220, + width: 300, + height: 200, + toJSON: () => ({}), + }); + container = document.createElement("div"); + document.body.appendChild(container); + root = createRoot(container); + }); + + afterEach(() => { + act(() => root.unmount()); + container.remove(); + rectSpy.mockRestore(); + latestLayout = null; + layoutContainerRef.current = null; + Reflect.deleteProperty(reactActEnvironment, "IS_REACT_ACT_ENVIRONMENT"); + }); + + it("disconnects observers and ignores layout work while hidden", async () => { + act(() => { + root.render(createElement(LayoutHarness, { isVisible: true })); + }); + expect(ResizeObserverMock.instances).toHaveLength(1); + + await act(async () => { + await latestLayout!.updatePosition({ force: true }); + }); + expect(invokeMock).toHaveBeenLastCalledWith( + "update_inline_webview_position", + expect.objectContaining({ + label: "browser-session-layout-test", + x: 10, + y: 20, + width: 300, + height: 200, + }) + ); + + act(() => { + root.render(createElement(LayoutHarness, { isVisible: false })); + }); + expect(ResizeObserverMock.instances[0].disconnect).toHaveBeenCalledTimes(1); + + invokeMock.mockClear(); + await act(async () => { + await latestLayout!.updatePosition({ force: true }); + window.dispatchEvent(new Event(WEBVIEW_LAYOUT_CHANGED_EVENT)); + await Promise.resolve(); + }); + expect(invokeMock).not.toHaveBeenCalled(); + }); + + it("uses the atomic reposition-and-show command when becoming visible", async () => { + act(() => { + root.render(createElement(LayoutHarness, { isVisible: true })); + }); + + await act(async () => { + await latestLayout!.updatePosition({ force: true, show: true }); + }); + + expect(invokeMock).toHaveBeenCalledWith( + "reposition_and_show_webview", + expect.objectContaining({ + label: "browser-session-layout-test", + x: 10, + y: 20, + width: 300, + height: 200, + }) + ); + }); + + it("cleans up every observer across repeated visible/hidden cycles", () => { + for (let index = 0; index < 20; index += 1) { + act(() => { + root.render(createElement(LayoutHarness, { isVisible: true })); + }); + act(() => { + root.render(createElement(LayoutHarness, { isVisible: false })); + }); + } + + expect(ResizeObserverMock.instances).toHaveLength(20); + for (const observer of ResizeObserverMock.instances) { + expect(observer.disconnect).toHaveBeenCalledTimes(1); + } + }); +}); diff --git a/src/hooks/platform/useInlineWebview/useInlineWebview.ts b/src/hooks/platform/useInlineWebview/useInlineWebview.ts index 0ebfea3af..dba41941e 100644 --- a/src/hooks/platform/useInlineWebview/useInlineWebview.ts +++ b/src/hooks/platform/useInlineWebview/useInlineWebview.ts @@ -77,6 +77,7 @@ export function useInlineWebview( containerRef, isWebviewCreated, isWebviewAvailable, + isVisible, labelRef, log, }); diff --git a/src/hooks/platform/useInlineWebview/useInlineWebviewNativeVisibility.ts b/src/hooks/platform/useInlineWebview/useInlineWebviewNativeVisibility.ts index e9bb5adfd..32befd22c 100644 --- a/src/hooks/platform/useInlineWebview/useInlineWebviewNativeVisibility.ts +++ b/src/hooks/platform/useInlineWebview/useInlineWebviewNativeVisibility.ts @@ -1,12 +1,15 @@ import { invoke } from "@tauri-apps/api/core"; -import { type MutableRefObject, useEffect } from "react"; +import { type MutableRefObject, useEffect, useRef } from "react"; export interface UseInlineWebviewNativeVisibilityParams { isWebviewCreated: boolean; isVisible: boolean; isWebviewAvailable: boolean; labelRef: MutableRefObject; - updatePosition: (options?: { force?: boolean }) => Promise; + updatePosition: (options?: { + force?: boolean; + show?: boolean; + }) => Promise; log: (...args: unknown[]) => void; } @@ -21,22 +24,21 @@ export function useInlineWebviewNativeVisibility( updatePosition, log, } = params; + const transitionGenerationRef = useRef(0); + const transitionQueueRef = useRef>(Promise.resolve()); useEffect(() => { if (!isWebviewCreated || !isWebviewAvailable) return; - let cancelled = false; + const generation = ++transitionGenerationRef.current; const handleVisibility = async () => { + if (generation !== transitionGenerationRef.current) return; + try { if (isVisible) { log("Showing WebView (isVisible=true)"); - await updatePosition({ force: true }); - if (cancelled) return; - await invoke("set_inline_webview_visibility", { - label: labelRef.current, - visible: true, - }); + await updatePosition({ force: true, show: true }); } else { log("Staging WebView offscreen (isVisible=false, but still mounted)"); await invoke("update_inline_webview_position", { @@ -48,16 +50,24 @@ export function useInlineWebviewNativeVisibility( }); } } catch (err) { - if (!cancelled) { + if (generation === transitionGenerationRef.current) { log("Visibility change failed:", err); } } }; - void handleVisibility(); + // Native WKWebView mutations are serialized per React owner. A newer + // visibility intent invalidates queued work before it reaches Tauri, while + // an already-running mutation is allowed to finish before the latest + // transition applies the final state. + transitionQueueRef.current = transitionQueueRef.current + .catch(() => undefined) + .then(handleVisibility); return () => { - cancelled = true; + if (transitionGenerationRef.current === generation) { + transitionGenerationRef.current += 1; + } }; }, [ isWebviewCreated, diff --git a/src/hooks/platform/useInlineWebview/useWebviewLayout.ts b/src/hooks/platform/useInlineWebview/useWebviewLayout.ts index 25f299d88..7ceee8496 100644 --- a/src/hooks/platform/useInlineWebview/useWebviewLayout.ts +++ b/src/hooks/platform/useInlineWebview/useWebviewLayout.ts @@ -22,20 +22,30 @@ export interface UseWebviewLayoutParams { containerRef: RefObject; isWebviewCreated: boolean; isWebviewAvailable: boolean; + isVisible: boolean; labelRef: MutableRefObject; log: (...args: unknown[]) => void; } export interface UseWebviewLayoutReturn { getContainerRect: () => DOMRect | null; - updatePosition: (options?: { force?: boolean }) => Promise; + updatePosition: (options?: { + force?: boolean; + show?: boolean; + }) => Promise; } export function useWebviewLayout( params: UseWebviewLayoutParams ): UseWebviewLayoutReturn { - const { containerRef, isWebviewCreated, isWebviewAvailable, labelRef, log } = - params; + const { + containerRef, + isWebviewCreated, + isWebviewAvailable, + isVisible, + labelRef, + log, + } = params; const resizeObserverRef = useRef(null); const scrollListenerRef = useRef<(() => void) | null>(null); @@ -52,8 +62,8 @@ export function useWebviewLayout( }, [containerRef]); const updatePosition = useCallback( - async (options?: { force?: boolean }) => { - if (!isWebviewCreated || !containerRef.current) return; + async (options?: { force?: boolean; show?: boolean }) => { + if (!isWebviewCreated || !isVisible || !containerRef.current) return; const rect = getContainerRect(); if (!rect) return; @@ -86,16 +96,21 @@ export function useWebviewLayout( lastResizeRect.current = nativeFrame; try { - await invoke("update_inline_webview_position", { - label: labelRef.current, - ...nativeFrame, - }); + await invoke( + options?.show + ? "reposition_and_show_webview" + : "update_inline_webview_position", + { + label: labelRef.current, + ...nativeFrame, + } + ); log("Position updated:", { rect, nativeFrame }); } catch (err) { log("Failed to update position:", err); } }, - [isWebviewCreated, containerRef, getContainerRect, labelRef, log] + [isWebviewCreated, isVisible, containerRef, getContainerRect, labelRef, log] ); const debouncedUpdatePosition = useDebouncedCallback(() => { @@ -103,7 +118,7 @@ export function useWebviewLayout( }, DEBOUNCE_DELAYS.FRAME); useEffect(() => { - if (!containerRef.current || !isWebviewAvailable) return; + if (!containerRef.current || !isWebviewAvailable || !isVisible) return; resizeObserverRef.current = new ResizeObserver(() => { debouncedUpdatePosition(); @@ -115,10 +130,10 @@ export function useWebviewLayout( resizeObserverRef.current?.disconnect(); debouncedUpdatePosition.cancel(); }; - }, [containerRef, isWebviewAvailable, debouncedUpdatePosition]); + }, [containerRef, isWebviewAvailable, isVisible, debouncedUpdatePosition]); useEffect(() => { - if (!isWebviewCreated || !isWebviewAvailable) return; + if (!isWebviewCreated || !isWebviewAvailable || !isVisible) return; const scaleUpdateTimers = new Set(); @@ -191,6 +206,7 @@ export function useWebviewLayout( containerRef, isWebviewCreated, isWebviewAvailable, + isVisible, debouncedUpdatePosition, updatePosition, ]); diff --git a/src/modules/ProjectManager/Projects/index.tsx b/src/modules/ProjectManager/Projects/index.tsx index cd2052029..a5a80cba1 100644 --- a/src/modules/ProjectManager/Projects/index.tsx +++ b/src/modules/ProjectManager/Projects/index.tsx @@ -48,6 +48,7 @@ import { Placeholder } from "@src/modules/shared/layouts/blocks"; import { ContentSearchPalette } from "@src/scaffold/GlobalSpotlight/palettes"; import { projectListRefreshAtom } from "@src/store/project/projectAtom"; import type { Project } from "@src/types/core/project"; +import { LatestScopedTask } from "@src/util/core/latestScopedTask"; import { confirmDestructiveAction } from "@src/util/dialogs/confirmDestructiveAction"; import { ProjectRow, ProjectsPageHeader } from "./components"; @@ -141,54 +142,43 @@ const ProjectsPage: React.FC = ({ const [fileProjectsLoading, setFileProjectsLoading] = useState(false); const [fileProjectsLoaded, setFileProjectsLoaded] = useState(false); const fileProjectsLoadedRef = useRef(false); - const loadLifecycleRef = useRef({ mounted: true, generation: 0 }); const [fileError, setFileError] = useState(null); - - useEffect(() => { - const lifecycle = loadLifecycleRef.current; - lifecycle.mounted = true; - return () => { - lifecycle.mounted = false; - lifecycle.generation += 1; - }; - }, []); + const projectLoadCoordinator = useMemo(() => new LatestScopedTask(), []); const loadProjectsForRepo = useCallback(async () => { - const generation = ++loadLifecycleRef.current.generation; - const isCurrent = () => { - const lifecycle = loadLifecycleRef.current; - return lifecycle.mounted && lifecycle.generation === generation; - }; - setFileProjectsLoading(true); - setFileError(null); - try { - const [projectsData, linearProjects] = await Promise.all([ - projectApi.readProjects({ orgId }), - includeExternalSources ? loadWorkspaceLinearProjects() : [], - ]); - if (!isCurrent()) return; - const localProjects = projectsData.map((project) => - projectDataToUI(project, { - labelMap: EMPTY_LABEL_MAP, - memberMap: EMPTY_MEMBER_MAP, - }) - ); - setFileProjects([...localProjects, ...linearProjects]); - fileProjectsLoadedRef.current = true; - setFileProjectsLoaded(true); - } catch (err) { - if (!isCurrent()) return; - log.error("[ProjectsPage] Failed to load projects:", err); - if (!fileProjectsLoadedRef.current) { - setFileProjects([]); + const scopeKey = JSON.stringify([orgId ?? null, includeExternalSources]); + await projectLoadCoordinator.run(scopeKey, async (context) => { + setFileProjectsLoading(true); + setFileError(null); + try { + const [projectsData, linearProjects] = await Promise.all([ + projectApi.readProjects({ orgId }), + includeExternalSources ? loadWorkspaceLinearProjects() : [], + ]); + if (!context.isCurrent()) return; + const localProjects = projectsData.map((project) => + projectDataToUI(project, { + labelMap: EMPTY_LABEL_MAP, + memberMap: EMPTY_MEMBER_MAP, + }) + ); + setFileProjects([...localProjects, ...linearProjects]); + fileProjectsLoadedRef.current = true; + setFileProjectsLoaded(true); + } catch (err) { + if (!context.isCurrent()) return; + log.error("[ProjectsPage] Failed to load projects:", err); + if (!fileProjectsLoadedRef.current) { + setFileProjects([]); + } + setFileError( + err instanceof Error ? err.message : t("projects.loadProjectsFailed") + ); + } finally { + if (context.isCurrent()) setFileProjectsLoading(false); } - setFileError( - err instanceof Error ? err.message : t("projects.loadProjectsFailed") - ); - } finally { - if (isCurrent()) setFileProjectsLoading(false); - } - }, [includeExternalSources, orgId, t]); + }); + }, [includeExternalSources, orgId, projectLoadCoordinator, t]); const loadFileProjects = useCallback(async () => { await loadProjectsForRepo(); @@ -196,7 +186,10 @@ const ProjectsPage: React.FC = ({ useEffect(() => { void loadProjectsForRepo(); - }, [loadProjectsForRepo, refreshSignal]); + return () => { + projectLoadCoordinator.supersede(); + }; + }, [loadProjectsForRepo, projectLoadCoordinator, refreshSignal]); useProjectDataChanged( useCallback(() => { diff --git a/src/modules/ProjectManager/WorkItems/hooks/__tests__/TEST_CASES.md b/src/modules/ProjectManager/WorkItems/hooks/__tests__/TEST_CASES.md new file mode 100644 index 000000000..c987150c0 --- /dev/null +++ b/src/modules/ProjectManager/WorkItems/hooks/__tests__/TEST_CASES.md @@ -0,0 +1,18 @@ +# Work Items async loading test cases + +## View-data requests + +- The mount effect and a same-scope project-data event share one request. +- Filtered reads with the same project, status, and search query share only + their active IPC request; a later intentional refresh still reaches Rust. +- Changing project, status, or debounced search supersedes the prior scope. +- A late response from the superseded scope cannot replace the visible data, + error, or loading state. +- Failed requests release their scope so the same filters can retry. + +## Workspace aggregates + +- Mount, manual refresh, and a project-data event share equal in-flight work. +- Switching org or external-source mode starts a new generation. +- Linear and local results from an older generation cannot overwrite the new + workspace selection. diff --git a/src/modules/ProjectManager/WorkItems/hooks/useWorkItemsData.ts b/src/modules/ProjectManager/WorkItems/hooks/useWorkItemsData.ts index f101a72dd..dd7cfb0a9 100644 --- a/src/modules/ProjectManager/WorkItems/hooks/useWorkItemsData.ts +++ b/src/modules/ProjectManager/WorkItems/hooks/useWorkItemsData.ts @@ -28,6 +28,7 @@ import { useDebouncedCallback } from "@src/hooks/perf"; import { useProjectDataChanged } from "@src/hooks/project"; import { useCurrentUserMemberIds } from "@src/hooks/project/useCurrentUserMemberId"; import type { WorkItem as WorkItemExtended } from "@src/types/core/workItem"; +import { LatestScopedTask } from "@src/util/core/latestScopedTask"; import { type OnAssignmentChanges, type StatusFilterType } from "../types"; import { toWorkItemPartialUpdate } from "../workItemPartialUpdate"; @@ -118,6 +119,7 @@ export function useWorkItemsData({ const [viewData, setViewData] = useState(null); const [viewLoading, setViewLoading] = useState(false); const [viewError, setViewError] = useState(null); + const viewLoadCoordinator = useMemo(() => new LatestScopedTask(), []); // Debounced search query for IPC calls (avoid IPC on every keystroke) const [debouncedSearchQuery, setDebouncedSearchQuery] = useState(searchQuery); @@ -133,33 +135,49 @@ export function useWorkItemsData({ const fetchViewData = useCallback(async () => { if (!projectSlug) { + viewLoadCoordinator.supersede(); setViewData(null); + setViewLoading(false); return; } - setViewLoading(true); - setViewError(null); - - try { - await projectApi.purgeExpiredDeletedWorkItems(projectSlug); - const data = await projectApi.readWorkItemsViewData(projectSlug, { - statusFilter: statusFilter !== "all" ? statusFilter : undefined, - searchQuery: debouncedSearchQuery.trim() || undefined, - }); - setViewData(data); - } catch (err) { - const message = - err instanceof Error ? err.message : "Failed to load work items"; - logger.error("View data fetch error:", err); - setViewError(message); - } finally { - setViewLoading(false); - } - }, [projectSlug, statusFilter, debouncedSearchQuery]); + const normalizedSearch = debouncedSearchQuery.trim(); + const scopeKey = JSON.stringify([ + projectSlug, + statusFilter, + normalizedSearch, + ]); + await viewLoadCoordinator.run(scopeKey, async (context) => { + setViewLoading(true); + setViewError(null); + + try { + await projectApi.purgeExpiredDeletedWorkItems(projectSlug); + const data = await projectApi.readWorkItemsViewData(projectSlug, { + statusFilter: statusFilter !== "all" ? statusFilter : undefined, + searchQuery: normalizedSearch || undefined, + }); + if (context.isCurrent()) { + setViewData(data); + } + } catch (err) { + if (!context.isCurrent()) return; + const message = + err instanceof Error ? err.message : "Failed to load work items"; + logger.error("View data fetch error:", err); + setViewError(message); + } finally { + if (context.isCurrent()) { + setViewLoading(false); + } + } + }); + }, [debouncedSearchQuery, projectSlug, statusFilter, viewLoadCoordinator]); useEffect(() => { - fetchViewData(); - }, [fetchViewData]); + void fetchViewData(); + return () => viewLoadCoordinator.supersede(); + }, [fetchViewData, viewLoadCoordinator]); // Listen for orgii-data-changed events useProjectDataChanged( diff --git a/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/sidebarSessionRefresh.test.ts b/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/sidebarSessionRefresh.test.ts index 4df486e60..edf50e812 100644 --- a/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/sidebarSessionRefresh.test.ts +++ b/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/sidebarSessionRefresh.test.ts @@ -77,4 +77,69 @@ describe("rescanSidebarSessions", () => { forceRefresh: true, }); }); + + it("shares one in-flight rescan between rapid refresh requests", async () => { + let releaseRescan!: () => void; + mocks.externalHistoryRescanSources.mockImplementation( + () => + new Promise((resolve) => { + releaseRescan = resolve; + }) + ); + + const firstRescan = rescanSidebarSessions(); + const secondRescan = rescanSidebarSessions(); + const thirdRescan = rescanSidebarSessions(); + + expect(mocks.externalHistoryRescanSources).toHaveBeenCalledTimes(1); + expect(mocks.loadSessionRoster).not.toHaveBeenCalled(); + + releaseRescan(); + await Promise.all([firstRescan, secondRescan, thirdRescan]); + expect(mocks.loadSessionRoster).toHaveBeenCalledTimes(1); + }); + + it("releases the in-flight guard after a failed rescan", async () => { + mocks.externalHistoryRescanSources + .mockRejectedValueOnce(new Error("scan failed")) + .mockResolvedValueOnce(undefined); + + await expect(rescanSidebarSessions()).rejects.toThrow("scan failed"); + await expect(rescanSidebarSessions()).resolves.toBeUndefined(); + + expect(mocks.externalHistoryRescanSources).toHaveBeenCalledTimes(2); + expect(mocks.loadSessionRoster).toHaveBeenCalledTimes(1); + }); + + it("runs a trailing rescan for a changed scope even if the old scan fails", async () => { + let rejectFirstRescan!: (error: Error) => void; + mocks.externalHistoryRescanSources + .mockImplementationOnce( + () => + new Promise((_resolve, reject) => { + rejectFirstRescan = reject; + }) + ) + .mockResolvedValueOnce(undefined); + + const firstRescan = rescanSidebarSessions(); + mocks.store?.set(dataSourceConfigAtom, { + warp: { enabled: false, frequency: "default", lastScannedAt: null }, + }); + const changedScopeRescan = rescanSidebarSessions(); + + expect(mocks.externalHistoryRescanSources).toHaveBeenCalledTimes(1); + rejectFirstRescan(new Error("obsolete scan failed")); + const results = await Promise.allSettled([firstRescan, changedScopeRescan]); + + expect(results.map(({ status }) => status)).toEqual([ + "rejected", + "fulfilled", + ]); + expect(mocks.externalHistoryRescanSources).toHaveBeenCalledTimes(2); + expect(mocks.externalHistoryRescanSources.mock.calls[1][0]).not.toContain( + "warp" + ); + expect(mocks.loadSessionRoster).toHaveBeenCalledTimes(1); + }); }); diff --git a/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/sidebarSessionRefresh.ts b/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/sidebarSessionRefresh.ts index 1ca1f120e..42345f8b5 100644 --- a/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/sidebarSessionRefresh.ts +++ b/src/scaffold/NavigationSidebar/connectors/WorkstationSidebarConnector/sidebarSessionRefresh.ts @@ -2,6 +2,7 @@ import { useEffect } from "react"; import { IMPORTED_HISTORY_SOURCE_DESCRIPTORS, + type ImportedHistorySourceId, externalHistoryRescanSources, } from "@src/api/tauri/externalHistory"; import { @@ -21,21 +22,41 @@ import { SIDEBAR_SESSION_IDLE_REFRESH_INTERVAL_MS, } from "../sidebarConnectorUtils"; -/** Rescan every enabled external source, then refresh the canonical roster. */ -export async function rescanSidebarSessions(): Promise { - const store = getInstrumentedStore(); +type SessionStore = ReturnType; + +interface SidebarRescanFlight { + scopeKey: string; + promise: Promise; +} + +const rescanInFlightByStore = new WeakMap(); + +function getRescanScope(store: SessionStore): { + scopeKey: string; + sourceIds: ImportedHistorySourceId[]; +} { if (!store.get(externalSessionsEnabledAtom)) { - // External sessions are switched off entirely — nothing to rescan, and - // the sidebar reload below would be a no-op for external categories. - await loadSessionRoster({ forceRefresh: true }); - return; + return { scopeKey: "external-sessions-disabled", sourceIds: [] }; } const config = store.get(dataSourceConfigAtom); const sourceIds = IMPORTED_HISTORY_SOURCE_DESCRIPTORS.filter( ({ sourceId }) => getSourceConfig(config, sourceId).enabled ).map(({ sourceId }) => sourceId); + return { scopeKey: JSON.stringify(sourceIds), sourceIds }; +} - const scanResult = await externalHistoryRescanSources(sourceIds); +async function performSidebarSessionsRescan( + store: SessionStore, + sourceIds: readonly ImportedHistorySourceId[] +): Promise { + if (!store.get(externalSessionsEnabledAtom)) { + // External sessions are switched off entirely — nothing to rescan, and + // the sidebar reload below would be a no-op for external categories. + await loadSessionRoster({ forceRefresh: true }); + return; + } + + const scanResult = await externalHistoryRescanSources([...sourceIds]); // Explicit refresh: reload unconditionally. Even a rescan that wrote // nothing can follow cache writes from other surfaces' syncs (e.g. a // continuation demotion) that the sidebar never rendered. @@ -58,6 +79,32 @@ export async function rescanSidebarSessions(): Promise { }); } +/** Coalesce overlapping refreshes without letting an obsolete scope win. */ +export async function rescanSidebarSessions(): Promise { + const store = getInstrumentedStore(); + const { scopeKey, sourceIds } = getRescanScope(store); + const inFlight = rescanInFlightByStore.get(store); + if (inFlight) { + if (inFlight.scopeKey === scopeKey) return inFlight.promise; + try { + await inFlight.promise; + } catch { + // A failed obsolete scope must not suppress the current source set. + } + return rescanSidebarSessions(); + } + + const pass = performSidebarSessionsRescan(store, sourceIds); + rescanInFlightByStore.set(store, { scopeKey, promise: pass }); + try { + await pass; + } finally { + if (rescanInFlightByStore.get(store)?.promise === pass) { + rescanInFlightByStore.delete(store); + } + } +} + export function useSidebarSessionRefreshEffects(): void { useEffect(() => { void loadSessionRoster(); diff --git a/src/store/session/sessionAtom/__tests__/sidebarLoaders.test.ts b/src/store/session/sessionAtom/__tests__/sidebarLoaders.test.ts index 901d7ff1e..b2abb5ba7 100644 --- a/src/store/session/sessionAtom/__tests__/sidebarLoaders.test.ts +++ b/src/store/session/sessionAtom/__tests__/sidebarLoaders.test.ts @@ -3,18 +3,29 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { IMPORTED_HISTORY_SOURCES } from "@src/api/tauri/externalHistory"; -import { dataSourceConfigAtom } from "../../dataSourceConfigAtom"; -import { sessionsAtom } from "../atoms"; +import { + dataSourceConfigAtom, + externalSessionsEnabledAtom, +} from "../../dataSourceConfigAtom"; +import { + sessionLastLoadedAtom, + sessionLoadingAtom, + sessionsAtom, +} from "../atoms"; import { __TESTS_ONLY, loadMoreCategory, loadSessionRoster, + loadSessions, loadSidebarSessionById, loadSidebarSessions, loadSidebarSessionsByIds, refreshRecentNativeSessions, } from "../loaders"; -import { sessionPaginationAtom } from "../paginationAtoms"; +import { + BASE_SESSION_LIST_CATEGORIES, + sessionPaginationAtom, +} from "../paginationAtoms"; const mocks = vi.hoisted(() => ({ externalHistorySidebarList: vi.fn(), @@ -218,6 +229,206 @@ describe("loadSidebarSessions", () => { } }); + it("shares one in-flight initial load between concurrent consumers", async () => { + let releaseExternalHistory!: () => void; + const externalHistoryPending = new Promise((resolve) => { + releaseExternalHistory = resolve; + }); + mocks.sessionAggregateList.mockResolvedValue({ sessions: [] }); + mocks.externalHistorySidebarList.mockImplementation( + async (request: { + requests: Array<{ + source: string; + buckets: Array<{ bucket: string }>; + }>; + }) => { + await externalHistoryPending; + return { + sources: request.requests.map((sourceRequest) => ({ + source: sourceRequest.source, + buckets: sourceRequest.buckets.map(({ bucket }) => ({ + bucket, + sessions: [], + hasMore: false, + })), + })), + }; + } + ); + + const firstLoad = loadSidebarSessions({ forceRefresh: true }); + const secondLoad = loadSidebarSessions({ forceRefresh: true }); + const thirdLoad = loadSidebarSessions(); + + expect(mocks.sessionAggregateList).toHaveBeenCalledTimes( + BASE_SESSION_LIST_CATEGORIES.length + ); + expect(mocks.externalHistorySidebarList).toHaveBeenCalledTimes(1); + + releaseExternalHistory(); + await Promise.all([firstLoad, secondLoad, thirdLoad]); + expect(mocks.persistSessions).toHaveBeenCalledTimes(1); + }); + + it("does not let a cache hit suppress a same-tick forced refresh", async () => { + mocks.store?.set(sessionLastLoadedAtom, Date.now()); + mocks.sessionAggregateList.mockResolvedValue({ sessions: [] }); + mocks.externalHistorySidebarList.mockImplementation( + async (request: { + requests: Array<{ + source: string; + buckets: Array<{ bucket: string }>; + }>; + }) => ({ + sources: request.requests.map((sourceRequest) => ({ + source: sourceRequest.source, + buckets: sourceRequest.buckets.map(({ bucket }) => ({ + bucket, + sessions: [], + hasMore: false, + })), + })), + }) + ); + + const cachedLoad = loadSidebarSessions(); + const forcedLoad = loadSidebarSessions({ forceRefresh: true }); + await Promise.all([cachedLoad, forcedLoad]); + + expect(mocks.sessionAggregateList).toHaveBeenCalledTimes( + BASE_SESSION_LIST_CATEGORIES.length + ); + expect(mocks.externalHistorySidebarList).toHaveBeenCalledTimes(1); + }); + + it("shares one flat-list request between concurrent hook consumers", async () => { + let release!: (value: { sessions: unknown[] }) => void; + mocks.sessionAggregateList.mockImplementation( + () => + new Promise<{ sessions: unknown[] }>((resolve) => { + release = resolve; + }) + ); + + const first = loadSessions(); + const second = loadSessions(); + + expect(mocks.sessionAggregateList).toHaveBeenCalledTimes(1); + release({ sessions: [] }); + await Promise.all([first, second]); + expect(mocks.persistSessions).toHaveBeenCalledTimes(1); + }); + + it("runs one forced flat-list refresh after an active non-forced load", async () => { + let releaseFirst!: (value: { sessions: unknown[] }) => void; + const staleSession = { + session_id: "stale", + name: "Stale", + status: "completed" as const, + created_at: "2026-07-01T00:00:00Z", + updated_at: "2026-07-01T00:00:00Z", + }; + const freshSession = { + ...staleSession, + session_id: "fresh", + name: "Fresh", + updated_at: "2026-07-02T00:00:00Z", + }; + mocks.sessionAggregateList + .mockImplementationOnce( + () => + new Promise<{ sessions: unknown[] }>((resolve) => { + releaseFirst = resolve; + }) + ) + .mockResolvedValueOnce({ sessions: [freshSession] }); + + const first = loadSessions(); + const forced = loadSessions({ forceRefresh: true }); + + expect(mocks.sessionAggregateList).toHaveBeenCalledTimes(1); + releaseFirst({ sessions: [staleSession] }); + await first; + await forced; + + expect(mocks.sessionAggregateList).toHaveBeenCalledTimes(2); + expect(mocks.store?.get(sessionsAtom)).toEqual([freshSession]); + expect(mocks.persistSessions).toHaveBeenCalledTimes(1); + expect(mocks.store?.get(sessionLoadingAtom)).toBe(false); + }); + + it("treats external-source configuration as part of the flat-list scope", async () => { + let releaseFirst!: (value: { sessions: unknown[] }) => void; + mocks.sessionAggregateList + .mockImplementationOnce( + () => + new Promise<{ sessions: unknown[] }>((resolve) => { + releaseFirst = resolve; + }) + ) + .mockResolvedValueOnce({ sessions: [] }); + + const first = loadSessions(); + mocks.store?.set(externalSessionsEnabledAtom, false); + const changedScope = loadSessions(); + + releaseFirst({ sessions: [] }); + await Promise.all([first, changedScope]); + + expect(mocks.sessionAggregateList).toHaveBeenCalledTimes(2); + expect(mocks.sessionAggregateList).toHaveBeenLastCalledWith( + expect.objectContaining({ includeExternalHistory: false }) + ); + }); + + it("runs a trailing load when the data-source scope changes in flight", async () => { + let releaseFirstLoad!: () => void; + const firstLoadPending = new Promise((resolve) => { + releaseFirstLoad = resolve; + }); + mocks.sessionAggregateList.mockResolvedValue({ sessions: [] }); + mocks.externalHistorySidebarList.mockImplementation( + async (request: { + requests: Array<{ + source: string; + buckets: Array<{ bucket: string }>; + }>; + }) => { + await firstLoadPending; + return { + sources: request.requests.map((sourceRequest) => ({ + source: sourceRequest.source, + buckets: sourceRequest.buckets.map(({ bucket }) => ({ + bucket, + sessions: [], + hasMore: false, + })), + })), + }; + } + ); + + const firstLoad = loadSidebarSessions({ forceRefresh: true }); + mocks.store?.set(dataSourceConfigAtom, { + warp: { enabled: false, frequency: "default", lastScannedAt: null }, + }); + const changedScopeLoad = loadSidebarSessions({ forceRefresh: true }); + + expect(mocks.externalHistorySidebarList).toHaveBeenCalledTimes(1); + releaseFirstLoad(); + await Promise.all([firstLoad, changedScopeLoad]); + + expect(mocks.sessionAggregateList).toHaveBeenCalledTimes( + BASE_SESSION_LIST_CATEGORIES.length * 2 + ); + expect(mocks.externalHistorySidebarList).toHaveBeenCalledTimes(2); + const trailingSources = + mocks.externalHistorySidebarList.mock.calls[1][0].requests.map( + ({ source }: { source: string }) => source + ); + expect(trailingSources).not.toContain("warp"); + }); + it("continues each external date bucket from its own offset", async () => { mocks.sessionAggregateList.mockResolvedValue({ sessions: [] }); mocks.externalHistorySidebarList.mockImplementation( diff --git a/src/store/session/sessionAtom/loaders.ts b/src/store/session/sessionAtom/loaders.ts index 98ca2a698..8dccc683d 100644 --- a/src/store/session/sessionAtom/loaders.ts +++ b/src/store/session/sessionAtom/loaders.ts @@ -66,6 +66,8 @@ import type { Session, SessionStatus } from "./types"; const log = createLogger("SessionAtom"); const getStore = () => getInstrumentedStore(); +type SessionStore = ReturnType; + const BULK_CACHE_DURATION_MS = 5 * 60 * 1000; const DEFAULT_FLAT_LIST_PAGE_SIZE = 200; const RECENT_NATIVE_REFRESH_LIMIT = @@ -87,6 +89,20 @@ function exactSessionBatchLoadsForStore( return loads; } +interface FlatLoadFlight { + scopeKey: string; + forceRefresh: boolean; + generation: number; + promise: Promise; +} + +interface FlatLoadState { + generation: number; + flight?: FlatLoadFlight; +} + +const flatLoadStateByStore = new WeakMap(); + interface LoadSessionsOptions { repoPath?: string; orgId?: string; @@ -105,11 +121,41 @@ function loadSessionsCacheSignature(options?: LoadSessionsOptions): string { options?.projectSlug ?? "", options?.workItemId ?? "", options?.status ?? "", - options?.limit ?? "", - options?.offset ?? "", + options?.limit ?? DEFAULT_FLAT_LIST_PAGE_SIZE, + options?.offset ?? 0, ].join("\u001f"); } +interface FlatLoadScope { + cacheSignature: string; + disabledSources: string[]; + includeExternalHistory: boolean; + scopeKey: string; +} + +function getFlatLoadScope( + store: SessionStore, + options?: LoadSessionsOptions +): FlatLoadScope { + const disabledSources = Object.entries(store.get(dataSourceConfigAtom)) + .filter(([, config]) => config?.enabled === false) + .map(([sourceId]) => sourceId) + .sort(); + const includeExternalHistory = store.get(externalSessionsEnabledAtom); + const filterSignature = loadSessionsCacheSignature(options); + const scopeKey = JSON.stringify([ + filterSignature, + includeExternalHistory, + disabledSources, + ]); + return { + cacheSignature: scopeKey, + disabledSources, + includeExternalHistory, + scopeKey, + }; +} + function mergeSessions( prev: readonly Session[], incoming: readonly Session[] @@ -159,10 +205,10 @@ function replaceExternalHistorySourceFirstPage( } function setPaginationFor( + store: SessionStore, category: SessionListCategory, patch: Partial ) { - const store = getStore(); store.set(sessionPaginationAtom, (prev) => ({ ...prev, [category]: { ...prev[category], ...patch }, @@ -319,24 +365,13 @@ function mergeDateBucketPagination( return next; } -export const loadSessions = async (options?: LoadSessionsOptions) => { - const store = getStore(); - const { forceRefresh = false } = options || {}; - const cacheSignature = loadSessionsCacheSignature(options); - - const lastLoaded = store.get(sessionFlatListLastLoadedBySignatureAtom)[ - cacheSignature - ]; - const now = Date.now(); - - if ( - !forceRefresh && - lastLoaded && - now - lastLoaded < BULK_CACHE_DURATION_MS - ) { - return; - } - +async function performFlatSessionLoad( + store: SessionStore, + state: FlatLoadState, + generation: number, + scope: FlatLoadScope, + options?: LoadSessionsOptions +): Promise { store.set(sessionLoadingAtom, true); store.set(sessionErrorAtom, null); @@ -360,18 +395,14 @@ export const loadSessions = async (options?: LoadSessionsOptions) => { } : undefined; - const disabledSources = Object.entries(store.get(dataSourceConfigAtom)) - .filter(([, cfg]) => cfg?.enabled === false) - .map(([sourceId]) => sourceId); - const response = await sessionAggregateList({ ...filter, limit: filter?.limit ?? DEFAULT_FLAT_LIST_PAGE_SIZE, - includeExternalHistory: store.get(externalSessionsEnabledAtom), + includeExternalHistory: scope.includeExternalHistory, sortBy: filter?.sortBy ?? "updated_at", sortOrder: filter?.sortOrder ?? "desc", disabledExternalHistorySources: - disabledSources.length > 0 ? disabledSources : undefined, + scope.disabledSources.length > 0 ? scope.disabledSources : undefined, }); const fetched: Session[] = mergeGuestImportedSessions( @@ -382,20 +413,94 @@ export const loadSessions = async (options?: LoadSessionsOptions) => { (sessionB.updated_at || "").localeCompare(sessionA.updated_at || "") ); + if (state.generation !== generation) return; + store.set(sessionsAtom, fetched); persistSessions(fetched); store.set(sessionFlatListLastLoadedBySignatureAtom, (prev) => ({ ...prev, - [cacheSignature]: now, + [scope.cacheSignature]: Date.now(), })); } catch (error) { + if (state.generation !== generation) return; log.error("[SessionAtom] Failed to load sessions:", error); store.set( sessionErrorAtom, error instanceof Error ? error.message : "Failed to load sessions" ); } finally { - store.set(sessionLoadingAtom, false); + if (state.generation === generation) { + store.set(sessionLoadingAtom, false); + } + } +} + +/** + * Coordinate flat-list requests across every hook instance using this store. + * + * Equal scopes share one request. A stronger forced refresh or changed scope + * supersedes the current generation, waits for it to release the IPC slot, + * and then performs one trailing request. Superseded responses never write. + */ +export const loadSessions = async ( + options?: LoadSessionsOptions +): Promise => { + const store = getStore(); + const forceRefresh = options?.forceRefresh ?? false; + const scope = getFlatLoadScope(store, options); + const lastLoaded = store.get(sessionFlatListLastLoadedBySignatureAtom)[ + scope.cacheSignature + ]; + + if ( + !forceRefresh && + lastLoaded && + Date.now() - lastLoaded < BULK_CACHE_DURATION_MS + ) { + return; + } + + let state = flatLoadStateByStore.get(store); + if (!state) { + state = { generation: 0 }; + flatLoadStateByStore.set(store, state); + } + + const current = state.flight; + if (current) { + const currentSatisfiesRequest = + current.scopeKey === scope.scopeKey && + (!forceRefresh || current.forceRefresh); + if (currentSatisfiesRequest) return current.promise; + + // Fence the old response immediately, before waiting for its IPC call. + state.generation += 1; + await current.promise; + return loadSessions(options); + } + + const generation = state.generation + 1; + state.generation = generation; + const promise = performFlatSessionLoad( + store, + state, + generation, + scope, + options + ); + state.flight = { + scopeKey: scope.scopeKey, + forceRefresh, + generation, + promise, + }; + + try { + await promise; + } finally { + if (state.flight?.promise === promise) { + state.flight = undefined; + } } }; @@ -476,10 +581,14 @@ function replaceFirstPageForCategory( interface SidebarLoadOptions { pageSize?: number; forceRefresh?: boolean; + /** Internal data-source identity used to serialize scope changes. */ + scopeKey?: string; } -const performSidebarSessionLoad = async (options?: SidebarLoadOptions) => { - const store = getStore(); +const performSidebarSessionLoad = async ( + store: SessionStore, + options?: SidebarLoadOptions +) => { const pageSize = options?.pageSize ?? SESSION_SIDEBAR_PAGE_SIZE; const { forceRefresh = false } = options ?? {}; @@ -510,7 +619,7 @@ const performSidebarSessionLoad = async (options?: SidebarLoadOptions) => { }; for (const category of SESSION_LIST_CATEGORIES) { - setPaginationFor(category, { loading: true }); + setPaginationFor(store, category, { loading: true }); } const enabledCategories = SESSION_LIST_CATEGORIES.filter((category) => { @@ -518,7 +627,7 @@ const performSidebarSessionLoad = async (options?: SidebarLoadOptions) => { store.set(sessionsAtom, (prev) => replaceFirstPageForCategory(category, prev, [], false) ); - setPaginationFor(category, { + setPaginationFor(store, category, { loaded: 0, hasMore: false, loading: false, @@ -533,7 +642,7 @@ const performSidebarSessionLoad = async (options?: SidebarLoadOptions) => { store.set(sessionsAtom, (prev) => replaceFirstPageForCategory(category, prev, sessions) ); - setPaginationFor(category, { + setPaginationFor(store, category, { loaded: sessions.length, hasMore, loading: false, @@ -549,7 +658,7 @@ const performSidebarSessionLoad = async (options?: SidebarLoadOptions) => { applyInitialPage(category, result); } catch (error) { log.warn(`[SessionAtom] ${category} initial page failed:`, error); - setPaginationFor(category, { loading: false }); + setPaginationFor(store, category, { loading: false }); } }); @@ -577,7 +686,7 @@ const performSidebarSessionLoad = async (options?: SidebarLoadOptions) => { } catch (error) { log.warn("[SessionAtom] external history initial pages failed:", error); for (const { category } of importedCategories) { - setPaginationFor(category, { loading: false }); + setPaginationFor(store, category, { loading: false }); } } })(); @@ -594,6 +703,7 @@ function mergeSidebarLoadOptions( current: SidebarLoadOptions | null, requested: SidebarLoadOptions ): SidebarLoadOptions { + const scopeKey = requested.scopeKey ?? current?.scopeKey; return { pageSize: Math.max( current?.pageSize ?? SESSION_SIDEBAR_PAGE_SIZE, @@ -601,6 +711,7 @@ function mergeSidebarLoadOptions( ), forceRefresh: (current?.forceRefresh ?? false) || (requested.forceRefresh ?? false), + ...(scopeKey === undefined ? {} : { scopeKey }), }; } @@ -612,6 +723,7 @@ function sidebarLoadCovers( const activePageSize = active.pageSize ?? SESSION_SIDEBAR_PAGE_SIZE; const requestedPageSize = requested.pageSize ?? SESSION_SIDEBAR_PAGE_SIZE; return ( + active.scopeKey === requested.scopeKey && activePageSize >= requestedPageSize && ((active.forceRefresh ?? false) || !(requested.forceRefresh ?? false)) ); @@ -653,13 +765,56 @@ function createSidebarLoadCoordinator( } /** - * One process-wide session-roster loader. Overlapping mounts/refreshes join the - * active read; a stronger request (forced or larger page) is merged into one - * follow-up pass instead of starting a parallel category fan-out. + * One session-roster loader per Jotai store. Overlapping mounts/refreshes join + * the active read; a stronger request (forced, larger, or a changed data-source + * scope) is merged into one follow-up pass instead of starting a parallel + * category fan-out. WeakMap ownership prevents cross-window/store leakage. */ -export const loadSessionRoster = createSidebarLoadCoordinator( - performSidebarSessionLoad -); +const sidebarLoadCoordinatorByStore = new WeakMap< + SessionStore, + ReturnType +>(); +const sidebarLoadScopeByStore = new WeakMap(); + +function getSidebarLoadScopeKey(store: SessionStore): string { + const disabledSources = Object.entries(store.get(dataSourceConfigAtom)) + .filter(([, config]) => config?.enabled === false) + .map(([sourceId]) => sourceId) + .sort(); + return JSON.stringify([ + store.get(externalSessionsEnabledAtom), + disabledSources, + ]); +} + +function sidebarLoadCoordinatorForStore( + store: SessionStore +): ReturnType { + let coordinator = sidebarLoadCoordinatorByStore.get(store); + if (!coordinator) { + coordinator = createSidebarLoadCoordinator((options) => + performSidebarSessionLoad(store, options) + ); + sidebarLoadCoordinatorByStore.set(store, coordinator); + } + return coordinator; +} + +export const loadSessionRoster = ( + options: SidebarLoadOptions = {} +): Promise => { + const store = getStore(); + const scopeKey = getSidebarLoadScopeKey(store); + const previousScopeKey = sidebarLoadScopeByStore.get(store); + const scopeChanged = + previousScopeKey !== undefined && previousScopeKey !== scopeKey; + sidebarLoadScopeByStore.set(store, scopeKey); + return sidebarLoadCoordinatorForStore(store)({ + ...options, + forceRefresh: (options.forceRefresh ?? false) || scopeChanged, + scopeKey, + }); +}; /** * Compatibility alias for callers outside the roster surfaces. New Sidebar @@ -781,7 +936,7 @@ export const loadMoreCategory = async ( const current = store.get(sessionPaginationAtom)[category]; if (current.loading || !current.hasMore) return; - setPaginationFor(category, { loading: true }); + setPaginationFor(store, category, { loading: true }); try { const { sessions, hasMore, dateBuckets } = await loadCategoryPage( @@ -792,7 +947,7 @@ export const loadMoreCategory = async ( ); const primarySessions = sessions.filter(isPrimarySessionListSession); store.set(sessionsAtom, (prev) => mergeSessions(prev, primarySessions)); - setPaginationFor(category, { + setPaginationFor(store, category, { loaded: current.loaded + sessions.length, hasMore, loading: false, @@ -801,7 +956,7 @@ export const loadMoreCategory = async ( persistSessions(store.get(sessionsAtom)); } catch (error) { log.warn(`[SessionAtom] loadMoreCategory(${category}) failed:`, error); - setPaginationFor(category, { loading: false }); + setPaginationFor(store, category, { loading: false }); } }; diff --git a/src/util/platform/tauri/fileSearch.test.ts b/src/util/platform/tauri/fileSearch.test.ts new file mode 100644 index 000000000..464f2d985 --- /dev/null +++ b/src/util/platform/tauri/fileSearch.test.ts @@ -0,0 +1,14 @@ +import { describe, expect, it } from "vitest"; + +import { shouldPrewarmFileIndex } from "./fileSearch"; + +describe("shouldPrewarmFileIndex", () => { + it("allows visible and non-DOM callers", () => { + expect(shouldPrewarmFileIndex("visible")).toBe(true); + expect(shouldPrewarmFileIndex(undefined)).toBe(true); + }); + + it("skips proactive work for hidden windows", () => { + expect(shouldPrewarmFileIndex("hidden")).toBe(false); + }); +}); diff --git a/src/util/platform/tauri/fileSearch.ts b/src/util/platform/tauri/fileSearch.ts index dad6127b0..10802d6a6 100644 --- a/src/util/platform/tauri/fileSearch.ts +++ b/src/util/platform/tauri/fileSearch.ts @@ -16,6 +16,7 @@ import type { SearchResultItem } from "@src/scaffold/ContextMenu/types"; import { ensureTauriReady, invokeTauri, isTauriReady } from "./init"; const log = createLogger("FileSearch"); +const prewarmRequests = new Map>(); // ============================================ // Types @@ -137,17 +138,32 @@ export async function indexProjectFiles( */ export async function prewarmFileIndex(rootPath: string): Promise { if (!isTauriReady()) return 0; - - try { - const count = await invokeTauri("prewarm_file_index", { - rootPath, + if (!shouldPrewarmFileIndex(globalThis.document?.visibilityState)) return 0; + + const existingRequest = prewarmRequests.get(rootPath); + if (existingRequest) return existingRequest; + + const request = invokeTauri("prewarm_file_index", { rootPath }) + .catch((error) => { + // Non-fatal — search will still work, just cold on first use. + log.warn("[FileSearch] Prewarm failed (non-fatal):", error); + return 0; + }) + .finally(() => { + if (prewarmRequests.get(rootPath) === request) { + prewarmRequests.delete(rootPath); + } }); - return count; - } catch (error) { - // Non-fatal — search will still work, just cold on first use. - log.warn("[FileSearch] Prewarm failed (non-fatal):", error); - return 0; - } + + prewarmRequests.set(rootPath, request); + return request; +} + +/** Hidden windows do not spend CPU pre-walking projects. */ +export function shouldPrewarmFileIndex( + visibilityState: DocumentVisibilityState | undefined +): boolean { + return visibilityState !== "hidden"; } /** @@ -168,6 +184,23 @@ export async function clearFileIndexCache(): Promise { } } +/** + * Mark one workspace's file-path index stale without starting a scan. + * The next foreground prewarm or search rebuilds it on demand. + */ +export async function invalidateFileIndexCache( + rootPath: string +): Promise { + if (!isTauriReady()) return; + + try { + await invokeTauri("invalidate_file_index_cache", { rootPath }); + } catch (error) { + log.error("[FileSearch] Failed to invalidate cache:", error); + throw error; + } +} + // ============================================ // Helper Functions // ============================================