diff --git a/python/packages/core/agent_framework/_skills.py b/python/packages/core/agent_framework/_skills.py index 9868c86d40..782d38b3ce 100644 --- a/python/packages/core/agent_framework/_skills.py +++ b/python/packages/core/agent_framework/_skills.py @@ -2953,13 +2953,12 @@ def _scan_directory_for_resources( resources.append(rel_path) - # Recurse into subdirectories if within depth limit + # Recurse into subdirectories if within depth limit. + # Subdirectories that contain their own SKILL.md are NOT skipped: a nested + # SKILL.md is not an independent skill (see _discover_skill_directories), so + # its contents belong to this skill. if current_depth < self._search_depth: for subdir in subdirectories: - # Skip subdirectories that contain their own SKILL.md — they are - # separate skills and their files should not be attached to this one. - if (subdir / SKILL_FILE_NAME).is_file(): - continue self._scan_directory_for_resources( target_dir=subdir, skill_dir=skill_dir, @@ -3104,13 +3103,12 @@ def _scan_directory_for_scripts( scripts.append(rel_path) - # Recurse into subdirectories if within depth limit + # Recurse into subdirectories if within depth limit. + # Subdirectories that contain their own SKILL.md are NOT skipped: a nested + # SKILL.md is not an independent skill (see _discover_skill_directories), so + # its contents belong to this skill. if current_depth < self._search_depth: for subdir in subdirectories: - # Skip subdirectories that contain their own SKILL.md — they are - # separate skills and their files should not be attached to this one. - if (subdir / SKILL_FILE_NAME).is_file(): - continue self._scan_directory_for_scripts( target_dir=subdir, skill_dir=skill_dir, @@ -3327,7 +3325,10 @@ def _read_and_parse_skill_file( def _discover_skill_directories(skill_paths: Sequence[str]) -> list[str]: """Return absolute paths of all directories that contain a ``SKILL.md`` file. - Recursively searches each root path up to :data:`MAX_SEARCH_DEPTH`. + Recursively searches each root path up to :data:`MAX_SEARCH_DEPTH`. Once a + ``SKILL.md`` is found in a directory, that directory is the skill root and the + search does not descend into its subdirectories: everything beneath a skill + boundary is part of that skill, not an independent skill root. Args: skill_paths: Root directory paths to search. @@ -3340,7 +3341,10 @@ def _discover_skill_directories(skill_paths: Sequence[str]) -> list[str]: def _search(directory: str, current_depth: int) -> None: dir_path = Path(directory) if (dir_path / SKILL_FILE_NAME).is_file(): + # This directory is a skill root. Subdirectories are part of this + # skill and must not be treated as independent skill roots. discovered.append(str(dir_path.absolute())) + return if current_depth >= MAX_SEARCH_DEPTH: return diff --git a/python/packages/core/tests/core/test_skills.py b/python/packages/core/tests/core/test_skills.py index 8de0439d3c..78b55a5c9c 100644 --- a/python/packages/core/tests/core/test_skills.py +++ b/python/packages/core/tests/core/test_skills.py @@ -1807,8 +1807,8 @@ async def test_from_paths_passes_resource_filter(self, tmp_path: Path) -> None: assert "keep.md" in resource_names assert "drop.md" not in resource_names - async def test_nested_skill_directory_not_crossed(self, tmp_path: Path) -> None: - """Files in a nested skill directory are NOT attached to the parent skill.""" + async def test_nested_skill_directory_absorbed_into_parent(self, tmp_path: Path) -> None: + """A nested SKILL.md is not an independent skill; its contents belong to the parent.""" parent_dir = tmp_path / "parent-skill" child_dir = parent_dir / "child-skill" child_dir.mkdir(parents=True) @@ -1828,22 +1828,19 @@ async def test_nested_skill_directory_not_crossed(self, tmp_path: Path) -> None: skills = await source.get_skills() skills_dict = {s.frontmatter.name: s for s in skills} - # Both skills are discovered + # Only the parent skill is discovered; the nested SKILL.md is not its own skill. assert "parent-skill" in skills_dict - assert "child-skill" in skills_dict + assert "child-skill" not in skills_dict - # Parent does NOT pick up child's files + # The parent absorbs the nested directory's resources and scripts. parent_resources = [r.name for r in skills_dict["parent-skill"]._resources] # type: ignore[attr-defined] # ty: ignore[unresolved-attribute] parent_scripts = [s.name for s in skills_dict["parent-skill"]._scripts] # type: ignore[attr-defined] # ty: ignore[unresolved-attribute] assert "parent-resource.md" in parent_resources - assert "child-skill/child-resource.md" not in parent_resources - assert "child-skill/child-script.py" not in parent_scripts + assert "child-skill/child-resource.md" in parent_resources + assert "child-skill/child-script.py" in parent_scripts - # Child has its own files - child_resources = [r.name for r in skills_dict["child-skill"]._resources] # type: ignore[attr-defined] # ty: ignore[unresolved-attribute] - child_scripts = [s.name for s in skills_dict["child-skill"]._scripts] # type: ignore[attr-defined] # ty: ignore[unresolved-attribute] - assert "child-resource.md" in child_resources - assert "child-script.py" in child_scripts + # The nested SKILL.md file itself is never surfaced as a resource. + assert "child-skill/SKILL.md" not in parent_resources # --------------------------------------------------------------------------- @@ -1996,6 +1993,17 @@ def test_finds_nested_skill(self, tmp_path: Path) -> None: assert len(dirs) == 1 assert str(sub.absolute()) in dirs[0] + def test_stops_searching_below_skill_boundary(self, tmp_path: Path) -> None: + skill_dir = tmp_path / "parent-skill" + nested_skill_dir = skill_dir / "nested-skill" + nested_skill_dir.mkdir(parents=True) + (skill_dir / "SKILL.md").write_text("---\nname: parent-skill\ndescription: d\n---\n", encoding="utf-8") + (nested_skill_dir / "SKILL.md").write_text("---\nname: nested-skill\ndescription: d\n---\n", encoding="utf-8") + + dirs = FileSkillsSource._discover_skill_directories([str(tmp_path)]) + + assert dirs == [str(skill_dir.absolute())] + def test_skips_empty_path_string(self) -> None: dirs = FileSkillsSource._discover_skill_directories(["", " "]) assert dirs == []