From 4ee000b024a38501832415d403352847f342a31f Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Sat, 24 Oct 2026 11:01:32 -0700 Subject: [PATCH 1/7] 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 000000000..9e1e01331 --- /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 52734027c..d162df838 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 b35c73033..d7a803b38 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.") From 3f67aeae3dc70d775de41055d3cdbe207eccd712 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Sun, 25 Oct 2026 22:30:40 -0700 Subject: [PATCH 2/7] fix(catalog): the bare catalog scan skips vendored and VCS directories read_skill_manifest without a skills.yaml walked every SKILL.md under the root, so a dependency's copy under node_modules or a virtualenv was listed as a catalog skill. The changelog of this PR says catalog commands no longer offer those copies; the bare scan now uses the loader's vendored-aware walk (walk_skill_files, made public from _skill_files). Signed-off-by: Nikolay Petrov --- dokimi/dokimi/external/catalog.py | 7 ++++--- dokimi/dokimi/skills/loader.py | 4 ++-- dokimi/tests/test_external_catalog.py | 9 +++++++++ 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/dokimi/dokimi/external/catalog.py b/dokimi/dokimi/external/catalog.py index b52e88120..0313762d3 100644 --- a/dokimi/dokimi/external/catalog.py +++ b/dokimi/dokimi/external/catalog.py @@ -30,7 +30,7 @@ from dokimi.harnesses.tasks import TaskSpec, discover_tasks, is_task_dir, load_task from dokimi.skills import Skill, load_skill -from dokimi.skills.loader import TEMPLATE_DIR_NAMES +from dokimi.skills.loader import TEMPLATE_DIR_NAMES, walk_skill_files DEFAULT_TASKS_SUBDIR = "evaluation/harbor/tasks" DEFAULT_SUITES_MANIFEST = "evaluation/harbor/suites.json" @@ -99,10 +99,11 @@ def _scan_skill_files(root: Path) -> list[SkillCatalogEntry]: A skill that does not load is an error row, so one broken skill does not abort the scan. A ``SKILL.md`` under a ``template``/``templates`` directory is a scaffold and is left - out, as :func:`dokimi.skills.discover_skills` leaves it out. + out, as :func:`dokimi.skills.discover_skills` leaves it out; one under a vendored or + VCS directory is a dependency's copy and is not walked, as there too. """ found: list[SkillCatalogEntry] = [] - for skill_file in sorted(root.rglob("SKILL.md")): + for skill_file in walk_skill_files(root): if TEMPLATE_DIR_NAMES.intersection(skill_file.relative_to(root).parts[:-1]): continue try: diff --git a/dokimi/dokimi/skills/loader.py b/dokimi/dokimi/skills/loader.py index f78370381..899e7dbe3 100644 --- a/dokimi/dokimi/skills/loader.py +++ b/dokimi/dokimi/skills/loader.py @@ -159,7 +159,7 @@ def load_skill(path) -> Skill: ) -def _skill_files(root: Path) -> List[Path]: +def walk_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): @@ -202,7 +202,7 @@ def skill_files(root, *, notes: Optional[List[str]] = None) -> List[Path]: """The ``SKILL.md`` files :func:`discover_skills` loads, in path order, unloaded.""" root = Path(root) found: List[Path] = [] - for skill_file in _skill_files(root): + for skill_file in walk_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_external_catalog.py b/dokimi/tests/test_external_catalog.py index 6595a35ed..62ea6da58 100644 --- a/dokimi/tests/test_external_catalog.py +++ b/dokimi/tests/test_external_catalog.py @@ -187,6 +187,15 @@ def test_read_skill_manifest_falls_back_to_bare_scan_without_manifest(tmp_path): assert entries[0].status is None +@pytest.mark.parametrize("vendored", ["node_modules/pkg", ".venv/lib/python3.12/site-packages/pkg"]) +def test_read_skill_manifest_bare_scan_skips_vendored_copies(tmp_path, vendored): + _write_skill(tmp_path, "bare-skill") + _write_skill(tmp_path / vendored, "bare-skill") + + entries = read_skill_manifest(tmp_path) + assert [e.path for e in entries] == [tmp_path / "skills" / "bare-skill"] + + # ── suites.json manifest ────────────────────────────────────────────────────── From 0a96697716d7bb73e2970c91b8c97c9843056941 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Mon, 26 Oct 2026 12:36:18 +0000 Subject: [PATCH 3/7] Fix changelog entry: skill: arm does not use discovery Signed-off-by: Anton Volkov --- .../20261024_130000_napetrov_discover-skills-skip-vendored.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 9e1e01331..6fe816c74 100644 --- a/dokimi/changelog.d/20261024_130000_napetrov_discover-skills-skip-vendored.md +++ b/dokimi/changelog.d/20261024_130000_napetrov_discover-skills-skip-vendored.md @@ -1,3 +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. +- **Vendored skills no longer load** — skill discovery (the `skill-catalog:` arm and the `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. From 62489709d8e64b550b675c09cffcccd913003277 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Mon, 26 Oct 2026 12:37:25 +0000 Subject: [PATCH 4/7] Document that discovery skips vendored and VCS directories Signed-off-by: Anton Volkov --- dokimi/docs/README.md | 2 +- dokimi/docs/evaluating-treatments.md | 6 ++++-- dokimi/docs/external-content.md | 3 ++- dokimi/dokimi/commands/novelty.py | 10 +++++----- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/dokimi/docs/README.md b/dokimi/docs/README.md index 41b0dafc3..ce58b9fc1 100644 --- a/dokimi/docs/README.md +++ b/dokimi/docs/README.md @@ -37,7 +37,7 @@ flowchart LR | `skill:` | a skill (`SKILL.md`) injected as context | | `profile:` | an agent persona system prompt | | `agent:` / `skill-agent:` | **agentic** use — the model gets a tool and *decides* whether to fetch docs / load the skill | -| `skill-catalog:` | **selection** — every skill in a catalog is offered (a copy under a `template/` or `templates/` directory and a byte-identical mirror that loads under the same skill name are left out); scored on which one loads (`catalog trigger-set` builds the queries) | +| `skill-catalog:` | **selection** — every skill in a catalog is offered (a copy under a `template/` or `templates/` directory, a byte-identical mirror that loads under the same skill name, and anything under a vendored or VCS directory such as `.git`, `node_modules` or `.venv` are left out); scored on which one loads (`catalog trigger-set` builds the queries) | Two fine-print notes: diff --git a/dokimi/docs/evaluating-treatments.md b/dokimi/docs/evaluating-treatments.md index 493bd782f..de7824634 100644 --- a/dokimi/docs/evaluating-treatments.md +++ b/dokimi/docs/evaluating-treatments.md @@ -30,7 +30,7 @@ measure the marginal value of any context-augmentation artifact: | `skill:` | agent skill | injects a `SKILL.md` as context | | `agent[:source]` | **agentic** doc use | gives the model a doc-search tool; it decides when to call it | | `skill-agent:` | **agentic** skill use | offers the skill via progressive disclosure | -| `skill-catalog:` | **agentic** skill selection | offers every skill under `` (a `template/` copy and a byte-identical mirror that loads under the same skill name are left out); scores the one loaded against `expected_skill` | +| `skill-catalog:` | **agentic** skill selection | offers every skill under `` (a `template/` copy, a byte-identical mirror that loads under the same skill name, and anything under a vendored or VCS directory such as `.git` or `node_modules` are left out); scores the one loaded against `expected_skill` | | `+[+...]` | **combined** treatment | merges multiple treatments into one arm (e.g., `docs+skill:path`) | The last two run through a tool-calling loop @@ -290,7 +290,9 @@ artifact per cell, a `matrix_rollup.v1` JSON/Markdown report, and paired - **`skill-catalog:`** — skill selection. Every skill under `` is offered the same way, so the question is answered among its rivals. A `SKILL.md` under a `template/` or `templates/` directory, at any depth below ``, is not a skill and is - skipped (a real skill kept under such a directory is skipped too), and a `SKILL.md` + skipped (a real skill kept under such a directory is skipped too). Vendored and VCS + directories (`.git`, `.hg`, `.svn`, `node_modules`, `site-packages`, `.venv`, `venv`, + `.tox`, `.nox`, `__pycache__`) below `` are not walked, and no note says so. A `SKILL.md` byte-identical to another under `` that loads under the same skill name (a plugin's copy of `skills/`) is offered once. Questions that carry `expected_skill` are scored on the skill loaded first; see diff --git a/dokimi/docs/external-content.md b/dokimi/docs/external-content.md index 78302a4fa..ba7929a61 100644 --- a/dokimi/docs/external-content.md +++ b/dokimi/docs/external-content.md @@ -51,7 +51,8 @@ way the readers expect. Substitute your own checkout path. Both manifests are optional — with no manifest, the readers fall back to a plain filesystem scan of the same directories (a `SKILL.md` under a `template/` -or `templates/` directory is a scaffold and is left out). `skills.yaml` comes in two +or `templates/` directory is a scaffold and is left out; vendored and VCS directories such +as `.git`, `node_modules`, `site-packages` and `.venv` are not walked). `skills.yaml` comes in two shapes, and both hold a *list* of entries: ```yaml diff --git a/dokimi/dokimi/commands/novelty.py b/dokimi/dokimi/commands/novelty.py index ec6367d9f..7e71c8cfe 100644 --- a/dokimi/dokimi/commands/novelty.py +++ b/dokimi/dokimi/commands/novelty.py @@ -636,11 +636,11 @@ def register(sub, positive_int) -> None: "candidate is not routing: the sibling confusion is the " "failure mode (every `dpnp-*` skill scored 0.0 lexically)") routing_p.add_argument("--catalog", default=None, - help="Directory of skills to take the competitors from: every " - "SKILL.md under it except the artifact itself (a template/ " - "copy and a byte-identical mirror that loads under the same " - "skill name are left out), i.e. the siblings a harness " - "lists next to it. Combines with --competitors") + help="Directory of skills to take the competitors from: every SKILL.md " + "under it except the artifact itself (a template/ copy, a byte-" + "identical mirror that loads under the same skill name, and " + "vendored or VCS directories like .git are left out), i.e. the " + "siblings a harness lists next to it. Combines with --competitors") routing_p.add_argument("--queries", default=None, help="JSON list (or one per line) of user-shaped requests that " "should select this artifact, written independently of it") From 56366a0f4b0644c5c974428b4fba7b95297ce0e5 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Mon, 26 Oct 2026 11:30:05 -0700 Subject: [PATCH 5/7] fix(skills): catalog formats skips the vendored dirs discover_skills skips `dokimi catalog formats` walked its own smaller skip set, so a vendored copy under lib/site-packages, venv, .tox and similar was listed as a package that cannot load, while discover_skills and the catalog scan skip it. detect_skill_packages now skips VENDORED_DIR_NAMES, the set the loader walks. test_discovery_skips_each_vendored_name pins the ten names explicitly, so dropping any one of them from the set fails the test. The formats test covers the same case for detect_skill_packages. Signed-off-by: Nikolay Petrov --- dokimi/dokimi/skills/formats.py | 4 ++-- dokimi/tests/test_skill_formats.py | 10 +++++++++- dokimi/tests/test_skill_loader.py | 22 ++++++++++++++++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/dokimi/dokimi/skills/formats.py b/dokimi/dokimi/skills/formats.py index 26830c499..7677c0d97 100644 --- a/dokimi/dokimi/skills/formats.py +++ b/dokimi/dokimi/skills/formats.py @@ -36,7 +36,7 @@ from pathlib import Path from typing import Any, Dict, List, Optional -from dokimi.skills.loader import skill_files +from dokimi.skills.loader import VENDORED_DIR_NAMES, skill_files from dokimi.utils import parse_frontmatter FORMAT_AGENT_SKILL = "agent-skill" @@ -60,7 +60,7 @@ (".agents/skills", "agents"), # the cross-tool alias Codex and Gemini CLI read ("environment/skills", "task-bundled"), # a SkillsBench / Harbor task's own skills ) -_SKIP_DIRS = frozenset({".git", "node_modules", "__pycache__", ".venv"}) +_SKIP_DIRS = VENDORED_DIR_NAMES # the same dependency and VCS dirs discover_skills does not walk @dataclass diff --git a/dokimi/tests/test_skill_formats.py b/dokimi/tests/test_skill_formats.py index f2ae8a0d7..b110599a8 100644 --- a/dokimi/tests/test_skill_formats.py +++ b/dokimi/tests/test_skill_formats.py @@ -83,10 +83,18 @@ def test_each_ecosystem_is_named_with_how_it_is_selected(tmp_path): def test_every_loadable_package_is_one_the_skill_loader_loads(tmp_path): root = _tree(tmp_path) loadable = {p.name for p in sf.detect_skill_packages(root) if p.loadable} - # The loader does not skip node_modules/ (a vendored copy loads as a skill); the scan does. + # node_modules/ is a vendored directory: the loader and the scan both skip it. assert loadable == {s.name for s in discover_skills(root)} - {"x"} +def test_a_vendored_skill_is_not_listed_as_a_package(tmp_path): + _skill(tmp_path / "skills/a", "a") + _skill(tmp_path / "lib/site-packages/pkg/skills/b", "b") + _skill(tmp_path / "venv/skills/c", "c") + paths = [p.path.relative_to(tmp_path).as_posix() for p in sf.detect_skill_packages(tmp_path)] + assert paths == ["skills/a/SKILL.md"] + + def test_a_malformed_manifest_is_a_problem_not_a_crash(tmp_path): for value in (5, True, "abc"): root = tmp_path / str(value) diff --git a/dokimi/tests/test_skill_loader.py b/dokimi/tests/test_skill_loader.py index d7a803b38..2689283b1 100644 --- a/dokimi/tests/test_skill_loader.py +++ b/dokimi/tests/test_skill_loader.py @@ -107,6 +107,28 @@ def test_discovery_skips_vendored_copies(tmp_path, vendored): assert notes == [] +@pytest.mark.parametrize( + "name", + [ + ".git", + ".hg", + ".svn", + "node_modules", + "site-packages", + ".venv", + "venv", + ".tox", + ".nox", + "__pycache__", + ], +) +def test_discovery_skips_each_vendored_name(tmp_path, name): + root = tmp_path / "repo" + _write_skill(root / "skills" / "a", "a", "Skill a.") + _write_skill(root / "skills" / "a" / name / "pkg", "b", "Vendored b.") + assert [s.name for s in discover_skills(root)] == ["a"] + + 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.") From 201bb90749d395caba9977dabe07a9e721da4703 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Mon, 26 Oct 2026 19:07:05 +0000 Subject: [PATCH 6/7] docs: say --catalog does not walk vendored and VCS directories Signed-off-by: Anton Volkov --- README.md | 3 ++- dokimi/docs/skill-novelty-walkthrough.md | 4 +++- dokimi/docs/skill-novelty.md | 3 ++- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 5ddce8210..ab0b642ff 100644 --- a/README.md +++ b/README.md @@ -166,7 +166,8 @@ the scores. `novelty` refuses a self-judge; `arms run` does not check, so name - **Competitors** are the other skills an agent sees next to this one when it picks a skill to load. For a catalog skill they are its siblings in the same catalog: `--catalog ` takes every other skill under it (a `template/` copy and a - byte-identical mirror that loads under the same skill name are left out), and + byte-identical mirror that loads under the same skill name are left out, and vendored and + VCS directories such as `.git`, `node_modules`, `site-packages` and virtualenvs are not walked), and `--competitors skill: …` names them one by one. Routing asks whether this skill wins the user requests meant for it. If you only have the one skill, skip step 2 and say so in the review: a number measured against diff --git a/dokimi/docs/skill-novelty-walkthrough.md b/dokimi/docs/skill-novelty-walkthrough.md index 0e94c594f..663c57c53 100644 --- a/dokimi/docs/skill-novelty-walkthrough.md +++ b/dokimi/docs/skill-novelty-walkthrough.md @@ -42,7 +42,9 @@ python cli.py novelty routing --artifact skill:skills/my-skill \ The competitors are the skills a harness lists next to this one: `--catalog ` takes every other `SKILL.md` under the directory (a `template/` copy and a byte-identical mirror -that loads under the same skill name are left out), and `--competitors` adds or names them one by +that loads under the same skill name are left out, and vendored and VCS directories such as +`.git`, `node_modules`, `site-packages` and virtualenvs are not walked), and `--competitors` +adds or names them one by one. With no real siblings at hand, skip this step and say so; a field of competitors made up for the run measures nothing. diff --git a/dokimi/docs/skill-novelty.md b/dokimi/docs/skill-novelty.md index 818da3806..cac5b26d6 100644 --- a/dokimi/docs/skill-novelty.md +++ b/dokimi/docs/skill-novelty.md @@ -602,7 +602,8 @@ python cli.py novelty routing --artifact skill:path/to/skill \ `--catalog ` takes the competitors from a skill directory instead: every `SKILL.md` under it except the artifact itself (a `template/` copy and a byte-identical mirror that -loads under the same skill name are left out). +loads under the same skill name are left out, and vendored and VCS directories such as `.git`, +`node_modules`, `site-packages` and virtualenvs are not walked). Two numbers, reported side by side and never merged: From a9874651159d7b95618893be18184a691fdc61a7 Mon Sep 17 00:00:00 2001 From: Alexander Rybkin Date: Tue, 27 Oct 2026 03:44:07 -0700 Subject: [PATCH 7/7] fix(catalog): bare catalog scan reports a SKILL.md that is not a file walk_skill_files dropped dangling symlinks and directories named SKILL.md, so catalog scan lost the error row and exited 0 on a broken skill. The walker now returns every SKILL.md entry; skill_files keeps the is_file guard, so discover_skills still leaves such entries out. Signed-off-by: Alexander Rybkin --- dokimi/dokimi/skills/loader.py | 13 ++++++++----- dokimi/tests/test_catalog_command.py | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/dokimi/dokimi/skills/loader.py b/dokimi/dokimi/skills/loader.py index d8fbfd148..05c01c562 100644 --- a/dokimi/dokimi/skills/loader.py +++ b/dokimi/dokimi/skills/loader.py @@ -160,14 +160,15 @@ def load_skill(path) -> Skill: def walk_skill_files(root: Path) -> List[Path]: - """Every ``SKILL.md`` below *root*, sorted, without walking :data:`VENDORED_DIR_NAMES`.""" + """Every entry named ``SKILL.md`` below *root*, sorted, without walking :data:`VENDORED_DIR_NAMES`. + + A dangling symlink or a directory is kept: ``catalog scan`` reports it as an error row. + """ found: List[Path] = [] for dirpath, dirnames, filenames in os.walk(root): dirnames[:] = [d for d in dirnames if d not in VENDORED_DIR_NAMES] - skill_file = Path(dirpath) / "SKILL.md" - # A dangling symlink named SKILL.md lands in filenames; only real files count. - if "SKILL.md" in filenames and skill_file.is_file(): - found.append(skill_file) + if "SKILL.md" in filenames or "SKILL.md" in dirnames: + found.append(Path(dirpath) / "SKILL.md") return sorted(found) @@ -205,6 +206,8 @@ def skill_files(root, *, notes: Optional[List[str]] = None) -> List[Path]: root = Path(root) found: List[Path] = [] for skill_file in walk_skill_files(root): + if not skill_file.is_file(): + continue 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_catalog_command.py b/dokimi/tests/test_catalog_command.py index b86fa026e..2760a164f 100644 --- a/dokimi/tests/test_catalog_command.py +++ b/dokimi/tests/test_catalog_command.py @@ -116,6 +116,28 @@ def test_catalog_scan_exits_nonzero_on_load_error(tmp_path, capsys): assert exc_info.value.code == 1 +@pytest.mark.parametrize("broken", ["dangling-symlink", "directory"]) +def test_catalog_scan_exits_nonzero_on_a_skill_md_that_is_not_a_file(tmp_path, capsys, broken): + # discover_skills leaves such a SKILL.md out; the bare scan must still report it. + _write_skill(tmp_path, "good") + bad = tmp_path / "skills" / "bad" / "SKILL.md" + bad.parent.mkdir(parents=True) + if broken == "directory": + bad.mkdir() + else: + try: + bad.symlink_to(tmp_path / "missing.md") + except OSError: # Windows without Developer Mode or the symlink privilege + pytest.skip("this OS refuses an unprivileged symlink") + + parser = build_parser() + args = parser.parse_args(["catalog", "scan", "--root", str(tmp_path), "--kind", "skills"]) + with pytest.raises(SystemExit) as exc_info: + args.func(args) + assert exc_info.value.code == 1 + assert "bad" in capsys.readouterr().out + + def test_catalog_scan_exits_nonzero_on_a_task_that_exists_but_cannot_load(tmp_path, capsys): # The tasks-half sibling of the test above. Both halves need their own case: # skills failures carry an `error`, task failures a `failed` flag, and the