From 7754c49217bb111ab528e87f8570c6e6725dc05c Mon Sep 17 00:00:00 2001 From: Xinmin Zeng <135568692+fancyboi999@users.noreply.github.com> Date: Mon, 2 Mar 2026 21:02:03 +0800 Subject: [PATCH] feat(skills): support recursive nested skill loading (#950) * feat(skills): support recursive nested skill loading * Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Willem Jiang Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- backend/CLAUDE.md | 2 +- backend/README.md | 1 + backend/src/skills/loader.py | 15 ++++---- backend/src/skills/parser.py | 3 +- backend/src/skills/types.py | 14 +++++-- backend/tests/test_skills_loader.py | 57 +++++++++++++++++++++++++++++ 6 files changed, 79 insertions(+), 13 deletions(-) create mode 100644 backend/tests/test_skills_loader.py diff --git a/backend/CLAUDE.md b/backend/CLAUDE.md index 25e0139..32b2bce 100644 --- a/backend/CLAUDE.md +++ b/backend/CLAUDE.md @@ -231,7 +231,7 @@ Proxied through nginx: `/api/langgraph/*` → LangGraph, all other `/api/*` → - **Location**: `deer-flow/skills/{public,custom}/` - **Format**: Directory with `SKILL.md` (YAML frontmatter: name, description, license, allowed-tools) -- **Loading**: `load_skills()` scans directories, parses SKILL.md, reads enabled state from extensions_config.json +- **Loading**: `load_skills()` recursively scans `skills/{public,custom}` for `SKILL.md`, parses metadata, and reads enabled state from extensions_config.json - **Injection**: Enabled skills listed in agent system prompt with container paths - **Installation**: `POST /api/skills/install` extracts .skill ZIP archive to custom/ directory diff --git a/backend/README.md b/backend/README.md index 1b96eb6..3b59625 100644 --- a/backend/README.md +++ b/backend/README.md @@ -77,6 +77,7 @@ Per-thread isolated execution with virtual path translation: - **Providers**: `LocalSandboxProvider` (filesystem) and `AioSandboxProvider` (Docker, in community/) - **Virtual paths**: `/mnt/user-data/{workspace,uploads,outputs}` → thread-specific physical directories - **Skills path**: `/mnt/skills` → `deer-flow/skills/` directory +- **Skills loading**: Recursively discovers nested `SKILL.md` files under `skills/{public,custom}` and preserves nested container paths - **Tools**: `bash`, `ls`, `read_file`, `write_file`, `str_replace` ### Subagent System diff --git a/backend/src/skills/loader.py b/backend/src/skills/loader.py index 1052108..7d4c37f 100644 --- a/backend/src/skills/loader.py +++ b/backend/src/skills/loader.py @@ -1,3 +1,4 @@ +import os from pathlib import Path from .parser import parse_skill_file @@ -59,16 +60,16 @@ def load_skills(skills_path: Path | None = None, use_config: bool = True, enable if not category_path.exists() or not category_path.is_dir(): continue - # Each subdirectory is a potential skill - for skill_dir in category_path.iterdir(): - if not skill_dir.is_dir(): + for current_root, dir_names, file_names in os.walk(category_path): + # Keep traversal deterministic and skip hidden directories. + dir_names[:] = sorted(name for name in dir_names if not name.startswith(".")) + if "SKILL.md" not in file_names: continue - skill_file = skill_dir / "SKILL.md" - if not skill_file.exists(): - continue + skill_file = Path(current_root) / "SKILL.md" + relative_path = skill_file.parent.relative_to(category_path) - skill = parse_skill_file(skill_file, category=category) + skill = parse_skill_file(skill_file, category=category, relative_path=relative_path) if skill: skills.append(skill) diff --git a/backend/src/skills/parser.py b/backend/src/skills/parser.py index eb96c2a..fe1c04c 100644 --- a/backend/src/skills/parser.py +++ b/backend/src/skills/parser.py @@ -4,7 +4,7 @@ from pathlib import Path from .types import Skill -def parse_skill_file(skill_file: Path, category: str) -> Skill | None: +def parse_skill_file(skill_file: Path, category: str, relative_path: Path | None = None) -> Skill | None: """ Parse a SKILL.md file and extract metadata. @@ -55,6 +55,7 @@ def parse_skill_file(skill_file: Path, category: str) -> Skill | None: license=license_text, skill_dir=skill_file.parent, skill_file=skill_file, + relative_path=relative_path or Path(skill_file.parent.name), category=category, enabled=True, # Default to enabled, actual state comes from config file ) diff --git a/backend/src/skills/types.py b/backend/src/skills/types.py index 183a406..0cdb668 100644 --- a/backend/src/skills/types.py +++ b/backend/src/skills/types.py @@ -11,13 +11,15 @@ class Skill: license: str | None skill_dir: Path skill_file: Path + relative_path: Path # Relative path from category root to skill directory category: str # 'public' or 'custom' enabled: bool = False # Whether this skill is enabled @property def skill_path(self) -> str: - """Returns the relative path from skills root to this skill's directory""" - return self.skill_dir.name + """Returns the relative path from the category root (skills/{category}) to this skill's directory""" + path = self.relative_path.as_posix() + return "" if path == "." else path def get_container_path(self, container_base_path: str = "/mnt/skills") -> str: """ @@ -29,7 +31,11 @@ class Skill: Returns: Full container path to the skill directory """ - return f"{container_base_path}/{self.category}/{self.skill_dir.name}" + category_base = f"{container_base_path}/{self.category}" + skill_path = self.skill_path + if skill_path: + return f"{category_base}/{skill_path}" + return category_base def get_container_file_path(self, container_base_path: str = "/mnt/skills") -> str: """ @@ -41,7 +47,7 @@ class Skill: Returns: Full container path to the skill's SKILL.md file """ - return f"{container_base_path}/{self.category}/{self.skill_dir.name}/SKILL.md" + return f"{self.get_container_path(container_base_path)}/SKILL.md" def __repr__(self) -> str: return f"Skill(name={self.name!r}, description={self.description!r}, category={self.category!r})" diff --git a/backend/tests/test_skills_loader.py b/backend/tests/test_skills_loader.py new file mode 100644 index 0000000..6d8fdd7 --- /dev/null +++ b/backend/tests/test_skills_loader.py @@ -0,0 +1,57 @@ +"""Tests for recursive skills loading.""" + +from pathlib import Path + +from src.skills.loader import load_skills + + +def _write_skill(skill_dir: Path, name: str, description: str) -> None: + """Write a minimal SKILL.md for tests.""" + skill_dir.mkdir(parents=True, exist_ok=True) + content = f"---\nname: {name}\ndescription: {description}\n---\n\n# {name}\n" + (skill_dir / "SKILL.md").write_text(content, encoding="utf-8") + + +def test_load_skills_discovers_nested_skills_and_sets_container_paths(tmp_path: Path): + """Nested skills should be discovered recursively with correct container paths.""" + skills_root = tmp_path / "skills" + + _write_skill(skills_root / "public" / "root-skill", "root-skill", "Root skill") + _write_skill(skills_root / "public" / "parent" / "child-skill", "child-skill", "Child skill") + _write_skill(skills_root / "custom" / "team" / "helper", "team-helper", "Team helper") + + skills = load_skills(skills_path=skills_root, use_config=False, enabled_only=False) + by_name = {skill.name: skill for skill in skills} + + assert {"root-skill", "child-skill", "team-helper"} <= set(by_name) + + root_skill = by_name["root-skill"] + child_skill = by_name["child-skill"] + team_skill = by_name["team-helper"] + + assert root_skill.skill_path == "root-skill" + assert root_skill.get_container_file_path() == "/mnt/skills/public/root-skill/SKILL.md" + + assert child_skill.skill_path == "parent/child-skill" + assert child_skill.get_container_file_path() == "/mnt/skills/public/parent/child-skill/SKILL.md" + + assert team_skill.skill_path == "team/helper" + assert team_skill.get_container_file_path() == "/mnt/skills/custom/team/helper/SKILL.md" + + +def test_load_skills_skips_hidden_directories(tmp_path: Path): + """Hidden directories should be excluded from recursive discovery.""" + skills_root = tmp_path / "skills" + + _write_skill(skills_root / "public" / "visible" / "ok-skill", "ok-skill", "Visible skill") + _write_skill( + skills_root / "public" / "visible" / ".hidden" / "secret-skill", + "secret-skill", + "Hidden skill", + ) + + skills = load_skills(skills_path=skills_root, use_config=False, enabled_only=False) + names = {skill.name for skill in skills} + + assert "ok-skill" in names + assert "secret-skill" not in names