From 4ee000b024a38501832415d403352847f342a31f Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Sat, 24 Oct 2026 11:01:32 -0700 Subject: [PATCH] fix(skills): discover_skills skips vendored and VCS directories A SKILL.md under .git, node_modules, site-packages, a virtualenv or __pycache__ below the catalog root is a dependency's copy, not a skill of the catalog; a differing version escaped the mirror rule and loaded as a rival skill. Those directories are no longer walked. Signed-off-by: Nikolay Petrov --- ..._napetrov_discover-skills-skip-vendored.md | 3 +++ dokimi/dokimi/skills/loader.py | 24 +++++++++++++++++-- dokimi/tests/test_skill_loader.py | 18 ++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 dokimi/changelog.d/20261024_130000_napetrov_discover-skills-skip-vendored.md diff --git a/dokimi/changelog.d/20261024_130000_napetrov_discover-skills-skip-vendored.md b/dokimi/changelog.d/20261024_130000_napetrov_discover-skills-skip-vendored.md new file mode 100644 index 00000000..9e1e0133 --- /dev/null +++ b/dokimi/changelog.d/20261024_130000_napetrov_discover-skills-skip-vendored.md @@ -0,0 +1,3 @@ +### Fixed + +- **Vendored skills no longer load** — skill discovery (`skill-catalog:` and `skill:` arms, `catalog` commands) does not walk `.git`, `node_modules`, `site-packages`, `.venv`/`venv`, `.tox`/`.nox`, `.hg`/`.svn` or `__pycache__` below the catalog root, so a dependency's copy of a skill is no longer offered as a skill of the catalog. diff --git a/dokimi/dokimi/skills/loader.py b/dokimi/dokimi/skills/loader.py index 52734027..d162df83 100644 --- a/dokimi/dokimi/skills/loader.py +++ b/dokimi/dokimi/skills/loader.py @@ -12,6 +12,7 @@ """ import hashlib +import os from dataclasses import dataclass, field from pathlib import Path from typing import Any, Dict, List, Optional, Tuple @@ -20,6 +21,10 @@ # Directory names that hold a scaffold to copy, not a skill to use (see discover_skills). TEMPLATE_DIR_NAMES = frozenset({"template", "templates"}) +# Directory names that hold version control, installed dependencies or virtualenvs: a +# SKILL.md there is a vendored copy of someone else's skill, not one of the catalog's. +VENDORED_DIR_NAMES = frozenset({".git", ".hg", ".svn", "node_modules", "site-packages", + ".venv", "venv", ".tox", ".nox", "__pycache__"}) @dataclass @@ -154,11 +159,25 @@ def load_skill(path) -> Skill: ) +def _skill_files(root: Path) -> List[Path]: + """Every ``SKILL.md`` below *root*, sorted, without walking :data:`VENDORED_DIR_NAMES`.""" + found: List[Path] = [] + for dirpath, dirnames, filenames in os.walk(root): + dirnames[:] = [d for d in dirnames if d not in VENDORED_DIR_NAMES] + if "SKILL.md" in filenames: + found.append(Path(dirpath) / "SKILL.md") + return sorted(found) + + def discover_skills(root, *, notes: Optional[List[str]] = None) -> List[Skill]: """Load every skill under *root* (each subdirectory with a ``SKILL.md``). - Two kinds of ``SKILL.md`` are not skills of *root* and are left out: + Three kinds of ``SKILL.md`` are not skills of *root* and are left out: + - one under a version-control, dependency or virtualenv directory + (:data:`VENDORED_DIR_NAMES`: ``.git``, ``node_modules``, ``site-packages``, + ``.venv`` ...) below *root*, a vendored copy of a package's skill that would + otherwise load as a skill of its own. Those directories are not walked; - one under a ``template`` or ``templates`` directory at any depth below *root*, a scaffold to copy (anthropics/skills keeps a blank one at ``template/SKILL.md``). A real skill kept under such a directory is left out too; @@ -171,13 +190,14 @@ def discover_skills(root, *, notes: Optional[List[str]] = None) -> List[Skill]: Args: root: Directory to scan. notes: When given, one line per skipped template or mirror is appended to it. + Vendored directories are skipped without a note. Returns: The skills, in path order. """ root = Path(root) skill_files: List[Path] = [] - for skill_file in sorted(root.rglob("SKILL.md")): + for skill_file in _skill_files(root): if TEMPLATE_DIR_NAMES.intersection(skill_file.relative_to(root).parts[:-1]): if notes is not None: notes.append(f"left out {skill_file}: a template, not a skill") diff --git a/dokimi/tests/test_skill_loader.py b/dokimi/tests/test_skill_loader.py index b35c7303..d7a803b3 100644 --- a/dokimi/tests/test_skill_loader.py +++ b/dokimi/tests/test_skill_loader.py @@ -95,6 +95,24 @@ def test_discovery_skips_a_template_directory(tmp_path, dirname): assert [s.name for s in discover_skills(root)] == ["linux-perf"] +@pytest.mark.parametrize("vendored", ["node_modules/pkg", ".git/modules/sub", ".venv/lib/python3.12/site-packages/pkg"]) +def test_discovery_skips_vendored_copies(tmp_path, vendored): + root = tmp_path / "repo" + _write_skill(root / "skills" / "linux-perf", "linux-perf", "Profile Linux programs.") + # A different version of the same skill, so the mirror rule would not drop it. + _write_skill(root / vendored / "skills" / "linux-perf", "linux-perf", "Profile programs, v0.") + _write_skill(root / vendored / "skills" / "other", "other", "Someone else's skill.") + notes: list[str] = [] + assert [s.name for s in discover_skills(root, notes=notes)] == ["linux-perf"] + assert notes == [] + + +def test_discovery_walks_a_root_inside_a_vendored_directory(tmp_path): + root = tmp_path / "node_modules" / "pkg" / "skills" + _write_skill(root / "a", "a", "Skill a.") + assert [s.name for s in discover_skills(root)] == ["a"] + + def test_discovery_reports_a_skipped_template(tmp_path): root = tmp_path / "catalog" _write_skill(root / "template", "template-skill", "Replace with description of the skill.")