fix: skill discovery in subdirectories

app/src/ai/skills/file_watchers/utils.rs

@@ -17,32 +17,71 @@ use crate::warp_managed_paths_watcher::warp_managed_skill_dirs;
 /// Finds all skill directories in a repository by querying the RepoMetadataModel tree.
 ///
 /// Returns a list of paths to skill directories (e.g., `/repo/.agents/skills/`, `/repo/sub/.claude/skills/`).
+///
+/// Two passes are used to handle gitignored provider directories:
+///
+/// **Pass 1 — loaded skill dirs:** Standard tree traversal collecting directories that
+/// end with a known provider skills path (e.g. `.agents/skills`). Gitignored directories
+/// are skipped here because they are lazy-loaded with empty children in the tree.
+///
+/// **Pass 2 — lazy-loaded directories:** Traversal with `include_ignored: true` to find
+/// directories that are lazy-loaded (`loaded: false`). Two sub-cases are handled:
+///
+/// - **Case (a) — provider root is lazy:** The lazy dir is named like a provider root
+///   (`.agents`, `.claude`, …), e.g. `sub-project/.agents/` is gitignored. A single
+///   `is_dir()` check is performed for `{provider_dir}/skills`.
+///
+/// - **Case (b) — parent of provider root is lazy:** The lazy dir is not a provider
+///   root but could be a parent of one, e.g. `sub-project/` is gitignored so `.agents/`
+///   is never in the tree at all. For each known provider, `{dir}/{provider_path}` is
+///   checked with `is_dir()`.
+///
+/// In both cases only directories already registered in the tree are examined, keeping
+/// the scope bounded and preventing accidental traversal of dependency subtrees: when
+/// `node_modules/` is itself lazy-loaded its children never appear in the tree, so they
+/// cannot be reached by this pass.
 pub fn find_skill_directories_in_tree(
     repo_path: &Path,
     repo_metadata: &RepoMetadataModel,
     ctx: &AppContext,
 ) -> Vec<PathBuf> {
-    // Collect provider skills paths (e.g., ".agents/skills", ".claude/skills")
-    let skill_path_suffixes: Vec<&Path> = SKILL_PROVIDER_DEFINITIONS
+    let Some(id) = repo_metadata::RepositoryIdentifier::try_local(repo_path) else {
+        return Vec::new();
+    };
+
+    // Collect provider skills paths (e.g. ".agents/skills", ".claude/skills") and the
+    // corresponding provider root names (e.g. ".agents", ".claude") for the second pass.
+    let skill_path_suffixes: Vec<String> = SKILL_PROVIDER_DEFINITIONS
+        .iter()
+        .map(|p| p.skills_path.to_string_lossy().into_owned())
+        .collect();
+
+    let provider_root_names: HashSet<String> = SKILL_PROVIDER_DEFINITIONS
         .iter()
-        .map(|p| p.skills_path.as_path())
+        .filter_map(|p| {
+            p.skills_path
+                .parent()
+                .and_then(Path::file_name)
+                .and_then(|n| n.to_str())
+                .map(str::to_owned)
+        })
         .collect();
 
+    // ── Pass 1: find fully-loaded skill directories ───────────────────────────
+    //
     // 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 suffixes_1 = skill_path_suffixes.clone();
     let args = GetContentsArgs::default().with_filter(move |content| {
         let RepoContent::Directory(dir) = content else {
             return false;
         };
-        skill_path_suffixes
+        suffixes_1
             .iter()
-            .any(|suffix| dir.path.ends_with(&suffix.to_string_lossy()))
+            .any(|suffix| dir.path.ends_with(suffix.as_str()))
     });
 
-    let Some(id) = repo_metadata::RepositoryIdentifier::try_local(repo_path) else {
-        return Vec::new();
-    };
-    repo_metadata
+    let mut result: Vec<PathBuf> = repo_metadata
         .get_repo_contents(&id, args, ctx)
         .unwrap_or_default()
         .into_iter()
@@ -52,7 +91,69 @@ pub fn find_skill_directories_in_tree(
             RepoContent::Directory(dir) => dir.path.to_local_path_lossy(),
             RepoContent::File(f) => f.path.to_local_path_lossy(),
         })
