diff --git a/app/src/ai/skills/file_watchers/skill_watcher.rs b/app/src/ai/skills/file_watchers/skill_watcher.rs index 3cb071bbf2..9e095e5380 100644 --- a/app/src/ai/skills/file_watchers/skill_watcher.rs +++ b/app/src/ai/skills/file_watchers/skill_watcher.rs @@ -5,10 +5,10 @@ use ai::skills::{ home_skills_path, parse_skill, ParsedSkill, SkillProvider, SKILL_PROVIDER_DEFINITIONS, }; use async_channel::Sender; -use chrono::{DateTime, Duration, Utc}; use repo_metadata::repositories::DetectedRepositories; use repo_metadata::repository::{Repository, SubscriberId}; -use repo_metadata::{DirectoryWatcher, RepoMetadataModel, RepositoryUpdate}; +use repo_metadata::{DirectoryWatcher, RepoMetadataModel, RepositoryIdentifier, RepositoryUpdate}; +use warp_util::local_or_remote_path::LocalOrRemotePath; use warpui::{AppContext, Entity, ModelContext, ModelHandle, SingletonEntity}; use watcher::{BulkFilesystemWatcherEvent, HomeDirectoryWatcher, HomeDirectoryWatcherEvent}; @@ -16,10 +16,10 @@ use super::subscribers::{ HomeSkillSubscriber, ProjectSkillSubscriber, SkillRepositoryMessage, SymlinkSkillSubscriber, }; use super::utils::{ - find_skill_directories_in_tree, is_home_provider_path, is_home_skill_directory, is_skill_file, - read_skills_from_directories, + find_skill_files_in_tree, find_symlinked_skill_files_in_tree, is_home_provider_path, + is_home_skill_directory, is_skill_file, read_local_project_skills_from_filesystem, + read_skills_from_directories, read_skills_from_files, }; -use crate::server::datetime_ext::DateTimeExt; use crate::warp_managed_paths_watcher::{ filter_repository_update_by_prefix, warp_managed_skill_dirs, WarpManagedPathsWatcher, WarpManagedPathsWatcherEvent, @@ -30,22 +30,24 @@ pub enum SkillWatcherEvent { SkillsAdded { skills: Vec }, SkillsDeleted { paths: Vec }, } - -// When a new directory is detected by file watchers, we queue it to be scanned for skills later. -// These are processed when the file tree is updated. -// If a directory is left unprocessed for too long, we will drop it. -#[derive(Clone)] -pub struct QueuedProjectDirectoryCreation { - pub path: PathBuf, - pub timestamp: DateTime, -} - pub struct SkillWatcher { // Channel for sending repository messages from subscribers. repository_message_tx: Sender, - /// Repos we've registered file watchers for (to prevent duplicate subscriptions). - watched_repos: HashSet, - queued_project_directory_creations: Vec, + /// Last known project skill files by repository. Project skill counts are small, + /// so repo metadata changes trigger a full refresh instead of a subtree diff. + project_skill_files_by_repo: HashMap>, + /// Latest full project-skill refresh generation by repository. Repo metadata refreshes + /// parse local files asynchronously, so results from superseded tree snapshots must + /// not re-add deleted skills or overwrite newer parsed content. + project_skill_refresh_generations: HashMap, + /// Allocates refresh generations that cannot be reused if a repository is removed + /// and subsequently re-added while an old task is still in flight. + next_project_skill_refresh_generation: u64, + /// Failed local repos still need the project file watcher path because + /// repo metadata indexing can fail for oversized repos. This replaces the + /// previous `watched_repos` set so we only subscribe when fallback is active + /// and can also clean up the subscriber on repo removal. + failed_local_project_watchers: HashMap, SubscriberId)>, watcher_event_tx: Sender, /// Tracks watchers on home provider directories (e.g. ~/.agents, ~/.claude) so they /// can be cleaned up when the directory is deleted. @@ -69,12 +71,21 @@ impl SkillWatcher { /// `SkillManager::handle_skills_added`. pub fn read_skills_for_repos(repo_paths: &[PathBuf], ctx: &AppContext) -> Vec { let repo_metadata = RepoMetadataModel::as_ref(ctx); - let skill_dirs: Vec = repo_paths + let skill_files: Vec = repo_paths .iter() - .flat_map(|repo_path| find_skill_directories_in_tree(repo_path, repo_metadata, ctx)) + .filter_map(|repo_path| RepositoryIdentifier::try_local(repo_path)) + .flat_map(|repo_id| { + let mut skill_files = find_skill_files_in_tree(&repo_id, repo_metadata, ctx); + skill_files.extend( + find_symlinked_skill_files_in_tree(&repo_id, repo_metadata, ctx) + .into_iter() + .map(LocalOrRemotePath::Local), + ); + skill_files + }) + .filter_map(|path| path.to_local_path().map(Path::to_path_buf)) .collect(); - - read_skills_from_directories(skill_dirs) + read_skills_from_files(skill_files) } pub fn new(ctx: &mut ModelContext, watcher_event_tx: Sender) -> Self { @@ -124,6 +135,12 @@ impl SkillWatcher { } // Subscribe to home directory skills via DirectoryWatcher. + // TODO: Migrate home/global skill watching onto RepoMetadataModel as well. + // Project skills have moved there first so local and remote project + // behavior share one path and avoid a separate local FileWatcher. Home + // provider directories and symlink target watches still use + // DirectoryWatcher/HomeDirectoryWatcher for now, but should eventually + // follow the same model for consistency. // // We watch each skills "parent directory" under the home directory (e.g., `~/.agents`, // `~/.claude`) rather than the entire home directory, to reduce watch overhead. @@ -154,37 +171,37 @@ impl SkillWatcher { } // RepositoryMetadataEvent::RepositoryUpdated fires after the file tree is - // built, so we can query it for skill directories. This covers both local - // project repos and environment repos registered via CloudEnvironmentPrep - // (which flow through DetectedRepositories -> DirectoryWatcher -> - // RepoMetadataModel). + // built, so we can query it for skill files. Project skill updates use + // RepoMetadataModel for both local and remote repos when available, while + // local repos fall back to a direct project watcher only if metadata + // indexing fails. ctx.subscribe_to_model(&RepoMetadataModel::handle(ctx), |me, event, ctx| { use repo_metadata::wrapper_model::RepoMetadataEvent; - use repo_metadata::RepositoryIdentifier; match event { - RepoMetadataEvent::RepositoryUpdated { - id: RepositoryIdentifier::Local(path), - } => { - if let Some(local_path) = path.to_local_path() { - me.watch_repo(local_path.clone(), ctx); - me.scan_repository_for_skills(&local_path, ctx); - } + RepoMetadataEvent::RepositoryUpdated { id } => { + me.refresh_project_skills_for_repo(id, ctx); } - RepoMetadataEvent::FileTreeEntryUpdated { .. } => { - me.handle_queued_project_directory_creations(ctx); + RepoMetadataEvent::FileTreeEntryUpdated { id } => { + me.refresh_project_skills_for_repo(id, ctx); } - RepoMetadataEvent::RepositoryUpdated { .. } - | RepoMetadataEvent::RepositoryRemoved { .. } - | RepoMetadataEvent::FileTreeUpdated { .. } - | RepoMetadataEvent::UpdatingRepositoryFailed { .. } + RepoMetadataEvent::RepositoryRemoved { id } => { + me.remove_project_skills_for_repo(id); + me.stop_failed_local_project_watcher(id, ctx); + } + RepoMetadataEvent::UpdatingRepositoryFailed { id } => { + me.fallback_to_local_project_watcher(id, ctx); + } + RepoMetadataEvent::FileTreeUpdated { .. } | RepoMetadataEvent::IncrementalUpdateReady { .. } => {} } }); Self { repository_message_tx, - watched_repos: HashSet::new(), - queued_project_directory_creations: Vec::new(), + project_skill_files_by_repo: HashMap::new(), + project_skill_refresh_generations: HashMap::new(), + next_project_skill_refresh_generation: 0, + failed_local_project_watchers: HashMap::new(), watcher_event_tx, home_provider_watchers, symlink_canonical_to_originals: HashMap::new(), @@ -192,47 +209,235 @@ impl SkillWatcher { } } - /// Register a project root path to watch for skill file changes. - fn watch_repo(&mut self, repo_path: PathBuf, ctx: &mut ModelContext) { - if self.watched_repos.contains(&repo_path) { + fn refresh_project_skills_for_repo( + &mut self, + repo_id: &RepositoryIdentifier, + ctx: &mut ModelContext, + ) { + let refresh_generation = self.advance_project_skill_refresh_generation(repo_id); + let current_skill_files: HashSet = { + let repo_metadata = RepoMetadataModel::as_ref(ctx); + let mut skill_files = find_skill_files_in_tree(repo_id, repo_metadata, ctx); + skill_files.extend( + find_symlinked_skill_files_in_tree(repo_id, repo_metadata, ctx) + .into_iter() + .map(LocalOrRemotePath::Local), + ); + skill_files.into_iter().collect() + }; + + let previous_skill_files = self + .project_skill_files_by_repo + .get(repo_id) + .cloned() + .unwrap_or_default(); + + let deleted_paths = previous_skill_files + .difference(¤t_skill_files) + .filter_map(|path| path.to_local_path().map(Path::to_path_buf)) + .collect::>(); + if !deleted_paths.is_empty() { + self.cleanup_symlink_watches(&deleted_paths); + let _ = self + .watcher_event_tx + .try_send(SkillWatcherEvent::SkillsDeleted { + paths: deleted_paths, + }); + } + + // Local hydration intentionally only parses local paths. Remote project + // skill discovery now comes from RepoMetadataModel, but hydrating/parsing + // remote content is coming in a subsequent PR with a remote-aware skill + // identity pipeline. + let skill_files = current_skill_files + .iter() + .filter_map(|path| path.to_local_path().map(Path::to_path_buf)) + .collect::>(); + + // Project skill counts are expected to be small, so repo metadata updates + // intentionally trigger a full refresh instead of attempting to diff the + // changed subtree. This keeps local and remote project-skill behavior on + // the same RepoMetadataModel path and avoids a separate FileWatcher update + // path for local repos. + self.spawn_read_project_skills_from_files( + repo_id.clone(), + refresh_generation, + skill_files, + ctx, + ); + + self.project_skill_files_by_repo + .insert(repo_id.clone(), current_skill_files); + } + + fn advance_project_skill_refresh_generation(&mut self, repo_id: &RepositoryIdentifier) -> u64 { + self.next_project_skill_refresh_generation += 1; + self.project_skill_refresh_generations + .insert(repo_id.clone(), self.next_project_skill_refresh_generation); + self.next_project_skill_refresh_generation + } + + fn fallback_to_local_project_watcher( + &mut self, + repo_id: &RepositoryIdentifier, + ctx: &mut ModelContext, + ) { + let RepositoryIdentifier::Local(repo_path) = repo_id else { + return; + }; + let Some(local_path) = repo_path.to_local_path() else { + return; + }; + + self.scan_local_project_skills_from_filesystem(&local_path, ctx); + self.watch_failed_local_project_repo(local_path, ctx); + } + + /// Register a failed local project root to watch for skill file changes. + fn watch_failed_local_project_repo( + &mut self, + repo_path: PathBuf, + ctx: &mut ModelContext, + ) { + if self.failed_local_project_watchers.contains_key(&repo_path) { return; } - // Get the repository handle from DetectedRepositories. - if let Some(repo_handle) = + let Some(repo_handle) = DetectedRepositories::as_ref(ctx).get_local_watched_repo_for_path(&repo_path, ctx) - { - // Optimistically add the repository to the set of watched repositories to prevent duplicate subscriptions - self.watched_repos.insert(repo_path.clone()); + else { + log::warn!( + "Could not start local project skill fallback watcher for {}; repo is not watched", + repo_path.display() + ); + return; + }; - let subscriber = Box::new(ProjectSkillSubscriber { - message_tx: self.repository_message_tx.clone(), - }); + let subscriber = Box::new(ProjectSkillSubscriber { + message_tx: self.repository_message_tx.clone(), + }); + let start = repo_handle.update(ctx, |repo, ctx| repo.start_watching(subscriber, ctx)); + let subscriber_id = start.subscriber_id; + self.failed_local_project_watchers + .insert(repo_path.clone(), (repo_handle.clone(), subscriber_id)); - let start = repo_handle.update(ctx, |repo, ctx| repo.start_watching(subscriber, ctx)); - ctx.spawn(start.registration_future, move |me, res, ctx| { - if let Err(err) = res { - log::warn!("Failed to start watching project skills directory: {err}"); - me.watched_repos.remove(&repo_path); + ctx.spawn(start.registration_future, move |me, res, ctx| { + if let Err(err) = res { + log::warn!( + "Failed to start local project skill fallback watcher for {}: {err}", + repo_path.display() + ); + if let Some((repo_handle, subscriber_id)) = + me.failed_local_project_watchers.remove(&repo_path) + { repo_handle.update(ctx, |repo, ctx| { - repo.stop_watching(start.subscriber_id, ctx) + repo.stop_watching(subscriber_id, ctx); }); } - }); + } + }); + } + + fn scan_local_project_skills_from_filesystem( + &mut self, + repo_path: &Path, + ctx: &mut ModelContext, + ) { + let repo_path = repo_path.to_path_buf(); + ctx.spawn( + async move { read_local_project_skills_from_filesystem(&repo_path) }, + move |me, skills, ctx| { + if !skills.is_empty() { + me.register_symlink_watches(&skills, ctx); + let _ = me + .watcher_event_tx + .try_send(SkillWatcherEvent::SkillsAdded { skills }); + } + }, + ); + } + + fn spawn_read_project_skills_from_files( + &mut self, + repo_id: RepositoryIdentifier, + refresh_generation: u64, + skill_files: impl IntoIterator, + ctx: &mut ModelContext, + ) { + let skill_files: Vec<_> = skill_files.into_iter().collect(); + if skill_files.is_empty() { + return; } + + ctx.spawn( + async move { read_skills_from_files(skill_files) }, + move |me, skills, ctx| { + me.emit_project_skills_if_current(&repo_id, refresh_generation, skills, ctx); + }, + ); } - /// Scans a repository for skills using the LocalRepoMetadataModel tree. - /// This is called when RepositoryMetadataEvent::RepositoryUpdated fires. - fn scan_repository_for_skills(&mut self, repo_path: &Path, ctx: &mut ModelContext) { - let repo_metadata = RepoMetadataModel::as_ref(ctx); + fn emit_project_skills_if_current( + &mut self, + repo_id: &RepositoryIdentifier, + refresh_generation: u64, + skills: Vec, + ctx: &mut ModelContext, + ) { + if self.project_skill_refresh_generations.get(repo_id) != Some(&refresh_generation) { + return; + } - // Find all skill directories in the tree - let skill_dirs = find_skill_directories_in_tree(repo_path, repo_metadata, ctx); - if skill_dirs.is_empty() { + if !skills.is_empty() { + self.register_symlink_watches(&skills, ctx); + let _ = self + .watcher_event_tx + .try_send(SkillWatcherEvent::SkillsAdded { skills }); + } + } + + fn stop_failed_local_project_watcher( + &mut self, + repo_id: &RepositoryIdentifier, + ctx: &mut ModelContext, + ) { + let RepositoryIdentifier::Local(repo_path) = repo_id else { return; + }; + let Some(local_path) = repo_path.to_local_path() else { + return; + }; + let Some((repo_handle, subscriber_id)) = + self.failed_local_project_watchers.remove(&local_path) + else { + return; + }; + + repo_handle.update(ctx, |repo, ctx| { + repo.stop_watching(subscriber_id, ctx); + }); + } + + fn remove_project_skills_for_repo(&mut self, repo_id: &RepositoryIdentifier) { + // Invalidate an in-flight full refresh before deleting its currently cached skills. + // New refreshes use globally increasing generations, so the entry can be dropped + // without colliding if the same repository is later re-added. + self.project_skill_refresh_generations.remove(repo_id); + let Some(skill_files) = self.project_skill_files_by_repo.remove(repo_id) else { + return; + }; + let deleted_paths = skill_files + .into_iter() + .filter_map(|path| path.to_local_path().map(Path::to_path_buf)) + .collect::>(); + if !deleted_paths.is_empty() { + self.cleanup_symlink_watches(&deleted_paths); + let _ = self + .watcher_event_tx + .try_send(SkillWatcherEvent::SkillsDeleted { + paths: deleted_paths, + }); } - Self::spawn_read_skills_from_directories(skill_dirs, ctx); } fn spawn_read_skills_from_directories( @@ -269,7 +474,10 @@ impl SkillWatcher { .watcher_event_tx .try_send(SkillWatcherEvent::SkillsAdded { skills }); } - SkillRepositoryMessage::RepositoryUpdate { update } => { + SkillRepositoryMessage::ProjectRepositoryUpdate { update } => { + self.handle_failed_local_project_update(&update, ctx); + } + SkillRepositoryMessage::HomeRepositoryUpdate { update } => { self.handle_repository_update(&update, ctx); } SkillRepositoryMessage::SymlinkTargetUpdate { update } => { @@ -278,12 +486,88 @@ impl SkillWatcher { } } + fn handle_failed_local_project_update( + &mut self, + update: &RepositoryUpdate, + ctx: &mut ModelContext, + ) { + let mut deleted_paths = Vec::new(); + + // Process deleted files + for target_file in &update.deleted { + deleted_paths.push(target_file.path.clone()); + } + + // Process moved files + for (to_target, from_target) in &update.moved { + deleted_paths.push(from_target.path.clone()); + self.handle_failed_local_project_added_or_modified_path(&to_target.path, ctx); + } + + // Process added or modified files + for target_file in update.added_or_modified() { + self.handle_failed_local_project_added_or_modified_path(&target_file.path, ctx); + } + + // Process deleted paths in a batch + if !deleted_paths.is_empty() { + self.cleanup_symlink_watches(&deleted_paths); + let _ = self + .watcher_event_tx + .try_send(SkillWatcherEvent::SkillsDeleted { + paths: deleted_paths, + }); + } + } + + fn handle_failed_local_project_added_or_modified_path( + &mut self, + path: &Path, + ctx: &mut ModelContext, + ) { + if is_skill_file(path) { + let skill_file_path = path.to_path_buf(); + ctx.spawn( + async move { parse_skill(&skill_file_path) }, + move |me, skill, ctx| { + if let Ok(skill) = skill { + me.register_symlink_watches(std::slice::from_ref(&skill), ctx); + let _ = me + .watcher_event_tx + .try_send(SkillWatcherEvent::SkillsAdded { + skills: vec![skill], + }); + } + }, + ); + } else if path.is_symlink() && path.is_dir() && path.join("SKILL.md").exists() { + let skill_file_path = path.join("SKILL.md"); + ctx.spawn( + async move { parse_skill(&skill_file_path) }, + move |me, skill, ctx| { + if let Ok(skill) = skill { + me.register_symlink_watches(std::slice::from_ref(&skill), ctx); + let _ = me + .watcher_event_tx + .try_send(SkillWatcherEvent::SkillsAdded { + skills: vec![skill], + }); + } + }, + ); + } else if path.is_dir() { + // The original local watcher deferred added directories until RepoMetadataModel + // incorporated them. This handler runs only after metadata indexing has failed, so + // scan added directories directly from disk instead of waiting for an update that + // may never arrive. + self.scan_local_project_skills_from_filesystem(path, ctx); + } + } fn handle_repository_update( &mut self, update: &RepositoryUpdate, ctx: &mut ModelContext, ) { - let mut queued_project_directories = HashSet::new(); let mut home_path_additions = HashSet::new(); let mut deleted_paths = Vec::new(); @@ -309,14 +593,7 @@ impl SkillWatcher { }); } } else { - let repo_path = self.get_watched_repo_path(&to_target.path); - if let Some(repo_path) = repo_path { - if to_target.path.is_dir() { - queued_project_directories.insert(repo_path); - } - } else { - home_path_additions.insert(to_target.path.clone()); - } + home_path_additions.insert(to_target.path.clone()); } } @@ -359,14 +636,7 @@ impl SkillWatcher { }, ); } else { - let repo_path = self.get_watched_repo_path(&target_file.path); - if let Some(repo_path) = repo_path { - if target_file.path.is_dir() { - queued_project_directories.insert(repo_path); - } - } else { - home_path_additions.insert(target_file.path.clone()); - } + home_path_additions.insert(target_file.path.clone()); } } @@ -412,87 +682,6 @@ impl SkillWatcher { paths: deleted_paths, }); } - - // Queue project directory creations for later processing since the file tree is not yet updated - self.queued_project_directory_creations - .extend(queued_project_directories.into_iter().map(|path| { - QueuedProjectDirectoryCreation { - path, - timestamp: DateTime::now().into(), - } - })); - } - - fn handle_queued_project_directory_creations(&mut self, ctx: &mut ModelContext) { - let mut queued_by_repo_path: HashMap> = - HashMap::new(); - - for queued_project_directory_creation in &self.queued_project_directory_creations { - let repo_path = self.get_watched_repo_path(&queued_project_directory_creation.path); - if let Some(repo_path) = repo_path { - queued_by_repo_path - .entry(repo_path) - .or_default() - .push(queued_project_directory_creation.clone()); - } - } - - let mut queued_project_directory_creations_to_requeue: Vec = - Vec::new(); - let mut skill_dirs_to_read: HashSet = HashSet::new(); - - for (repo_path, queued_project_directory_creations) in queued_by_repo_path { - // Find all skill directories in the repository - let repo_metadata = RepoMetadataModel::as_ref(ctx); - let skill_dirs = find_skill_directories_in_tree(&repo_path, repo_metadata, ctx); - if skill_dirs.is_empty() { - continue; - } - - for queued_project_directory_creation in queued_project_directory_creations { - let relevant_skill_dirs = skill_dirs - .iter() - .filter(|skill_dir| { - // If the skill_dir is the child of the new directory, we need to read it again - // E.g. new dir is /repo/frontend/feature and skill dir is /repo/frontend/feature/.agents/skills - // If the new directory is a child of the skill dir, we need to read it again - // E.g. skill_dir is /repo/frontend/.agents/skills and new dir is /repo/frontend/.agents/skills/skill-name - skill_dir.starts_with(&queued_project_directory_creation.path) - || queued_project_directory_creation - .path - .starts_with(skill_dir) - }) - .collect::>(); - - // If the file tree doesn't have the newly created directory, we should requeue it for when the file tree is updated again - if relevant_skill_dirs.is_empty() { - // If 10s after the initial directory creation, the file tree still doesn't have the directory, we will give up and not requeue it - let elapsed = DateTime::now() - .signed_duration_since(queued_project_directory_creation.timestamp); - if elapsed < Duration::seconds(10) { - queued_project_directory_creations_to_requeue - .push(queued_project_directory_creation.clone()); - } - } else { - skill_dirs_to_read.extend(relevant_skill_dirs.into_iter().cloned()); - } - } - } - - ctx.spawn( - async move { read_skills_from_directories(skill_dirs_to_read) }, - move |me, skills, ctx| { - if !skills.is_empty() { - me.register_symlink_watches(&skills, ctx); - let _ = me - .watcher_event_tx - .try_send(SkillWatcherEvent::SkillsAdded { skills }); - } - }, - ); - - // Requeue project directory creations that could not be processed immediately - self.queued_project_directory_creations = queued_project_directory_creations_to_requeue; } /// Cleans up symlink canonical→original mappings for deleted skill paths. @@ -671,14 +860,6 @@ impl SkillWatcher { } } - // Given a path, return the path of the watched repository, if any. - fn get_watched_repo_path(&self, path: &Path) -> Option { - self.watched_repos - .iter() - .find(|repo_path| path.starts_with(repo_path)) - .cloned() - } - /// Handle changes to top-level files in the home directory. /// For skills, these are newly created provider directories fn handle_home_files_changed( diff --git a/app/src/ai/skills/file_watchers/skill_watcher_tests.rs b/app/src/ai/skills/file_watchers/skill_watcher_tests.rs index bb87306c04..8cc999da9a 100644 --- a/app/src/ai/skills/file_watchers/skill_watcher_tests.rs +++ b/app/src/ai/skills/file_watchers/skill_watcher_tests.rs @@ -1,18 +1,31 @@ use std::collections::{HashMap, HashSet}; use std::fs; +use super::super::subscribers::SkillRepositoryMessage; +use super::SkillWatcher; +use crate::ai::skills::skill_manager::SkillWatcherEvent; use ai::skills::{ParsedSkill, SkillProvider, SkillScope}; +use repo_metadata::entry::{DirectoryEntry, Entry, FileMetadata}; +use repo_metadata::file_tree_store::FileTreeState; use repo_metadata::repositories::DetectedRepositories; -use repo_metadata::{DirectoryWatcher, RepoMetadataModel, RepositoryUpdate, TargetFile}; +use repo_metadata::{ + DirectoryWatcher, RepoMetadataModel, RepositoryIdentifier, RepositoryUpdate, TargetFile, +}; use tempfile::TempDir; use warp_util::standardized_path::StandardizedPath; use warpui::App; -use super::SkillWatcher; -use crate::ai::skills::skill_manager::SkillWatcherEvent; - /// Helper function for creating a single skill file fn create_skill_file(dir: &TempDir, name: &str, description: &str, content: &str) -> ParsedSkill { + create_skill_file_in_directory(dir.path(), name, description, content) +} + +fn create_skill_file_in_directory( + parent_dir: &std::path::Path, + name: &str, + description: &str, + content: &str, +) -> ParsedSkill { let skill_content = format!( r#"--- name: {} @@ -22,7 +35,7 @@ description: {} "#, name, description, content ); - let skills_path = dir.path().join(".agents").join("skills"); + let skills_path = parent_dir.join(".agents").join("skills"); let skill_dir_path = skills_path.join(name); let skill_file_path = skill_dir_path.join("SKILL.md"); @@ -82,6 +95,299 @@ fn test_handle_repository_update_single_skill_added() { }); } +#[test] +fn test_stale_project_skill_refresh_result_is_ignored() { + let (tx, rx) = async_channel::unbounded(); + + App::test((), |mut app| async move { + app.add_singleton_model(DirectoryWatcher::new_for_testing); + app.add_singleton_model(|_| DetectedRepositories::default()); + app.add_singleton_model(RepoMetadataModel::new); + let skill_watcher_handle = app.add_model(|ctx| SkillWatcher::new_for_testing(ctx, tx)); + + let temp_dir = TempDir::new().unwrap(); + let skill = create_skill_file(&temp_dir, "stale", "Stale skill", "Old content"); + let repo_id = RepositoryIdentifier::try_local(temp_dir.path()).unwrap(); + + skill_watcher_handle.update(&mut app, |skill_watcher, ctx| { + let stale_generation = skill_watcher.advance_project_skill_refresh_generation(&repo_id); + skill_watcher.advance_project_skill_refresh_generation(&repo_id); + skill_watcher.emit_project_skills_if_current( + &repo_id, + stale_generation, + vec![skill], + ctx, + ); + }); + + assert!(rx.try_recv().is_err()); + }); +} + +#[test] +fn test_removing_project_repo_invalidates_pending_refresh_result() { + let (tx, rx) = async_channel::unbounded(); + + App::test((), |mut app| async move { + app.add_singleton_model(DirectoryWatcher::new_for_testing); + app.add_singleton_model(|_| DetectedRepositories::default()); + app.add_singleton_model(RepoMetadataModel::new); + let skill_watcher_handle = app.add_model(|ctx| SkillWatcher::new_for_testing(ctx, tx)); + + let temp_dir = TempDir::new().unwrap(); + let skill = create_skill_file(&temp_dir, "removed", "Removed skill", "Old content"); + let repo_id = RepositoryIdentifier::try_local(temp_dir.path()).unwrap(); + + skill_watcher_handle.update(&mut app, |skill_watcher, ctx| { + let pending_generation = + skill_watcher.advance_project_skill_refresh_generation(&repo_id); + skill_watcher.remove_project_skills_for_repo(&repo_id); + skill_watcher.emit_project_skills_if_current( + &repo_id, + pending_generation, + vec![skill], + ctx, + ); + }); + + assert!(rx.try_recv().is_err()); + }); +} + +#[test] +#[cfg(unix)] +fn test_refresh_project_skills_for_repo_loads_symlinked_project_skill_directory() { + let (tx, rx) = async_channel::unbounded(); + + App::test((), |mut app| async move { + app.add_singleton_model(DirectoryWatcher::new_for_testing); + app.add_singleton_model(|_| DetectedRepositories::default()); + let repo_metadata_handle = app.add_singleton_model(RepoMetadataModel::new); + let skill_watcher_handle = app.add_model(|ctx| SkillWatcher::new_for_testing(ctx, tx)); + + let repo_dir = TempDir::new().unwrap(); + let target_dir = TempDir::new().unwrap(); + let target_skill = create_skill_file( + &target_dir, + "linked-skill", + "Linked skill", + "Linked content", + ); + let repo = repo_dir.path().to_path_buf(); + let symlink_parent = repo.join(".agents/skills"); + fs::create_dir_all(&symlink_parent).unwrap(); + let symlink_skill_dir = symlink_parent.join("linked-skill"); + std::os::unix::fs::symlink(target_skill.path.parent().unwrap(), &symlink_skill_dir) + .unwrap(); + + let mut expected_skill = target_skill; + expected_skill.path = symlink_skill_dir.join("SKILL.md"); + + let repo_id = RepositoryIdentifier::try_local(&repo).unwrap(); + let repo_key = StandardizedPath::try_from_local(&repo).unwrap(); + repo_metadata_handle.update(&mut app, |model, ctx| { + model.insert_test_state(repo_key, project_provider_state(&repo), ctx); + }); + + skill_watcher_handle.update(&mut app, |skill_watcher, ctx| { + skill_watcher.refresh_project_skills_for_repo(&repo_id, ctx); + }); + + assert_eq!( + rx.recv().await.unwrap(), + SkillWatcherEvent::SkillsAdded { + skills: vec![expected_skill] + } + ); + }); +} + +#[test] +fn test_refresh_project_skills_for_repo_uses_repo_metadata_without_fallback_watcher() { + let (tx, rx) = async_channel::unbounded(); + + App::test((), |mut app| async move { + app.add_singleton_model(DirectoryWatcher::new_for_testing); + app.add_singleton_model(|_| DetectedRepositories::default()); + let repo_metadata_handle = app.add_singleton_model(RepoMetadataModel::new); + let skill_watcher_handle = app.add_model(|ctx| SkillWatcher::new_for_testing(ctx, tx)); + + let temp_dir = TempDir::new().unwrap(); + let skill = create_skill_file(&temp_dir, "metadata-skill", "Metadata skill", "Content"); + let repo = temp_dir.path().to_path_buf(); + let repo_id = RepositoryIdentifier::try_local(&repo).unwrap(); + let repo_key = StandardizedPath::try_from_local(&repo).unwrap(); + + repo_metadata_handle.update(&mut app, |model, ctx| { + model.insert_test_state(repo_key, project_state(&repo, Some(&skill)), ctx); + }); + skill_watcher_handle.update(&mut app, |skill_watcher, ctx| { + skill_watcher.refresh_project_skills_for_repo(&repo_id, ctx); + assert!(skill_watcher.failed_local_project_watchers.is_empty()); + }); + + assert_eq!( + rx.recv().await.unwrap(), + SkillWatcherEvent::SkillsAdded { + skills: vec![skill] + } + ); + }); +} + +#[test] +fn test_local_project_fallback_scans_filesystem_when_repo_metadata_fails() { + let (tx, rx) = async_channel::unbounded(); + + App::test((), |mut app| async move { + app.add_singleton_model(DirectoryWatcher::new_for_testing); + app.add_singleton_model(|_| DetectedRepositories::default()); + app.add_singleton_model(RepoMetadataModel::new); + let skill_watcher_handle = app.add_model(|ctx| SkillWatcher::new_for_testing(ctx, tx)); + + let temp_dir = TempDir::new().unwrap(); + let repo = dunce::canonicalize(temp_dir.path()).unwrap(); + let root_skill = + create_skill_file_in_directory(&repo, "root-skill", "Root skill", "Root content"); + let subdir = repo.join("packages/frontend"); + let subdir_skill = + create_skill_file_in_directory(&subdir, "frontend-skill", "Frontend skill", "Content"); + + let repo_id = RepositoryIdentifier::try_local(&repo).unwrap(); + skill_watcher_handle.update(&mut app, |skill_watcher, ctx| { + skill_watcher.fallback_to_local_project_watcher(&repo_id, ctx); + assert!(skill_watcher.failed_local_project_watchers.is_empty()); + }); + + let SkillWatcherEvent::SkillsAdded { mut skills } = rx.recv().await.unwrap() else { + panic!("Expected SkillsAdded event"); + }; + skills.sort_by(|a, b| a.path.cmp(&b.path)); + let mut expected = vec![root_skill, subdir_skill]; + expected.sort_by(|a, b| a.path.cmp(&b.path)); + assert_eq!(skills, expected); + }); +} + +#[test] +#[cfg(unix)] +fn test_local_project_fallback_initial_scan_loads_symlinked_skill_directory() { + let (tx, rx) = async_channel::unbounded(); + + App::test((), |mut app| async move { + app.add_singleton_model(DirectoryWatcher::new_for_testing); + app.add_singleton_model(|_| DetectedRepositories::default()); + app.add_singleton_model(RepoMetadataModel::new); + let skill_watcher_handle = app.add_model(|ctx| SkillWatcher::new_for_testing(ctx, tx)); + + let repo_dir = TempDir::new().unwrap(); + let target_dir = TempDir::new().unwrap(); + let target_skill = create_skill_file( + &target_dir, + "fallback-linked-skill", + "Fallback linked skill", + "Linked content", + ); + let repo = dunce::canonicalize(repo_dir.path()).unwrap(); + let symlink_parent = repo.join(".agents/skills"); + fs::create_dir_all(&symlink_parent).unwrap(); + let symlink_skill_dir = symlink_parent.join("fallback-linked-skill"); + std::os::unix::fs::symlink(target_skill.path.parent().unwrap(), &symlink_skill_dir) + .unwrap(); + + let mut expected_skill = target_skill; + expected_skill.path = symlink_skill_dir.join("SKILL.md"); + + let repo_id = RepositoryIdentifier::try_local(&repo).unwrap(); + skill_watcher_handle.update(&mut app, |skill_watcher, ctx| { + skill_watcher.fallback_to_local_project_watcher(&repo_id, ctx); + }); + + assert_eq!( + rx.recv().await.unwrap(), + SkillWatcherEvent::SkillsAdded { + skills: vec![expected_skill] + } + ); + }); +} +#[test] +fn test_local_project_fallback_update_reuses_repository_update_handler() { + let (tx, rx) = async_channel::unbounded(); + + App::test((), |mut app| async move { + app.add_singleton_model(DirectoryWatcher::new_for_testing); + app.add_singleton_model(|_| DetectedRepositories::default()); + app.add_singleton_model(RepoMetadataModel::new); + let skill_watcher_handle = app.add_model(|ctx| SkillWatcher::new_for_testing(ctx, tx)); + + let temp_dir = TempDir::new().unwrap(); + let skill = create_skill_file(&temp_dir, "fallback-update", "Fallback update", "Content"); + let update = RepositoryUpdate { + added: HashSet::new(), + modified: HashSet::from([TargetFile::new(skill.path.clone(), false)]), + deleted: HashSet::new(), + moved: HashMap::new(), + commit_updated: false, + index_lock_detected: false, + remote_ref_updated: false, + }; + + skill_watcher_handle.update(&mut app, |skill_watcher, ctx| { + skill_watcher.handle_message( + SkillRepositoryMessage::ProjectRepositoryUpdate { update }, + ctx, + ); + }); + + assert_eq!( + rx.recv().await.unwrap(), + SkillWatcherEvent::SkillsAdded { + skills: vec![skill] + } + ); + }); +} + +#[test] +fn test_local_project_fallback_directory_addition_scans_filesystem() { + let (tx, rx) = async_channel::unbounded(); + + App::test((), |mut app| async move { + app.add_singleton_model(DirectoryWatcher::new_for_testing); + app.add_singleton_model(|_| DetectedRepositories::default()); + app.add_singleton_model(RepoMetadataModel::new); + let skill_watcher_handle = app.add_model(|ctx| SkillWatcher::new_for_testing(ctx, tx)); + + let temp_dir = TempDir::new().unwrap(); + let new_dir = temp_dir.path().join("packages/frontend"); + let skill = + create_skill_file_in_directory(&new_dir, "fallback-dir", "Fallback dir", "Content"); + let update = RepositoryUpdate { + added: HashSet::from([TargetFile::new(new_dir, false)]), + modified: HashSet::new(), + deleted: HashSet::new(), + moved: HashMap::new(), + commit_updated: false, + index_lock_detected: false, + remote_ref_updated: false, + }; + + skill_watcher_handle.update(&mut app, |skill_watcher, ctx| { + skill_watcher.handle_message( + SkillRepositoryMessage::ProjectRepositoryUpdate { update }, + ctx, + ); + }); + + assert_eq!( + rx.recv().await.unwrap(), + SkillWatcherEvent::SkillsAdded { + skills: vec![skill] + } + ); + }); +} #[test] fn test_handle_repository_update_skill_modified() { let (tx, rx) = async_channel::unbounded(); @@ -249,39 +555,21 @@ fn test_handle_repository_update_skill_moved() { } // ============================================================================ -// Tests for handle_repository_update - directory addition +// Tests for project skill refreshes // ============================================================================ -/// When a non-skill directory is added within a known repo, `handle_repository_update` should -/// queue the repo root in `queued_project_directory_creations` for a later skill scan. #[test] -fn test_handle_repository_update_non_skill_directory_added_queues_project_directory() { +fn test_handle_repository_update_non_skill_directory_added_does_not_emit_project_event() { + let (tx, rx) = async_channel::unbounded(); + App::test((), |mut app| async move { app.add_singleton_model(DirectoryWatcher::new_for_testing); - let detected_repos_handle = app.add_singleton_model(|_| DetectedRepositories::default()); + app.add_singleton_model(|_| DetectedRepositories::default()); app.add_singleton_model(RepoMetadataModel::new); - let (tx, _rx) = async_channel::unbounded(); let skill_watcher_handle = app.add_model(|ctx| SkillWatcher::new_for_testing(ctx, tx)); let temp_dir = TempDir::new().unwrap(); - let canonical_repo = StandardizedPath::from_local_canonicalized(temp_dir.path()).unwrap(); - - // Register the temp dir as a known repo root so get_root_for_path resolves it. - detected_repos_handle.update(&mut app, |repos, _| { - repos.insert_test_repo_root(canonical_repo.clone()); - }); - - // Seed watched_repos so get_watched_repo_path can resolve the temp dir to this root. - // Use the canonicalized path to match what CanonicalizedPath::try_from resolves on macOS - // (where /var is a symlink to /private/var). - skill_watcher_handle.update(&mut app, |watcher, _| { - watcher - .watched_repos - .insert(canonical_repo.to_local_path().unwrap()); - }); - - // The added path must exist on disk for CanonicalizedPath resolution. - let new_dir = canonical_repo.to_local_path().unwrap().join("new-feature"); + let new_dir = temp_dir.path().join("new-feature"); fs::create_dir_all(&new_dir).unwrap(); let update = RepositoryUpdate { @@ -298,105 +586,110 @@ fn test_handle_repository_update_non_skill_directory_added_queues_project_direct skill_watcher.handle_repository_update(&update, ctx); }); - // The repo root should be queued for a skill scan. - skill_watcher_handle.read(&app, |watcher, _| { - assert_eq!(watcher.queued_project_directory_creations.len(), 1); - assert_eq!( - watcher.queued_project_directory_creations[0].path, - canonical_repo.to_local_path().unwrap() - ); - }); + assert!(rx.try_recv().is_err()); }); } -/// A modified non-skill file in a known repo should NOT queue anything in -/// `queued_project_directory_creations`; only directory additions can introduce new skill files. -#[test] -fn test_handle_repository_update_non_skill_file_modified_in_repo_does_not_queue_project_directory() -{ - App::test((), |mut app| async move { - app.add_singleton_model(DirectoryWatcher::new_for_testing); - let detected_repos_handle = app.add_singleton_model(|_| DetectedRepositories::default()); - app.add_singleton_model(RepoMetadataModel::new); - let (tx, _rx) = async_channel::unbounded(); - let skill_watcher_handle = app.add_model(|ctx| SkillWatcher::new_for_testing(ctx, tx)); - - let temp_dir = TempDir::new().unwrap(); - let canonical_repo = StandardizedPath::from_local_canonicalized(temp_dir.path()).unwrap(); - - detected_repos_handle.update(&mut app, |repos, _| { - repos.insert_test_repo_root(canonical_repo.clone()); +fn project_state(repo: &std::path::Path, skill: Option<&ParsedSkill>) -> FileTreeState { + let children = if let Some(skill) = skill { + let skill_file = Entry::File(FileMetadata::new(skill.path.clone(), false)); + let skill_dir = Entry::Directory(DirectoryEntry { + path: StandardizedPath::try_from_local(skill.path.parent().unwrap()).unwrap(), + children: vec![skill_file], + ignored: false, + loaded: true, }); - - // Create the file on disk so CanonicalizedPath resolution succeeds. - let readme = temp_dir.path().join("README.md"); - fs::write(&readme, "# Project").unwrap(); - - let update = RepositoryUpdate { - added: HashSet::new(), - modified: HashSet::from([TargetFile::new(readme, false)]), - deleted: HashSet::new(), - moved: HashMap::new(), - commit_updated: false, - index_lock_detected: false, - remote_ref_updated: false, - }; - - skill_watcher_handle.update(&mut app, |skill_watcher, ctx| { - skill_watcher.handle_repository_update(&update, ctx); + let skills_dir = Entry::Directory(DirectoryEntry { + path: StandardizedPath::try_from_local(&repo.join(".agents/skills")).unwrap(), + children: vec![skill_dir], + ignored: false, + loaded: true, }); - - // Modifying a plain file must NOT queue a project directory scan. - skill_watcher_handle.read(&app, |watcher, _| { - assert_eq!(watcher.queued_project_directory_creations.len(), 0); + let agents_dir = Entry::Directory(DirectoryEntry { + path: StandardizedPath::try_from_local(&repo.join(".agents")).unwrap(), + children: vec![skills_dir], + ignored: false, + loaded: true, }); + vec![agents_dir] + } else { + Vec::new() + }; + + let root = Entry::Directory(DirectoryEntry { + path: StandardizedPath::try_from_local(repo).unwrap(), + children, + ignored: false, + loaded: true, + }); + FileTreeState::new(root, Vec::new(), None) +} + +#[cfg(unix)] +fn project_provider_state(repo: &std::path::Path) -> FileTreeState { + let skills_dir = Entry::Directory(DirectoryEntry { + path: StandardizedPath::try_from_local(&repo.join(".agents/skills")).unwrap(), + children: Vec::new(), + ignored: false, + loaded: true, + }); + let agents_dir = Entry::Directory(DirectoryEntry { + path: StandardizedPath::try_from_local(&repo.join(".agents")).unwrap(), + children: vec![skills_dir], + ignored: false, + loaded: true, + }); + let root = Entry::Directory(DirectoryEntry { + path: StandardizedPath::try_from_local(repo).unwrap(), + children: vec![agents_dir], + ignored: false, + loaded: true, }); + FileTreeState::new(root, Vec::new(), None) } -/// When a regular (non-skill) file is added within a known repo, `handle_repository_update` -/// should NOT queue anything in `queued_project_directory_creations` because only directory -/// additions may introduce new skill files. #[test] -fn test_handle_repository_update_non_skill_file_added_does_not_queue_project_directory() { +fn test_refresh_project_skills_for_repo_removes_missing_project_skill_paths() { + let (tx, rx) = async_channel::unbounded(); + App::test((), |mut app| async move { app.add_singleton_model(DirectoryWatcher::new_for_testing); - let detected_repos_handle = app.add_singleton_model(|_| DetectedRepositories::default()); - app.add_singleton_model(RepoMetadataModel::new); - let (tx, _rx) = async_channel::unbounded(); + app.add_singleton_model(|_| DetectedRepositories::default()); + let repo_metadata_handle = app.add_singleton_model(RepoMetadataModel::new); let skill_watcher_handle = app.add_model(|ctx| SkillWatcher::new_for_testing(ctx, tx)); let temp_dir = TempDir::new().unwrap(); - let canonical_repo = StandardizedPath::from_local_canonicalized(temp_dir.path()).unwrap(); + let skill = create_skill_file(&temp_dir, "test", "Test skill", "Test content"); + let repo = temp_dir.path().to_path_buf(); + let repo_id = RepositoryIdentifier::try_local(&repo).unwrap(); + let repo_key = StandardizedPath::try_from_local(&repo).unwrap(); - detected_repos_handle.update(&mut app, |repos, _| { - repos.insert_test_repo_root(canonical_repo.clone()); + repo_metadata_handle.update(&mut app, |model, ctx| { + model.insert_test_state(repo_key.clone(), project_state(&repo, Some(&skill)), ctx); + }); + skill_watcher_handle.update(&mut app, |skill_watcher, ctx| { + skill_watcher.refresh_project_skills_for_repo(&repo_id, ctx); }); - // Create a regular file (not a directory, not a skill file) on disk. - let readme = temp_dir.path().join("README.md"); - fs::write(&readme, "# Project").unwrap(); - - let update = RepositoryUpdate { - added: HashSet::from([TargetFile::new(readme, false)]), - modified: HashSet::new(), - deleted: HashSet::new(), - moved: HashMap::new(), - commit_updated: false, - index_lock_detected: false, - remote_ref_updated: false, - }; + assert_eq!( + rx.recv().await.unwrap(), + SkillWatcherEvent::SkillsAdded { + skills: vec![skill.clone()] + } + ); + repo_metadata_handle.update(&mut app, |model, ctx| { + model.insert_test_state(repo_key, project_state(&repo, None), ctx); + }); skill_watcher_handle.update(&mut app, |skill_watcher, ctx| { - skill_watcher.handle_repository_update(&update, ctx); + skill_watcher.refresh_project_skills_for_repo(&repo_id, ctx); }); - // A plain file being added must NOT queue a project directory scan. - skill_watcher_handle.read(&app, |watcher, _| { - assert_eq!(watcher.queued_project_directory_creations.len(), 0); - }); + assert_eq!( + rx.recv().await.unwrap(), + SkillWatcherEvent::SkillsDeleted { + paths: vec![skill.path] + } + ); }); } - -// ============================================================================ -// Tests for handle_queued_project_directory_creations -// ============================================================================ diff --git a/app/src/ai/skills/file_watchers/subscribers.rs b/app/src/ai/skills/file_watchers/subscribers.rs index 62f939f759..80f8d75460 100644 --- a/app/src/ai/skills/file_watchers/subscribers.rs +++ b/app/src/ai/skills/file_watchers/subscribers.rs @@ -11,8 +11,10 @@ use warpui::ModelContext; pub enum SkillRepositoryMessage { /// Initial scan of a home skills directory (e.g., `~/.agents`). HomeInitialScan { skills: Vec }, - /// Incremental file system updates from either a home provider directory or a project skills directory. - RepositoryUpdate { update: RepositoryUpdate }, + /// Incremental file system updates from a local project fallback watcher. + ProjectRepositoryUpdate { update: RepositoryUpdate }, + /// Incremental file system updates from a home provider directory. + HomeRepositoryUpdate { update: RepositoryUpdate }, /// File changes detected in a resolved symlink target directory. SymlinkTargetUpdate { update: RepositoryUpdate }, } @@ -28,9 +30,8 @@ impl RepositorySubscriber for ProjectSkillSubscriber { _repository: &Repository, _ctx: &mut ModelContext, ) -> Pin + Send + 'static>> { - // Initial skill scanning is handled via RepositoryMetadataEvent::RepositoryUpdated, - // which fires AFTER the file tree is built. This subscriber is only used for - // incremental file change updates via on_files_updated. + // Initial fallback scans are triggered directly when repo metadata indexing fails. + // This subscriber only keeps failed local repos hot-reloaded afterward. Box::pin(async {}) } @@ -45,7 +46,7 @@ impl RepositorySubscriber for ProjectSkillSubscriber { Box::pin(async move { let _ = tx - .send(SkillRepositoryMessage::RepositoryUpdate { update }) + .send(SkillRepositoryMessage::ProjectRepositoryUpdate { update }) .await; }) } @@ -132,7 +133,7 @@ impl RepositorySubscriber for HomeSkillSubscriber { Box::pin(async move { let _ = tx - .send(SkillRepositoryMessage::RepositoryUpdate { update }) + .send(SkillRepositoryMessage::HomeRepositoryUpdate { update }) .await; }) } diff --git a/app/src/ai/skills/file_watchers/utils.rs b/app/src/ai/skills/file_watchers/utils.rs index 3f92d98d92..5ba2514aad 100644 --- a/app/src/ai/skills/file_watchers/utils.rs +++ b/app/src/ai/skills/file_watchers/utils.rs @@ -3,57 +3,179 @@ use std::path::{Path, PathBuf}; use std::sync::LazyLock; use ai::skills::{ - home_skills_path, read_skills, ParsedSkill, SkillProvider, SKILL_PROVIDER_DEFINITIONS, + home_skills_path, parse_skill, read_skills, ParsedSkill, SkillProvider, + SKILL_PROVIDER_DEFINITIONS, }; use anyhow::Error; use regex::Regex; use repo_metadata::local_model::GetContentsArgs; -use repo_metadata::{RepoContent, RepoMetadataModel}; +use repo_metadata::{RepoContent, RepoMetadataModel, RepositoryIdentifier}; +use walkdir::{DirEntry, WalkDir}; +use warp_util::local_or_remote_path::LocalOrRemotePath; +use warp_util::remote_path::RemotePath; +use warp_util::standardized_path::StandardizedPath; use warpui::AppContext; use crate::warp_managed_paths_watcher::warp_managed_skill_dirs; -/// Finds all skill directories in a repository by querying the RepoMetadataModel tree. +fn local_or_remote_path_for_repo_path( + repo_id: &RepositoryIdentifier, + path: &StandardizedPath, +) -> LocalOrRemotePath { + match repo_id { + RepositoryIdentifier::Local(_) => LocalOrRemotePath::Local(path.to_local_path_lossy()), + RepositoryIdentifier::Remote(remote) => { + LocalOrRemotePath::Remote(RemotePath::new(remote.host_id.clone(), path.clone())) + } + } +} + +/// Finds all skill files in a repository by querying the RepoMetadataModel tree. +/// +/// Returns a list of paths to concrete `SKILL.md` files (e.g., +/// `/repo/.agents/skills/deploy/SKILL.md`, `/repo/sub/.claude/skills/build/SKILL.md`). +pub fn find_skill_files_in_tree( + repo_id: &RepositoryIdentifier, + repo_metadata: &RepoMetadataModel, + ctx: &AppContext, +) -> Vec { + // Filter during traversal: only collect concrete SKILL.md files that match a known provider + // path. This keeps project acquisition on repo metadata until local or remote file hydration. + let args = GetContentsArgs { + include_folders: false, + ..GetContentsArgs::default() + } + .include_ignored() + .with_filter(|content| { + let RepoContent::File(file) = content else { + return false; + }; + is_skill_file(&file.path.to_local_path_lossy()) + }); + repo_metadata + .get_repo_contents(repo_id, args, ctx) + .unwrap_or_default() + .into_iter() + // Only files should reach this iterator due to the GetContentsArgs::filter. + // Keep the Directory arm for exhaustive matching in case RepoContent grows new variants. + .filter_map(|content| match content { + RepoContent::File(file) => { + Some(local_or_remote_path_for_repo_path(repo_id, &file.path)) + } + RepoContent::Directory(_) => None, + }) + .collect() +} + +/// Reads local project skills by discovering provider directories on the filesystem. +/// +/// This is a local-only fallback for repositories whose repo metadata indexing fails. Successful +/// local and remote repos should use [`find_skill_files_in_tree`] so the normal metadata-backed +/// path remains shared. +pub(super) fn read_local_project_skills_from_filesystem(scan_root: &Path) -> Vec { + let direct_skill_file = scan_root.join("SKILL.md"); + if is_skill_file(&direct_skill_file) { + return read_skills_from_files([direct_skill_file]); + } + + read_skills_from_directories(find_local_provider_directories_on_filesystem(scan_root)) +} + +fn find_local_provider_directories_on_filesystem(scan_root: &Path) -> Vec { + let mut provider_dirs = Vec::new(); + let mut entries = WalkDir::new(scan_root).follow_links(false).into_iter(); + while let Some(entry) = entries.next() { + let Ok(entry) = entry else { + continue; + }; + if is_ignored_fallback_scan_entry(&entry) { + if entry.file_type().is_dir() { + entries.skip_current_dir(); + } + continue; + } + if entry.file_type().is_dir() && is_project_provider_path(entry.path()) { + provider_dirs.push(entry.into_path()); + entries.skip_current_dir(); + } + } + provider_dirs.sort(); + provider_dirs +} + +fn is_ignored_fallback_scan_entry(entry: &DirEntry) -> bool { + entry.file_name().to_str() == Some(".git") +} + +/// Finds symlinked skill directories under loaded local provider directories in a repository. /// -/// Returns a list of paths to skill directories (e.g., `/repo/.agents/skills/`, `/repo/sub/.claude/skills/`). -pub fn find_skill_directories_in_tree( - repo_path: &Path, +/// Repo metadata intentionally skips directory symlinks to avoid duplicate trees/cycles. Project +/// skill refreshes are still triggered by repo metadata, but local hydration supplements the tree +/// with `SKILL.md` files from symlinked skill directories so existing symlink handling is preserved. +pub(super) fn find_symlinked_skill_files_in_tree( + repo_id: &RepositoryIdentifier, repo_metadata: &RepoMetadataModel, ctx: &AppContext, ) -> Vec { - // Collect provider skills paths (e.g., ".agents/skills", ".claude/skills") - let skill_path_suffixes: Vec<&Path> = SKILL_PROVIDER_DEFINITIONS - .iter() - .map(|p| p.skills_path.as_path()) - .collect(); + if !matches!(repo_id, RepositoryIdentifier::Local(_)) { + return Vec::new(); + } + + let provider_dirs = find_local_provider_directories_in_tree(repo_id, repo_metadata, ctx); + provider_dirs + .into_iter() + .flat_map(|provider_dir| { + std::fs::read_dir(provider_dir) + .into_iter() + .flatten() + .filter_map(|entry| entry.ok()) + .filter_map(|entry| { + let skill_dir = entry.path(); + if skill_dir.is_symlink() && skill_dir.is_dir() { + let skill_file = skill_dir.join("SKILL.md"); + if skill_file.exists() { + return Some(skill_file); + } + } + None + }) + }) + .collect() +} - // Filter during traversal: only collect directories that end with a skill provider path. - // The filter rejects files and non-matching directories, avoiding intermediate allocations. - let args = GetContentsArgs::default().with_filter(move |content| { - let RepoContent::Directory(dir) = content else { +fn find_local_provider_directories_in_tree( + repo_id: &RepositoryIdentifier, + repo_metadata: &RepoMetadataModel, + ctx: &AppContext, +) -> Vec { + let args = GetContentsArgs { + include_folders: true, + ..GetContentsArgs::default() + } + .include_ignored() + .with_filter(|content| { + let RepoContent::Directory(directory) = content else { return false; }; - skill_path_suffixes - .iter() - .any(|suffix| dir.path.ends_with(&suffix.to_string_lossy())) + is_project_provider_path(&directory.path.to_local_path_lossy()) }); - let Some(id) = repo_metadata::RepositoryIdentifier::try_local(repo_path) else { - return Vec::new(); - }; repo_metadata - .get_repo_contents(&id, args, ctx) + .get_repo_contents(repo_id, args, ctx) .unwrap_or_default() .into_iter() - // Only directories should reach this iterator due to the GetContentsArgs::filter. - // Keep the File arm for exhaustive matching in case RepoContent grows new variants. - .map(|content| match content { - RepoContent::Directory(dir) => dir.path.to_local_path_lossy(), - RepoContent::File(f) => f.path.to_local_path_lossy(), + .filter_map(|content| match content { + RepoContent::Directory(directory) => directory.path.to_local_path(), + RepoContent::File(_) => None, }) .collect() } +fn is_project_provider_path(path: &Path) -> bool { + SKILL_PROVIDER_DEFINITIONS + .iter() + .any(|provider| path.ends_with(&provider.skills_path)) +} /// Reads all skills from the given skill directories. pub fn read_skills_from_directories( skill_dirs: impl IntoIterator, @@ -63,6 +185,13 @@ pub fn read_skills_from_directories( .flat_map(|dir| read_skills(&dir)) .collect() } +/// Reads all skills from the given concrete skill files. +pub fn read_skills_from_files(skill_files: impl IntoIterator) -> Vec { + skill_files + .into_iter() + .filter_map(|path| parse_skill(&path).ok()) + .collect() +} pub fn is_skill_file(path: &Path) -> bool { extract_skill_parent_directory(path).is_ok() diff --git a/app/src/ai/skills/file_watchers/utils_tests.rs b/app/src/ai/skills/file_watchers/utils_tests.rs index cfadedeb09..69aa20d0a4 100644 --- a/app/src/ai/skills/file_watchers/utils_tests.rs +++ b/app/src/ai/skills/file_watchers/utils_tests.rs @@ -1,13 +1,23 @@ use repo_metadata::entry::{DirectoryEntry, Entry, FileMetadata}; use repo_metadata::file_tree_store::FileTreeState; +use repo_metadata::file_tree_update::{ + DirectoryNodeMetadata, FileNodeMetadata, FileTreeEntryUpdate, RepoNodeMetadata, +}; use repo_metadata::repositories::DetectedRepositories; -use repo_metadata::{DirectoryWatcher, RepoMetadataModel}; +use repo_metadata::{ + DirectoryWatcher, RepoMetadataModel, RepoMetadataUpdate, RepositoryIdentifier, +}; +use std::path::Path; use virtual_fs::{Stub, VirtualFS}; +use warp_util::host_id::HostId; +use warp_util::local_or_remote_path::LocalOrRemotePath; +use warp_util::remote_path::RemotePath; +use warp_util::standardized_path::StandardizedPath; use warpui::App; use super::{ - extract_skill_parent_directory, find_skill_directories_in_tree, is_home_provider_path, - is_home_skill_directory, is_skill_file, read_skills_from_directories, + extract_skill_parent_directory, find_skill_files_in_tree, is_home_provider_path, + is_home_skill_directory, is_skill_file, read_skills_from_files, }; // ============================================================================ @@ -401,11 +411,11 @@ fn is_home_provider_path_false_for_partial_path() { } // ============================================================================ -// Tests for find_skill_directories_in_tree +// Tests for find_skill_files_in_tree // ============================================================================ #[test] -fn find_skill_directories_in_tree_finds_root_skills() { +fn find_skill_files_in_tree_finds_root_skills() { VirtualFS::test("find_root_skills", |dirs, mut vfs| { let repo = dirs.tests().join("repo"); @@ -515,12 +525,20 @@ fn find_skill_directories_in_tree_finds_root_skills() { }); model_handle.read(&app, |model, ctx| { - let skill_dirs = find_skill_directories_in_tree(&repo, model, ctx); - assert_eq!(skill_dirs.len(), 2); - assert!(skill_dirs.contains(&repo.join(".agents/skills"))); - assert!(skill_dirs.contains(&repo.join(".claude/skills"))); - - let skills = read_skills_from_directories(skill_dirs); + let repo_id = RepositoryIdentifier::try_local(&repo).unwrap(); + let skill_files = find_skill_files_in_tree(&repo_id, model, ctx); + assert_eq!(skill_files.len(), 2); + assert!(skill_files.contains(&LocalOrRemotePath::Local( + repo.join(".agents/skills/root-skill-1/SKILL.md") + ))); + assert!(skill_files.contains(&LocalOrRemotePath::Local( + repo.join(".claude/skills/root-skill-2/SKILL.md") + ))); + + let local_skill_files = skill_files + .into_iter() + .filter_map(|path| path.to_local_path().map(Path::to_path_buf)); + let skills = read_skills_from_files(local_skill_files); assert_eq!(skills.len(), 2); let names: Vec<&str> = skills.iter().map(|s| s.name.as_str()).collect(); assert!(names.contains(&"root-skill-1")); @@ -531,7 +549,7 @@ fn find_skill_directories_in_tree_finds_root_skills() { } #[test] -fn find_skill_directories_in_tree_finds_subdirectory_skills() { +fn find_skill_files_in_tree_finds_subdirectory_skills() { VirtualFS::test("find_subdir_skills", |dirs, mut vfs| { let repo = dirs.tests().join("repo"); @@ -659,12 +677,20 @@ fn find_skill_directories_in_tree_finds_subdirectory_skills() { }); model_handle.read(&app, |model, ctx| { - let skill_dirs = find_skill_directories_in_tree(&repo, model, ctx); - assert_eq!(skill_dirs.len(), 2); - assert!(skill_dirs.contains(&repo.join(".agents/skills"))); - assert!(skill_dirs.contains(&repo.join("packages/frontend/.agents/skills"))); - - let skills = read_skills_from_directories(skill_dirs); + let repo_id = RepositoryIdentifier::try_local(&repo).unwrap(); + let skill_files = find_skill_files_in_tree(&repo_id, model, ctx); + assert_eq!(skill_files.len(), 2); + assert!(skill_files.contains(&LocalOrRemotePath::Local( + repo.join(".agents/skills/root-skill/SKILL.md") + ))); + assert!(skill_files.contains(&LocalOrRemotePath::Local( + repo.join("packages/frontend/.agents/skills/frontend-skill/SKILL.md") + ))); + + let local_skill_files = skill_files + .into_iter() + .filter_map(|path| path.to_local_path().map(Path::to_path_buf)); + let skills = read_skills_from_files(local_skill_files); assert_eq!(skills.len(), 2); let names: Vec<&str> = skills.iter().map(|s| s.name.as_str()).collect(); assert!(names.contains(&"root-skill")); @@ -675,7 +701,136 @@ fn find_skill_directories_in_tree_finds_subdirectory_skills() { } #[test] -fn find_skill_directories_in_tree_empty_repo() { +fn find_skill_files_in_tree_returns_remote_skill_paths_for_remote_repos() { + App::test((), |mut app| async move { + app.add_singleton_model(|_| DetectedRepositories::default()); + let model_handle = app.add_singleton_model(RepoMetadataModel::new); + let host_id = HostId::new("test-host".to_string()); + let repo_path = StandardizedPath::try_new("/repo").unwrap(); + let skill_path = + StandardizedPath::try_new("/repo/.agents/skills/remote-skill/SKILL.md").unwrap(); + let repo_id = + RepositoryIdentifier::Remote(RemotePath::new(host_id.clone(), repo_path.clone())); + + let update = RepoMetadataUpdate { + repo_path: repo_path.clone(), + remove_entries: vec![], + update_entries: vec![FileTreeEntryUpdate { + parent_path_to_replace: repo_path.clone(), + subtree_metadata: vec![ + RepoNodeMetadata::Directory(DirectoryNodeMetadata { + path: StandardizedPath::try_new("/repo/.agents").unwrap(), + ignored: false, + loaded: true, + }), + RepoNodeMetadata::Directory(DirectoryNodeMetadata { + path: StandardizedPath::try_new("/repo/.agents/skills").unwrap(), + ignored: false, + loaded: true, + }), + RepoNodeMetadata::Directory(DirectoryNodeMetadata { + path: StandardizedPath::try_new("/repo/.agents/skills/remote-skill") + .unwrap(), + ignored: false, + loaded: true, + }), + RepoNodeMetadata::File(FileNodeMetadata { + path: skill_path.clone(), + extension: Some("md".to_string()), + ignored: false, + }), + ], + }], + }; + + model_handle.update(&mut app, |model, ctx| { + model.insert_remote_snapshot(host_id.clone(), &update, ctx); + }); + + model_handle.read(&app, |model, ctx| { + let skill_files = find_skill_files_in_tree(&repo_id, model, ctx); + assert_eq!( + skill_files, + vec![LocalOrRemotePath::Remote(RemotePath::new( + host_id, skill_path + ))] + ); + }); + }); +} + +#[test] +fn find_skill_files_in_tree_includes_ignored_skill_files() { + VirtualFS::test("find_skills_ignored", |dirs, mut vfs| { + let repo = dirs.tests().join("repo"); + vfs.mkdir("repo/.agents/skills/ignored-skill") + .with_files(vec![Stub::FileWithContent( + "repo/.agents/skills/ignored-skill/SKILL.md", + "name: ignored-skill", + )]); + + let skill_file = Entry::File(FileMetadata::new( + repo.join(".agents/skills/ignored-skill/SKILL.md"), + true, + )); + let skill_dir = Entry::Directory(DirectoryEntry { + path: StandardizedPath::try_from_local(&repo.join(".agents/skills/ignored-skill")) + .unwrap(), + children: vec![skill_file], + ignored: true, + loaded: true, + }); + let skills_dir = Entry::Directory(DirectoryEntry { + path: StandardizedPath::try_from_local(&repo.join(".agents/skills")).unwrap(), + children: vec![skill_dir], + ignored: true, + loaded: true, + }); + let agents_dir = Entry::Directory(DirectoryEntry { + path: StandardizedPath::try_from_local(&repo.join(".agents")).unwrap(), + children: vec![skills_dir], + ignored: true, + loaded: true, + }); + let root = Entry::Directory(DirectoryEntry { + path: StandardizedPath::try_from_local(&repo).unwrap(), + children: vec![agents_dir], + ignored: false, + loaded: true, + }); + + App::test((), |mut app| async move { + let watcher = app.add_singleton_model(DirectoryWatcher::new); + app.add_singleton_model(|_| DetectedRepositories::default()); + let repo_handle = watcher.update(&mut app, |w, ctx| { + w.add_directory( + StandardizedPath::from_local_canonicalized(&repo).unwrap(), + ctx, + ) + .unwrap() + }); + let state = FileTreeState::new(root, vec![], Some(repo_handle)); + + let model_handle = app.add_singleton_model(RepoMetadataModel::new); + model_handle.update(&mut app, |model, ctx| { + let key = StandardizedPath::from_local_canonicalized(&repo).unwrap(); + model.insert_test_state(key, state, ctx); + }); + + model_handle.read(&app, |model, ctx| { + let repo_id = RepositoryIdentifier::try_local(&repo).unwrap(); + assert_eq!( + find_skill_files_in_tree(&repo_id, model, ctx), + vec![LocalOrRemotePath::Local( + repo.join(".agents/skills/ignored-skill/SKILL.md") + )] + ); + }); + }); + }); +} +#[test] +fn find_skill_files_in_tree_empty_repo() { VirtualFS::test("find_skills_empty", |dirs, mut vfs| { let repo = dirs.tests().join("repo"); vfs.mkdir("repo/src"); @@ -716,8 +871,9 @@ fn find_skill_directories_in_tree_empty_repo() { }); model_handle.read(&app, |model, ctx| { - let skill_dirs = find_skill_directories_in_tree(&repo, model, ctx); - assert!(skill_dirs.is_empty()); + let repo_id = RepositoryIdentifier::try_local(&repo).unwrap(); + let skill_files = find_skill_files_in_tree(&repo_id, model, ctx); + assert!(skill_files.is_empty()); }); }); }); diff --git a/app/src/lib.rs b/app/src/lib.rs index ebc9502aba..995d4b2f67 100644 --- a/app/src/lib.rs +++ b/app/src/lib.rs @@ -1519,6 +1519,12 @@ pub(crate) fn initialize_app( } else { RepoMetadataModel::new(ctx) }; + model.register_ignored_path_interests( + ::ai::skills::SKILL_PROVIDER_DEFINITIONS + .iter() + .map(|provider| provider.skills_path.clone()), + ctx, + ); // Subscribe to RemoteServerManager push events so that remote repo // metadata snapshots and incremental updates populate the remote diff --git a/crates/repo_metadata/src/entry.rs b/crates/repo_metadata/src/entry.rs index b29cd61c8a..614794796d 100644 --- a/crates/repo_metadata/src/entry.rs +++ b/crates/repo_metadata/src/entry.rs @@ -53,6 +53,22 @@ pub enum Entry { File(FileMetadata), Directory(DirectoryEntry), } +#[derive(Clone, Copy)] +pub(crate) struct BuildTreeOptions<'a> { + pub max_depth: usize, + pub current_depth: usize, + pub ignored_path_strategy: &'a IgnoredPathStrategy, + pub ignored_path_interests: &'a [PathBuf], +} + +impl<'a> BuildTreeOptions<'a> { + fn child(self) -> Self { + Self { + current_depth: self.current_depth + 1, + ..self + } + } +} #[derive(Debug, Clone, Copy, Hash, Eq, PartialEq)] pub struct FileId(usize); @@ -101,14 +117,36 @@ impl Entry { current_depth: usize, ignored_path_strategy: &IgnoredPathStrategy, ) -> Result { - Self::build_tree_with_ignored_ancestor( + Self::build_tree_with_ignored_path_interests_and_ancestor( path, files, gitignores, remaining_file_quota, - max_depth, - current_depth, - ignored_path_strategy, + BuildTreeOptions { + max_depth, + current_depth, + ignored_path_strategy, + ignored_path_interests: &[], + }, + false, + ) + } + + /// Builds a tree of entries from a given path, loading ignored paths that match + /// one of the supplied component-sequence interests instead of leaving them lazy. + pub(crate) fn build_tree_with_ignored_path_interests( + path: impl Into, + files: &mut Vec, + gitignores: &mut Vec, + remaining_file_quota: Option<&mut usize>, + options: BuildTreeOptions<'_>, + ) -> Result { + Self::build_tree_with_ignored_path_interests_and_ancestor( + path, + files, + gitignores, + remaining_file_quota, + options, false, ) } @@ -118,11 +156,35 @@ impl Entry { path: impl Into, files: &mut Vec, gitignores: &mut Vec, - mut remaining_file_quota: Option<&mut usize>, + remaining_file_quota: Option<&mut usize>, max_depth: usize, current_depth: usize, ignored_path_strategy: &IgnoredPathStrategy, ancestor_is_ignored: bool, + ) -> Result { + Self::build_tree_with_ignored_path_interests_and_ancestor( + path, + files, + gitignores, + remaining_file_quota, + BuildTreeOptions { + max_depth, + current_depth, + ignored_path_strategy, + ignored_path_interests: &[], + }, + ancestor_is_ignored, + ) + } + + #[allow(clippy::too_many_arguments)] + pub(crate) fn build_tree_with_ignored_path_interests_and_ancestor( + path: impl Into, + files: &mut Vec, + gitignores: &mut Vec, + mut remaining_file_quota: Option<&mut usize>, + options: BuildTreeOptions<'_>, + ancestor_is_ignored: bool, ) -> Result { let curr_path: PathBuf = path.into(); let is_dir = curr_path.is_dir(); @@ -148,10 +210,10 @@ impl Entry { ); // If we've reached the max depth, force lazy-loading even of non-ignored folders. - let mut lazy_load = current_depth >= max_depth; + let mut lazy_load = options.current_depth >= options.max_depth; if path_is_ignored { - match ignored_path_strategy { + match options.ignored_path_strategy { IgnoredPathStrategy::Exclude => { return Err(BuildTreeError::Ignored); } @@ -163,7 +225,8 @@ impl Entry { } } IgnoredPathStrategy::IncludeLazy => { - lazy_load = true; + lazy_load = + !matches_ignored_path_interest(&curr_path, options.ignored_path_interests); } IgnoredPathStrategy::Include => {} } @@ -209,14 +272,12 @@ impl Entry { }; if let Some(canonical_path) = canonical_path { - match Entry::build_tree_with_ignored_ancestor( + match Entry::build_tree_with_ignored_path_interests_and_ancestor( canonical_path, files, gitignores, remaining_file_quota.as_deref_mut(), - max_depth, - current_depth + 1, - ignored_path_strategy, + options.child(), path_is_ignored, ) { Ok(entry) => Some(entry), @@ -361,6 +422,48 @@ pub fn is_git_internal_path(path: &Path) -> bool { }) } +fn matches_ignored_path_interest(path: &Path, ignored_path_interests: &[PathBuf]) -> bool { + let path_components: Vec<_> = path + .components() + .filter_map(|component| match component { + Component::Normal(name) => Some(name), + Component::Prefix(_) + | Component::RootDir + | Component::CurDir + | Component::ParentDir => None, + }) + .collect(); + + ignored_path_interests.iter().any(|interest| { + let interest_components: Vec<_> = interest + .components() + .filter_map(|component| match component { + Component::Normal(name) => Some(name), + Component::Prefix(_) + | Component::RootDir + | Component::CurDir + | Component::ParentDir => None, + }) + .collect(); + + if interest_components.is_empty() { + return false; + } + + if path_components + .windows(interest_components.len()) + .any(|window| window == interest_components.as_slice()) + { + return true; + } + + (1..interest_components.len()).any(|prefix_len| { + path_components.len() >= prefix_len + && path_components[path_components.len() - prefix_len..] + == interest_components[..prefix_len] + }) + }) +} /// Returns true if a path matches any of the gitignores. /// /// For example, if the directory `/target` is ignored: diff --git a/crates/repo_metadata/src/entry_tests.rs b/crates/repo_metadata/src/entry_tests.rs index 2aaf5e1a61..5d5c4b4761 100644 --- a/crates/repo_metadata/src/entry_tests.rs +++ b/crates/repo_metadata/src/entry_tests.rs @@ -173,6 +173,137 @@ fn test_git_path_filtering_allowlist() { } } +fn find_entry<'a>(entry: &'a super::Entry, path: &std::path::Path) -> Option<&'a super::Entry> { + let std_path = warp_util::standardized_path::StandardizedPath::try_from_local(path).ok()?; + if entry.path() == &std_path { + return Some(entry); + } + let super::Entry::Directory(directory) = entry else { + return None; + }; + directory + .children + .iter() + .find_map(|child| find_entry(child, path)) +} + +fn build_skill_tree_with_gitignore(root: &std::path::Path, gitignore: &str) -> super::Entry { + std::fs::write(root.join(".gitignore"), gitignore).unwrap(); + let mut files = Vec::new(); + let mut gitignores = Vec::new(); + let mut file_limit = 1000; + super::Entry::build_tree_with_ignored_path_interests( + root, + &mut files, + &mut gitignores, + Some(&mut file_limit), + super::BuildTreeOptions { + max_depth: 200, + current_depth: 0, + ignored_path_strategy: &super::IgnoredPathStrategy::IncludeLazy, + ignored_path_interests: &[std::path::PathBuf::from(".agents/skills")], + }, + ) + .unwrap() +} + +#[test] +fn ignored_skill_file_is_loaded_for_registered_provider_path() { + virtual_fs::VirtualFS::test("ignored_skill_file_loaded", |dirs, mut vfs| { + vfs.mkdir("repo/.agents/skills/test") + .with_files(vec![virtual_fs::Stub::FileWithContent( + "repo/.agents/skills/test/SKILL.md", + "name: test", + )]); + let repo = dirs.tests().join("repo"); + + let tree = build_skill_tree_with_gitignore(&repo, ".agents/skills/test/SKILL.md\n"); + let skill_file = find_entry(&tree, &repo.join(".agents/skills/test/SKILL.md")) + .expect("ignored skill file should be present"); + assert!(skill_file.ignored()); + }); +} + +#[test] +fn ignored_skill_directory_is_loaded_for_registered_provider_path() { + virtual_fs::VirtualFS::test("ignored_skill_dir_loaded", |dirs, mut vfs| { + vfs.mkdir("repo/.agents/skills/test") + .with_files(vec![virtual_fs::Stub::FileWithContent( + "repo/.agents/skills/test/SKILL.md", + "name: test", + )]); + let repo = dirs.tests().join("repo"); + + let tree = build_skill_tree_with_gitignore(&repo, ".agents/skills/test/\n"); + let skill_dir = find_entry(&tree, &repo.join(".agents/skills/test")) + .expect("ignored skill directory should be present"); + assert!(skill_dir.ignored()); + assert!(skill_dir.loaded()); + assert!(find_entry(&tree, &repo.join(".agents/skills/test/SKILL.md")).is_some()); + }); +} + +#[test] +fn ignored_agents_directory_is_loaded_for_registered_provider_path() { + virtual_fs::VirtualFS::test("ignored_agents_dir_loaded", |dirs, mut vfs| { + vfs.mkdir("repo/.agents/skills/test") + .with_files(vec![virtual_fs::Stub::FileWithContent( + "repo/.agents/skills/test/SKILL.md", + "name: test", + )]); + let repo = dirs.tests().join("repo"); + + let tree = build_skill_tree_with_gitignore(&repo, ".agents/\n"); + let agents_dir = find_entry(&tree, &repo.join(".agents")) + .expect("ignored .agents directory should be present"); + assert!(agents_dir.ignored()); + assert!(agents_dir.loaded()); + assert!(find_entry(&tree, &repo.join(".agents/skills/test/SKILL.md")).is_some()); + }); +} + +#[test] +fn ignored_agents_skills_directory_is_loaded_for_registered_provider_path() { + virtual_fs::VirtualFS::test("ignored_agents_skills_dir_loaded", |dirs, mut vfs| { + vfs.mkdir("repo/.agents/skills/test") + .with_files(vec![virtual_fs::Stub::FileWithContent( + "repo/.agents/skills/test/SKILL.md", + "name: test", + )]); + let repo = dirs.tests().join("repo"); + + let tree = build_skill_tree_with_gitignore(&repo, ".agents/skills/\n"); + let skills_dir = find_entry(&tree, &repo.join(".agents/skills")) + .expect("ignored .agents/skills directory should be present"); + assert!(skills_dir.ignored()); + assert!(skills_dir.loaded()); + assert!(find_entry(&tree, &repo.join(".agents/skills/test/SKILL.md")).is_some()); + }); +} + +#[test] +fn unrelated_ignored_directory_stays_lazy_without_registered_interest() { + virtual_fs::VirtualFS::test("unrelated_ignored_dir_lazy", |dirs, mut vfs| { + vfs.mkdir("repo/.agents/skills/test") + .mkdir("repo/target/debug") + .with_files(vec![ + virtual_fs::Stub::FileWithContent( + "repo/.agents/skills/test/SKILL.md", + "name: test", + ), + virtual_fs::Stub::FileWithContent("repo/target/debug/app", "binary"), + ]); + let repo = dirs.tests().join("repo"); + + let tree = build_skill_tree_with_gitignore(&repo, "target/\n"); + let target_dir = find_entry(&tree, &repo.join("target")) + .expect("ignored unrelated directory should be present as lazy"); + assert!(target_dir.ignored()); + assert!(!target_dir.loaded()); + assert!(find_entry(&tree, &repo.join("target/debug/app")).is_none()); + }); +} + #[test] fn build_tree_marks_descendants_of_ignored_directory_as_ignored() { let temp_dir = tempfile::tempdir().unwrap(); diff --git a/crates/repo_metadata/src/local_model.rs b/crates/repo_metadata/src/local_model.rs index 860955bcab..e727288a80 100644 --- a/crates/repo_metadata/src/local_model.rs +++ b/crates/repo_metadata/src/local_model.rs @@ -22,7 +22,7 @@ pub enum RepoContent<'a> { use warp_util::standardized_path::StandardizedPath; -use crate::entry::{BuildTreeError, Entry, FileId, IgnoredPathStrategy}; +use crate::entry::{BuildTreeError, BuildTreeOptions, Entry, FileId, IgnoredPathStrategy}; use crate::repository::Repository; use crate::telemetry::RepoMetadataTelemetryEvent; use crate::{gitignores_for_directory, matches_gitignores, RepoMetadataError}; @@ -143,6 +143,10 @@ pub struct LocalRepoMetadataModel { /// events after applying watcher mutations. Only the remote server /// variant enables this. emit_incremental_updates: bool, + /// Component-sequence paths that consumers need loaded even when ignored. + /// For example, a consumer can register `.foo/bar` so ignored `.foo`, + /// `.foo/bar`, and descendants of `.foo/bar` are loaded into the tree. + ignored_path_interests: Vec, } #[derive(Debug, Clone, Default)] @@ -227,6 +231,7 @@ impl LocalRepoMetadataModel { #[cfg(feature = "local_fs")] watcher: None, emit_incremental_updates: false, + ignored_path_interests: Vec::new(), }; cfg_if::cfg_if! { if #[cfg(feature = "local_fs")] { @@ -262,6 +267,26 @@ impl LocalRepoMetadataModel { self.emit_incremental_updates = enabled; } + /// Registers component-sequence paths that should be loaded even when ignored. + /// + /// This stays intentionally generic: consumers own the meaning of the paths, + /// while repo metadata only uses them to decide which ignored subtrees should + /// be represented eagerly instead of as lazy placeholders. + pub fn register_ignored_path_interests( + &mut self, + interests: impl IntoIterator, + ) { + for interest in interests { + if !self + .ignored_path_interests + .iter() + .any(|existing| existing == &interest) + { + self.ignored_path_interests.push(interest); + } + } + } + /// Handles events from the BulkFilesystemWatcher. #[cfg(feature = "local_fs")] fn handle_watcher_event( @@ -313,12 +338,14 @@ impl LocalRepoMetadataModel { if let Some(IndexedRepoState::Indexed(state)) = self.repositories.get_mut(&repo_path) { let repo_path_clone = repo_path.clone(); let gitignores_clone = state.gitignores.clone(); + let ignored_path_interests = self.ignored_path_interests.clone(); let lazy_load = self.lazy_loaded_paths.contains_key(&repo_path); ctx.spawn( async move { let mutations = Self::compute_file_tree_mutations( &repo_scoped_update, &gitignores_clone, + &ignored_path_interests, ) .await; (mutations, repo_path_clone, lazy_load) @@ -589,6 +616,7 @@ impl LocalRepoMetadataModel { async fn compute_file_tree_mutations( update: &RepoUpdate, gitignores: &[Gitignore], + ignored_path_interests: &[PathBuf], ) -> Vec { let mut mutations = Vec::new(); @@ -609,14 +637,17 @@ impl LocalRepoMetadataModel { let mut files = Vec::new(); let mut gitignores = gitignores.to_owned(); let mut file_limit = MAX_FILES_PER_REPO; - match Entry::build_tree_with_ignored_ancestor( + match Entry::build_tree_with_ignored_path_interests_and_ancestor( path_to_add, &mut files, &mut gitignores, Some(&mut file_limit), - MAX_TREE_DEPTH, - 0, - &IgnoredPathStrategy::IncludeLazy, + BuildTreeOptions { + max_depth: MAX_TREE_DEPTH, + current_depth: 0, + ignored_path_strategy: &IgnoredPathStrategy::IncludeLazy, + ignored_path_interests, + }, is_ignored, ) { Ok(subtree) => { @@ -897,6 +928,7 @@ impl LocalRepoMetadataModel { // Build the complete file tree for the repository asynchronously let repo_path_for_build = local_path; let gitignores_for_build = gitignores.clone(); + let ignored_path_interests = self.ignored_path_interests.clone(); let repo_path_str_for_log = std_path.to_string(); let std_path_for_completion = std_path; let repository_handle_for_completion = repository_handle.clone(); @@ -912,14 +944,17 @@ impl LocalRepoMetadataModel { let mut file_limit = MAX_FILES_PER_REPO; - let mut build_result = Entry::build_tree( + let mut build_result = Entry::build_tree_with_ignored_path_interests( &repo_path_for_build, &mut files, &mut gitignores_for_build, Some(&mut file_limit), - MAX_TREE_DEPTH, // max_depth - 0, // current_depth - &IgnoredPathStrategy::IncludeLazy, + BuildTreeOptions { + max_depth: MAX_TREE_DEPTH, + current_depth: 0, + ignored_path_strategy: &IgnoredPathStrategy::IncludeLazy, + ignored_path_interests: &ignored_path_interests, + }, ); // Repos with more than MAX_FILES_PER_REPO tracked files can't @@ -934,14 +969,17 @@ impl LocalRepoMetadataModel { if matches!(build_result, Err(BuildTreeError::ExceededMaxFileLimit)) { files.clear(); gitignores_for_build = initial_gitignores; - build_result = Entry::build_tree( + build_result = Entry::build_tree_with_ignored_path_interests( &repo_path_for_build, &mut files, &mut gitignores_for_build, None, - 1, // max_depth — only first level - 0, - &IgnoredPathStrategy::IncludeLazy, + BuildTreeOptions { + max_depth: 1, // Only first level. + current_depth: 0, + ignored_path_strategy: &IgnoredPathStrategy::IncludeLazy, + ignored_path_interests: &ignored_path_interests, + }, ); if build_result.is_ok() { indexed_with_limit = true; diff --git a/crates/repo_metadata/src/local_model_tests.rs b/crates/repo_metadata/src/local_model_tests.rs index e2b41a3775..67f2c31014 100644 --- a/crates/repo_metadata/src/local_model_tests.rs +++ b/crates/repo_metadata/src/local_model_tests.rs @@ -36,6 +36,7 @@ mod tests { #[cfg(feature = "local_fs")] watcher: Default::default(), emit_incremental_updates: false, + ignored_path_interests: Vec::new(), } } } @@ -743,6 +744,7 @@ mod tests { let mutations = block_on(LocalRepoMetadataModel::compute_file_tree_mutations( &update, &gitignores, + &[], )); LocalRepoMetadataModel::apply_file_tree_mutations(&mut root, mutations, false, false); diff --git a/crates/repo_metadata/src/wrapper_model.rs b/crates/repo_metadata/src/wrapper_model.rs index 79535ae909..1db15a3ab8 100644 --- a/crates/repo_metadata/src/wrapper_model.rs +++ b/crates/repo_metadata/src/wrapper_model.rs @@ -295,6 +295,22 @@ impl RepoMetadataModel { }) } + /// Registers component-sequence paths that should be loaded even when ignored. + /// + /// This delegates to the local model because ignored-path matching happens + /// while building local file trees. Remote repositories receive the resulting + /// file-tree metadata over the existing remote sync protocol. + pub fn register_ignored_path_interests( + &self, + interests: impl IntoIterator, + ctx: &mut ModelContext, + ) { + let interests: Vec<_> = interests.into_iter().collect(); + self.local.update(ctx, |local, _| { + local.register_ignored_path_interests(interests); + }); + } + /// Removes a lazily-loaded local standalone path from tracking. #[cfg(feature = "local_fs")] pub fn remove_lazy_loaded_path(&self, path: &StandardizedPath, ctx: &mut ModelContext) {