Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 15 additions & 11 deletions python/packages/core/agent_framework/_skills.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand All @@ -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
Comment thread
giles17 marked this conversation as resolved.

if current_depth >= MAX_SEARCH_DEPTH:
return
Expand Down
32 changes: 20 additions & 12 deletions python/packages/core/tests/core/test_skills.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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


# ---------------------------------------------------------------------------
Expand Down Expand Up @@ -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())]

Comment thread
giles17 marked this conversation as resolved.
def test_skips_empty_path_string(self) -> None:
dirs = FileSkillsSource._discover_skill_directories(["", " "])
assert dirs == []
Expand Down
Loading