-        .collect()
+        .collect();
+
+    // ── Pass 2: check lazy-loaded directories ────────────────────────────────
+    //
+    // Gitignored directories appear in the tree with `loaded: false` and no
+    // children. Two sub-cases are handled with targeted `is_dir()` probes:
+    //
+    //   Case (a) — provider root is lazy (e.g. `sub-project/.agents/` is
+    //   gitignored): probe `{dir}/skills`.
+    //
+    //   Case (b) — parent of provider root is lazy (e.g. `sub-project/` is
+    //   gitignored, so `.agents/` is never in the tree at all): probe
+    //   `{dir}/{provider_path}` for every known provider.
+    //
+    // Only directories already registered in the tree are examined, keeping
+    // the scope bounded. Dependency subtrees like `node_modules` are safe:
+    // when their parent is lazy-loaded their children are absent from the
+    // tree, so Pass 2 can never reach a `node_modules/.agents/` entry.
+    let mut result_set: HashSet<PathBuf> = result.iter().cloned().collect();
+    let args_lazy = GetContentsArgs::default()
+        .include_ignored()
+        .with_filter(move |content| {
+            let RepoContent::Directory(dir) = content else {
+                return false;
+            };
+            !dir.loaded
+        });
+
+    let lazy_dirs: Vec<PathBuf> = repo_metadata
+        .get_repo_contents(&id, args_lazy, ctx)
+        .unwrap_or_default()
+        .into_iter()
+        .map(|content| match content {
+            RepoContent::Directory(dir) => dir.path.to_local_path_lossy(),
+            RepoContent::File(f) => f.path.to_local_path_lossy(),
+        })
+        .collect();
+
+    for lazy_dir in lazy_dirs {
+        let dir_name = lazy_dir.file_name().and_then(|n| n.to_str()).unwrap_or("");
+        if provider_root_names.contains(dir_name) {
+            // Case (a): the lazy dir is itself a provider root (e.g. `.agents/`).
+            // Probe one level deeper for the `skills` subdirectory.
+            let skills_path = lazy_dir.join("skills");
+            if !result_set.contains(&skills_path) && skills_path.is_dir() {
+                result_set.insert(skills_path.clone());
+                result.push(skills_path);
+            }
+        } else {
+            // Case (b): the lazy dir is a parent of a potential provider root
+            // (e.g. `sub-project/` is gitignored, so `.agents/` was never
+            // indexed). Probe `{dir}/{provider_path}` for every known provider.
+            for provider in SKILL_PROVIDER_DEFINITIONS.iter() {
+                let skills_path = lazy_dir.join(&provider.skills_path);
+                if !result_set.contains(&skills_path) && skills_path.is_dir() {
+                    result_set.insert(skills_path.clone());
+                    result.push(skills_path);
+                }
+            }
+        }
+    }
+
+    result
 }
 
 /// Reads all skills from the given skill directories.

app/src/ai/skills/file_watchers/utils_tests.rs

@@ -724,3 +724,166 @@ fn find_skill_directories_in_tree_empty_repo() {
         });
     });
 }
+
+/// Regression test for warpdotdev/warp#9486 — provider root is lazy-loaded.
+///
+/// Skills inside a gitignored `.agents/` directory were not discovered because
+/// the indexed file tree represents gitignored directories as lazy-loaded entries
+/// with empty children.  The fix performs a targeted `is_dir()` check for
+/// `{lazy_provider_dir}/skills` on disk for every unloaded provider directory
+/// found in the tree, without a broad filesystem walk.
+///
+/// This test constructs a tree where `sub-project/.agents/` is present but
+/// `loaded: false` (as it would be when gitignored), while the real
+/// `sub-project/.agents/skills/` directory exists on disk.
+#[test]
+fn find_skill_directories_in_tree_finds_skills_in_lazy_loaded_provider_dir() {
+    VirtualFS::test("find_lazy_loaded_skills", |dirs, mut vfs| {
+        let repo = dirs.tests().join("repo");
+
+        // Create the actual filesystem structure: sub-project/.agents/skills/sub-skill/SKILL.md
+        vfs.mkdir("repo/sub-project/.agents/skills/sub-skill")
+            .with_files(vec![Stub::FileWithContent(
+                "repo/sub-project/.agents/skills/sub-skill/SKILL.md",
+                "---\nname: sub-skill\ndescription: test\n---\n# sub-skill",
+            )]);
+
+        // Build a tree that mirrors what the indexer produces when .agents/ is gitignored:
+        // sub-project/ is fully loaded, but .agents/ inside it is lazy-loaded (loaded: false,
+        // children: []) because it is gitignored.  The skills/ subdirectory is absent from
+        // the tree, which is exactly the scenario that caused the original bug.
+        let agents_dir = Entry::Directory(DirectoryEntry {
+            path: warp_util::standardized_path::StandardizedPath::try_from_local(
+                &repo.join("sub-project/.agents"),
+            )
+            .unwrap(),
+            children: vec![], // lazy-loaded: children not indexed
+            ignored: true,
+            loaded: false,
+        });
+        let sub_project = Entry::Directory(DirectoryEntry {
+            path: warp_util::standardized_path::StandardizedPath::try_from_local(
+                &repo.join("sub-project"),
+            )
+            .unwrap(),
+            children: vec![agents_dir],
+            ignored: false,
+            loaded: true,
+        });
+        let root = Entry::Directory(DirectoryEntry {
+            path: warp_util::standardized_path::StandardizedPath::try_from_local(&repo).unwrap(),
+            children: vec![sub_project],
+            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(
+                    warp_util::standardized_path::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 =
+                    warp_util::standardized_path::StandardizedPath::from_local_canonicalized(&repo)
+                        .unwrap();
+                model.insert_test_state(key, state, ctx);
+            });
+
+            model_handle.read(&app, |model, ctx| {
+                let skill_dirs = find_skill_directories_in_tree(&repo, model, ctx);
+                assert_eq!(skill_dirs.len(), 1);
+                assert!(skill_dirs.contains(&repo.join("sub-project/.agents/skills")));
+
+                let skills = read_skills_from_directories(skill_dirs);
+                assert_eq!(skills.len(), 1);
+                assert_eq!(skills[0].name, "sub-skill");
+            });
+        });
+    });
+}
+
+/// Regression test for warpdotdev/warp#9486 — parent of provider root is lazy-loaded.
+///
+/// When a subdirectory like `sub-project/` is itself gitignored, the tree contains
+/// it as a lazy-loaded entry with no children.  As a result, `.agents/` is never
+/// indexed at all, so Pass 2 (which previously only searched for lazy-loaded
+/// provider-named directories) could not find it.
+///
+/// The fix expands Pass 2 to also probe `{lazy_dir}/{provider_path}` for every
+/// known provider when the lazy-loaded directory is not itself a provider root.
+///
+/// This test constructs a tree where `sub-project/` has `loaded: false` (no
+/// children), while the real `sub-project/.agents/skills/` directory exists on disk.
+#[test]
+fn find_skill_directories_in_tree_finds_skills_when_parent_dir_is_lazy_loaded() {
+    VirtualFS::test("find_lazy_parent_skills", |dirs, mut vfs| {
+        let repo = dirs.tests().join("repo");
+
+        // Create the actual filesystem structure: sub-project/.agents/skills/sub-skill/SKILL.md
+        vfs.mkdir("repo/sub-project/.agents/skills/sub-skill")
+            .with_files(vec![Stub::FileWithContent(
+                "repo/sub-project/.agents/skills/sub-skill/SKILL.md",
+                "---\nname: sub-skill\ndescription: test\n---\n# sub-skill",
+            )]);
+
+        // Build a tree that mirrors what the indexer produces when `sub-project/` is
+        // gitignored: `sub-project/` appears with `loaded: false` and no children,
+        // so `.agents/` is entirely absent from the tree.
+        let sub_project = Entry::Directory(DirectoryEntry {
+            path: warp_util::standardized_path::StandardizedPath::try_from_local(
+                &repo.join("sub-project"),
+            )
+            .unwrap(),
+            children: vec![], // lazy-loaded: children not indexed
+            ignored: true,
+            loaded: false,
+        });
+        let root = Entry::Directory(DirectoryEntry {
+            path: warp_util::standardized_path::StandardizedPath::try_from_local(&repo).unwrap(),
+            children: vec![sub_project],
+            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(
+                    warp_util::standardized_path::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 =
+                    warp_util::standardized_path::StandardizedPath::from_local_canonicalized(&repo)
+                        .unwrap();
+                model.insert_test_state(key, state, ctx);
+            });
+
+            model_handle.read(&app, |model, ctx| {
+                let skill_dirs = find_skill_directories_in_tree(&repo, model, ctx);
+                assert_eq!(skill_dirs.len(), 1);
+                assert!(skill_dirs.contains(&repo.join("sub-project/.agents/skills")));
+
+                let skills = read_skills_from_directories(skill_dirs);
+                assert_eq!(skills.len(), 1);
+                assert_eq!(skills[0].name, "sub-skill");
+            });
+        });
+    });
+}