From 6e1e1e951102119f47e549b9adbe008a5482ddc3 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Wed, 21 Oct 2026 21:47:35 -0700 Subject: [PATCH 1/8] feat(catalog): review skills against the spec and flag identical copies `catalog scan` checked only that a skill loads. Each loaded skill is now also reviewed: the directory name must equal `name` and be kebab-case (at most 64 characters), and `description` must be at most 1024 characters (errors, exit 1). A missing `name` and a SKILL.md byte-identical to an earlier skill's are warnings naming the first copy. disable-model-invocation is shown as a note. `--format json` prints one object with a row per skill and task. Signed-off-by: Nikolay Petrov --- ...50000_napetrov_catalog-scan-spec-review.md | 20 ++ dokimi/docs/external-content.md | 10 + dokimi/dokimi/commands/catalog.py | 76 ++++++-- dokimi/dokimi/external/__init__.py | 4 + dokimi/dokimi/external/validate.py | 124 +++++++++++++ dokimi/tests/test_catalog_validate.py | 173 ++++++++++++++++++ 6 files changed, 396 insertions(+), 11 deletions(-) create mode 100644 dokimi/changelog.d/20261021_150000_napetrov_catalog-scan-spec-review.md create mode 100644 dokimi/dokimi/external/validate.py create mode 100644 dokimi/tests/test_catalog_validate.py diff --git a/dokimi/changelog.d/20261021_150000_napetrov_catalog-scan-spec-review.md b/dokimi/changelog.d/20261021_150000_napetrov_catalog-scan-spec-review.md new file mode 100644 index 00000000..bc3a863b --- /dev/null +++ b/dokimi/changelog.d/20261021_150000_napetrov_catalog-scan-spec-review.md @@ -0,0 +1,20 @@ +### Added + +- **`catalog scan` reviews each skill against the Agent Skills spec** — a loaded skill + whose `name` is not its directory name or not kebab-case (at most 64 characters), or + whose `description` is over 1024 characters, is an `error`; a `name` missing from the + frontmatter, or a `SKILL.md` byte-identical to an earlier skill's (a mirror, named + with its first copy), is a `warn`. `disable-model-invocation: true` is printed as a + note, since the model is never offered that skill. Each finding is printed under its + skill. +- **`catalog scan --format json`** — one object with a row per skill (`state` is + `ok`, `warn` or `error`, `findings`, `content_sha256`, `duplicate_of`) and per task + (`state` is `ok`, `skipped` or `error`), for a CI gate or a script. + +### Changed + +- **`catalog scan` exits 1 on a skill spec error** (a name that is not the directory + name or not kebab-case, a description over 1024 characters), as it already did for a + skill that does not load. Warnings (mirrors, a missing `name`) do not change the exit + code. A catalog that declares such a skill now fails the scan; fix the frontmatter + or drop the skill from the scan. diff --git a/dokimi/docs/external-content.md b/dokimi/docs/external-content.md index 7c095155..efc5aac9 100644 --- a/dokimi/docs/external-content.md +++ b/dokimi/docs/external-content.md @@ -115,6 +115,16 @@ with no directory is skipped, while a `planned` *skill* with no directory is an error — the skill reader has no notion of a not-yet-written entry. Either drop such a skill from `skills.yaml` or expect the non-zero exit. +A loaded skill is also checked against the Agent Skills spec. Its `name` must be +its directory name and kebab-case (at most 64 characters), and its `description` +at most 1024 characters: a violation is an `error` and exits 1, as a load error +does. A `name` missing from the frontmatter, or a `SKILL.md` byte-identical to an +earlier skill's, is a `warn` that names the first copy; warnings do not change +the exit code. `disable-model-invocation: true` is printed as a `note`, because +the model is never offered that skill. Each finding is printed under its skill. +`--format json` prints the same rows as one object (`state`, `findings`, +`content_sha256`, `duplicate_of`) for a script or a CI gate. + Restrict the scan with `--kind`: ```console diff --git a/dokimi/dokimi/commands/catalog.py b/dokimi/dokimi/commands/catalog.py index 30a2be7c..27073033 100644 --- a/dokimi/dokimi/commands/catalog.py +++ b/dokimi/dokimi/commands/catalog.py @@ -13,17 +13,41 @@ from __future__ import annotations import argparse +import json import sys from pathlib import Path +def _skill_row(skill, review) -> dict: + """One skill of the scan report. ``state`` is ``error`` for a load error or an error finding, + ``warn`` for a warning finding only, else ``ok``.""" + if not skill.loaded: + state, findings = "error", [] + else: + state, findings = review.state, review.findings + return { + "name": skill.name, + "manifest_status": skill.status, + "path": str(skill.path) if skill.path else None, + "state": state, + "error": skill.error, + "disable_model_invocation": bool(review and review.disable_model_invocation), + "content_sha256": review.content_sha256 if review else None, + "duplicate_of": review.duplicate_of if review else None, + "findings": [{"level": f.level, "code": f.code, "message": f.message} for f in findings], + } + + def cmd_catalog_scan(args: argparse.Namespace) -> None: from dokimi.external import read_skill_manifest, read_task_manifest + from dokimi.external.validate import review_skills root = Path(args.root) if not root.is_dir(): raise SystemExit(f"--root is not a directory: {root}") + as_json = args.format == "json" + report: dict = {"root": str(root), "skills": [], "tasks": []} any_failure = False if args.kind in ("skills", "all"): @@ -31,21 +55,39 @@ def cmd_catalog_scan(args: argparse.Namespace) -> None: skills = read_skill_manifest(root) except ValueError as exc: raise SystemExit(f"skills.yaml: {exc}") from exc - print(f"# skills ({len(skills)} declared under {root})") - for skill in skills: - state = "ok" if skill.loaded else f"ERROR: {skill.error}" - status = skill.status or "-" - print(f"{skill.name}\t{status}\t{skill.path}\t{state}") - if not skill.loaded: + reviews = review_skills(skills) + counts = {"ok": 0, "warn": 0, "error": 0} + for skill, review in zip(skills, reviews, strict=True): + row = _skill_row(skill, review) + counts[row["state"]] += 1 + report["skills"].append(row) + if row["state"] == "error": any_failure = True - print() + if not as_json: + print(f"# skills ({len(skills)} declared under {root}; " + f"{counts['ok']} ok, {counts['warn']} warn, {counts['error']} error)") + for row in report["skills"]: + if row["error"] is not None: + state = f"ERROR: {row['error']}" + elif row["state"] == "ok": + state = "ok" + else: + worst = next(f for f in row["findings"] if f["level"] == row["state"]) + state = f"{'ERROR' if row['state'] == 'error' else 'warn'}: {worst['message']}" + print(f"{row['name']}\t{row['manifest_status'] or '-'}\t{row['path'] or '-'}\t{state}") + for f in row["findings"]: + print(f" {f['level']} {f['code']}: {f['message']}") + if row["disable_model_invocation"]: + print(" note disable-model-invocation: true (the model is never offered this skill)") + print() if args.kind in ("tasks", "all"): try: tasks = read_task_manifest(root, tasks_subdir=args.tasks_subdir) except ValueError as exc: raise SystemExit(f"suites.json: {exc}") from exc - print(f"# tasks ({len(tasks)} declared under {root})") + if not as_json: + print(f"# tasks ({len(tasks)} declared under {root})") for task in tasks: if task.loaded: state = "ok" @@ -54,9 +96,17 @@ def cmd_catalog_scan(args: argparse.Namespace) -> None: else: state = f"ERROR: {task.reason}" any_failure = True - track = task.track or "-" - status = task.status or "-" - print(f"{task.name}\t{status}\t{track}\t{state}") + report["tasks"].append({ + "name": task.name, "manifest_status": task.status, "track": task.track, + "path": str(task.path) if task.path else None, + "state": "ok" if task.loaded else "skipped" if task.skipped else "error", + "reason": task.reason, + }) + if not as_json: + print(f"{task.name}\t{task.status or '-'}\t{task.track or '-'}\t{state}") + + if as_json: + print(json.dumps(report, indent=2)) if any_failure: raise SystemExit(1) @@ -159,6 +209,10 @@ def register(sub, positive_int) -> None: "--kind", choices=["skills", "tasks", "all"], default="all", help="Restrict the scan to skills, tasks, or both (default: all)", ) + scan_p.add_argument( + "--format", choices=["text", "json"], default="text", + help="text: one line per entry; json: one object with a row per skill and task (default: text)", + ) scan_p.add_argument( "--tasks-subdir", default="evaluation/harbor/tasks", help="Task directory relative to --root (default: evaluation/harbor/tasks, " diff --git a/dokimi/dokimi/external/__init__.py b/dokimi/dokimi/external/__init__.py index 227aa312..373be746 100644 --- a/dokimi/dokimi/external/__init__.py +++ b/dokimi/dokimi/external/__init__.py @@ -9,10 +9,14 @@ read_skill_manifest, read_task_manifest, ) +from .validate import Finding, SkillReview, review_skills __all__ = [ "SkillCatalogEntry", "TaskCatalogEntry", "read_skill_manifest", "read_task_manifest", + "Finding", + "SkillReview", + "review_skills", ] diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py new file mode 100644 index 00000000..76a367a8 --- /dev/null +++ b/dokimi/dokimi/external/validate.py @@ -0,0 +1,124 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Review the skills of an external catalog: spec frontmatter, and identical copies. + +Runs after the readers of :mod:`dokimi.external.catalog` and only on skills +that loaded (a load error is already reported as an error by the scan). Checks +follow the Agent Skills spec: + +- ``name`` is the directory name, kebab-case (lowercase letters, digits and + single hyphens) and at most 64 characters. A mismatch or a bad format is an + ``error``; a ``name`` missing from the frontmatter is a ``warn``, because the + loader falls back to the directory name silently. +- ``description`` is at most 1024 characters (``error``). A missing or empty + description is a load error, raised by the loader. + +Identical copies: a ``SKILL.md`` whose bytes are the same as an earlier +skill's is a ``warn`` naming that earlier skill. The model sees only the name and +description, so two such skills compete for the same query even when their bundled +files differ (a plugin copy with an extra ``agents/`` folder is still a mirror). +Mirrors are common (one skill checked in under two plugin directories), and +counting them twice inflates routing and coverage numbers. + +``disable-model-invocation: true`` is not a finding: the model is never offered +that skill, so it is reported as a flag (the same reading as the skill arm's +check in :mod:`dokimi.harnesses.skill_treatment`). +""" + +from __future__ import annotations + +import hashlib +import re +from dataclasses import dataclass, field +from typing import Optional, Sequence + +from dokimi.skills import Skill +from dokimi.utils import parse_frontmatter + +from .catalog import SkillCatalogEntry + +NAME_PATTERN = re.compile(r"[a-z0-9]+(?:-[a-z0-9]+)*") +NAME_MAX = 64 +DESCRIPTION_MAX = 1024 + + +@dataclass +class Finding: + """One check that did not pass: ``level`` is ``warn`` or ``error``.""" + + level: str + code: str + message: str + + +@dataclass +class SkillReview: + """What :func:`review_skills` found for one loaded skill.""" + + findings: list[Finding] = field(default_factory=list) + disable_model_invocation: bool = False + content_sha256: Optional[str] = None + duplicate_of: Optional[str] = None # the skill directory of the earlier copy, relative to the catalog root + + @property + def state(self) -> str: + """``error`` if any finding is an error, else ``warn`` if any is a warning, else ``ok``.""" + levels = {f.level for f in self.findings} + return "error" if "error" in levels else "warn" if "warn" in levels else "ok" + + +def _frontmatter_findings(skill: Skill) -> tuple[list[Finding], bool]: + """The spec checks on the frontmatter of ``skill``, and its ``disable-model-invocation`` flag.""" + meta, _ = parse_frontmatter(skill.path.read_text(encoding="utf-8")) + dir_name = skill.path.parent.name + findings: list[Finding] = [] + declared = str(meta.get("name") or "").strip() + if not declared: + findings.append(Finding("warn", "name-missing", + f"no name in the frontmatter; the directory name {dir_name!r} is used")) + else: + if declared != dir_name: + findings.append(Finding("error", "name-mismatch", + f"name {declared!r} is not the directory name {dir_name!r}")) + if len(declared) > NAME_MAX or not NAME_PATTERN.fullmatch(declared): + findings.append(Finding("error", "name-format", + f"name {declared!r} is not kebab-case (lowercase letters, digits, " + f"single hyphens; at most {NAME_MAX} characters)")) + if len(skill.description) > DESCRIPTION_MAX: + findings.append(Finding("error", "description-too-long", + f"description is {len(skill.description)} characters; the limit is {DESCRIPTION_MAX}")) + # Read as the skill arm reads it (dokimi.harnesses.skill_treatment.check_skill). + hidden = str(meta.get("disable-model-invocation", "")).strip().lower() in ("1", "true", "yes", "on") + return findings, hidden + + +def _skill_digest(skill: Skill) -> str: + """SHA-256 of the ``SKILL.md`` bytes: the text the model is shown when it chooses a skill.""" + return hashlib.sha256(skill.path.read_bytes()).hexdigest() + + +def review_skills(entries: Sequence[SkillCatalogEntry]) -> list[Optional[SkillReview]]: + """One :class:`SkillReview` per entry, in order; None for an entry that did not load. + + The first skill of each identical ``SKILL.md`` is the original; a later copy names it in + ``duplicate_of``. + """ + reviews: list[Optional[SkillReview]] = [] + first_by_digest: dict[str, Skill] = {} + for entry in entries: + if not entry.loaded or entry.skill is None: + reviews.append(None) + continue + skill = entry.skill + findings, hidden = _frontmatter_findings(skill) + review = SkillReview(findings=findings, disable_model_invocation=hidden) + review.content_sha256 = _skill_digest(skill) + original = first_by_digest.setdefault(review.content_sha256, skill) + if original is not skill: + where = original.path.parent.relative_to(entry.source_root).as_posix() + review.duplicate_of = where + review.findings.append(Finding("warn", "duplicate", + f"SKILL.md is identical to {where}; the same skill is counted twice")) + reviews.append(review) + return reviews diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py new file mode 100644 index 00000000..f566b3ae --- /dev/null +++ b/dokimi/tests/test_catalog_validate.py @@ -0,0 +1,173 @@ +# Copyright (C) 2026 Intel Corporation +# SPDX-License-Identifier: Apache-2.0 + +"""Spec and mirror review of ``catalog scan`` (``dokimi.external.validate``), and its ``--format json`` output.""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from dokimi.cli import build_parser +from dokimi.external import read_skill_manifest, review_skills + + +def _skill(root: Path, rel: str, name: str | None = None, description: str = "Use for things.", + extra: str = "", body: str = "# Body\n") -> Path: + """Write ``//SKILL.md``; ``name`` None leaves the key out of the frontmatter.""" + skill_dir = root / rel + skill_dir.mkdir(parents=True, exist_ok=True) + head = f"name: {name}\n" if name is not None else "" + (skill_dir / "SKILL.md").write_text( + f"---\n{head}description: {description}\n{extra}---\n{body}", encoding="utf-8") + return skill_dir + + +def _scan_json(root: Path, capsys, *, kind: str = "skills") -> tuple[dict, int]: + """Run ``catalog scan --format json``; return the parsed report and the exit code (0 when none raised).""" + args = build_parser().parse_args( + ["catalog", "scan", "--root", str(root), "--kind", kind, "--format", "json"]) + code = 0 + try: + args.func(args) + except SystemExit as exc: + code = exc.code + return json.loads(capsys.readouterr().out), code + + +def _reviews_by_name(root: Path) -> dict: + entries = read_skill_manifest(root) + return {e.name: r for e, r in zip(entries, review_skills(entries), strict=True)} + + +def test_name_that_is_not_the_directory_name_is_an_error(tmp_path): + _skill(tmp_path / "skills", "writing-rules", name="Writing Hookify Rules") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "error" + codes = {f.code for f in review.findings} + assert {"name-mismatch", "name-format"} <= codes + + +def test_title_case_name_matching_directory_is_still_not_kebab_case(tmp_path): + _skill(tmp_path / "skills", "Writing", name="Writing") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "error" + assert [f.code for f in review.findings] == ["name-format"] + + +def test_kebab_case_name_matching_directory_is_ok(tmp_path): + _skill(tmp_path / "skills", "linux-perf-2", name="linux-perf-2") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "ok" + assert review.findings == [] + + +def test_name_missing_from_frontmatter_is_a_warning(tmp_path): + _skill(tmp_path / "skills", "no-name", name=None) + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "warn" + assert [f.code for f in review.findings] == ["name-missing"] + + +def test_description_length_limit_is_1024_characters(tmp_path): + _skill(tmp_path / "skills", "at-limit", name="at-limit", description="d" * 1024) + _skill(tmp_path / "skills", "over-limit", name="over-limit", description="d" * 1025) + + reviews = _reviews_by_name(tmp_path) + assert reviews["at-limit"].state == "ok" + over = reviews["over-limit"] + assert over.state == "error" + assert [f.code for f in over.findings] == ["description-too-long"] + + +def test_identical_skill_md_is_a_warning_naming_the_first_copy(tmp_path): + # A plugin copy with an extra bundled folder is still the same SKILL.md: the model sees the same text. + _skill(tmp_path / "plugins", "x", name="x", description="Use for x.") + copy = _skill(tmp_path / "skills", "x", name="x", description="Use for x.") + (copy / "agents").mkdir() + (copy / "agents" / "openai.yaml").write_text("a: 1\n", encoding="utf-8") + + reviews = review_skills(read_skill_manifest(tmp_path)) + originals = [r for r in reviews if r.duplicate_of is None] + copies = [r for r in reviews if r.duplicate_of is not None] + assert len(originals) == 1 and len(copies) == 1 + assert copies[0].duplicate_of == "plugins/x" + assert copies[0].state == "warn" + assert copies[0].findings[-1].code == "duplicate" + assert copies[0].content_sha256 == originals[0].content_sha256 + + +def test_different_skill_md_with_the_same_name_is_not_a_duplicate(tmp_path): + _skill(tmp_path / "plugins", "x", name="x", description="Use for x.") + _skill(tmp_path / "skills", "x", name="x", description="Use for x, a different text.") + + reviews = review_skills(read_skill_manifest(tmp_path)) + assert all(r.duplicate_of is None for r in reviews) + assert all(r.state == "ok" for r in reviews) + + +def test_disable_model_invocation_is_reported_as_a_flag_not_a_finding(tmp_path): + _skill(tmp_path / "skills", "hidden", name="hidden", + extra="disable-model-invocation: true\n") + _skill(tmp_path / "skills", "shown", name="shown") + + reviews = _reviews_by_name(tmp_path) + assert reviews["hidden"].disable_model_invocation is True + assert reviews["hidden"].state == "ok" + assert reviews["shown"].disable_model_invocation is False + + +def test_json_report_carries_the_findings_and_exits_1_on_an_error(tmp_path, capsys): + _skill(tmp_path / "skills", "claude-api", name="Claude API") + report, code = _scan_json(tmp_path, capsys) + + assert code == 1 + [row] = report["skills"] + assert row["state"] == "error" + assert row["error"] is None + assert {f["code"] for f in row["findings"]} >= {"name-mismatch", "name-format"} + assert row["content_sha256"] and len(row["content_sha256"]) == 64 + + +def test_json_report_on_warnings_only_exits_0(tmp_path, capsys): + _skill(tmp_path / "plugins", "x", name="x") + _skill(tmp_path / "skills", "x", name="x") + _skill(tmp_path / "skills", "hidden", name="hidden", extra="disable-model-invocation: yes\n") + report, code = _scan_json(tmp_path, capsys) + + assert code == 0 + states = {row["path"].split(str(tmp_path) + "/")[1]: row["state"] for row in report["skills"]} + assert states == {"plugins/x": "ok", "skills/x": "warn", "skills/hidden": "ok"} + hidden = next(r for r in report["skills"] if r["name"] == "hidden") + assert hidden["disable_model_invocation"] is True + assert report["tasks"] == [] + + +def test_json_report_keeps_load_errors_as_error_rows(tmp_path, capsys): + (tmp_path / "skills.yaml").write_text( + 'skills:\n - name: ghost\n catalog_path: skills/does-not-exist\n', encoding="utf-8") + report, code = _scan_json(tmp_path, capsys) + + assert code == 1 + [row] = report["skills"] + assert row["state"] == "error" + assert "does-not-exist" in row["error"] + + +def test_text_output_lists_each_finding_under_its_skill(tmp_path, capsys): + _skill(tmp_path / "skills", "claude-api", name="Claude API") + args = build_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 + out = capsys.readouterr().out + assert "# skills (1 declared" in out and "1 error" in out + assert "ERROR: name 'Claude API' is not the directory name" in out + assert " error name-mismatch:" in out + assert " error name-format:" in out From 1868f79f0e9a31655443198f30bcd60acb542768 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Wed, 21 Oct 2026 22:12:54 -0700 Subject: [PATCH 2/8] fix(catalog): spec errors exit 0 without --strict; name broken YAML `catalog scan` exited 1 on a skill spec error. Spec errors now print and exit 0; `--strict` makes them exit 1, as a load error does. Load errors still exit 1 always. A SKILL.md whose frontmatter is not valid YAML is reported as a `frontmatter-parse` error carrying the YAML message. The loader reads that as "no description", so the scan showed only a misleading missing description error before. The scan uses a new parse_frontmatter_strict path; parse_frontmatter is unchanged for other callers. Without a skills.yaml, the scan's discovery path now records a broken skill as an error row instead of stopping at the first one. validate.py is ruff-formatted; catalog.py only in the lines this branch added. Signed-off-by: Nikolay Petrov --- README.md | 2 +- ...50000_napetrov_catalog-scan-spec-review.md | 16 ++- dokimi/docs/external-content.md | 25 ++-- dokimi/dokimi/commands/catalog.py | 48 +++++--- dokimi/dokimi/external/catalog.py | 37 ++++-- dokimi/dokimi/external/validate.py | 110 ++++++++++++++---- dokimi/dokimi/utils.py | 23 +++- dokimi/tests/test_catalog_validate.py | 75 ++++++++++-- 8 files changed, 267 insertions(+), 69 deletions(-) diff --git a/README.md b/README.md index 05b341d2..e5ac18a9 100644 --- a/README.md +++ b/README.md @@ -154,7 +154,7 @@ the scores. `novelty` refuses a self-judge; `arms run` does not check, so name | Step | Command | Cost | |---|---|---| -| 1. Does it load? | `dokimi catalog scan --root --kind skills` | free | +| 1. Does it load? | `dokimi catalog scan --root --kind skills [--strict]` | free | | 2. Is it ever picked? | `dokimi novelty routing --artifact skill: --catalog --queries queries.json [--model …]` | free without `--model` | | 3. What will step 4 cost? | `dokimi novelty analyze --artifact skill: --tier triage --include-resources --estimate-only` | free | | 4. What does it add? | the same command without `--estimate-only`, plus `--model`, `--provider`, `--judge-model` and `--max-usd` | ~100 calls | diff --git a/dokimi/changelog.d/20261021_150000_napetrov_catalog-scan-spec-review.md b/dokimi/changelog.d/20261021_150000_napetrov_catalog-scan-spec-review.md index bc3a863b..61aee1ae 100644 --- a/dokimi/changelog.d/20261021_150000_napetrov_catalog-scan-spec-review.md +++ b/dokimi/changelog.d/20261021_150000_napetrov_catalog-scan-spec-review.md @@ -7,14 +7,18 @@ with its first copy), is a `warn`. `disable-model-invocation: true` is printed as a note, since the model is never offered that skill. Each finding is printed under its skill. +- **`catalog scan --strict`** — exits 1 on a skill spec error as well as on a skill that + does not load. Without it a spec error is printed and the exit code stays 0. - **`catalog scan --format json`** — one object with a row per skill (`state` is `ok`, `warn` or `error`, `findings`, `content_sha256`, `duplicate_of`) and per task (`state` is `ok`, `skipped` or `error`), for a CI gate or a script. -### Changed +### Fixed -- **`catalog scan` exits 1 on a skill spec error** (a name that is not the directory - name or not kebab-case, a description over 1024 characters), as it already did for a - skill that does not load. Warnings (mirrors, a missing `name`) do not change the exit - code. A catalog that declares such a skill now fails the scan; fix the frontmatter - or drop the skill from the scan. +- **`catalog scan` names broken YAML frontmatter** — a `SKILL.md` whose frontmatter is not + valid YAML (a bare colon in `description:`, say) is reported as a `frontmatter-parse` + error with the YAML message, and exits 1. The scan showed only "missing a description" + before. `parse_frontmatter` is unchanged for other callers. +- **`catalog scan` without a `skills.yaml` no longer stops at the first broken skill** — each + `SKILL.md` under the root is an entry, so a skill that does not load is an error row and + the rest are still reviewed. diff --git a/dokimi/docs/external-content.md b/dokimi/docs/external-content.md index efc5aac9..d12aaf84 100644 --- a/dokimi/docs/external-content.md +++ b/dokimi/docs/external-content.md @@ -109,21 +109,30 @@ Read that output as three outcomes, not two: drive (`answer-quality` entries are question-set material, not Docker tasks). - `ERROR` — an entry that should have worked and did not. -Only errors change the exit code, so `catalog scan` exiting 0 means "everything -declared as runnable is runnable". Note the asymmetry above: a `planned` *task* +Only load errors change the exit code, so `catalog scan` exiting 0 means +"everything declared as runnable is runnable". A spec error (below) changes it +only under `--strict`. Note the asymmetry above: a `planned` *task* with no directory is skipped, while a `planned` *skill* with no directory is an error — the skill reader has no notion of a not-yet-written entry. Either drop such a skill from `skills.yaml` or expect the non-zero exit. A loaded skill is also checked against the Agent Skills spec. Its `name` must be its directory name and kebab-case (at most 64 characters), and its `description` -at most 1024 characters: a violation is an `error` and exits 1, as a load error -does. A `name` missing from the frontmatter, or a `SKILL.md` byte-identical to an -earlier skill's, is a `warn` that names the first copy; warnings do not change +at most 1024 characters: a violation is an `error`, printed under the skill. It +exits 1 only with `--strict`; without it, the scan reports the error and still +exits 0. A `name` missing from the frontmatter, or a `SKILL.md` byte-identical to +an earlier skill's, is a `warn` that names the first copy; warnings never change the exit code. `disable-model-invocation: true` is printed as a `note`, because -the model is never offered that skill. Each finding is printed under its skill. -`--format json` prints the same rows as one object (`state`, `findings`, -`content_sha256`, `duplicate_of`) for a script or a CI gate. +the model is never offered that skill. + +Frontmatter that is not valid YAML (a bare colon in `description:`, say) stops +the skill from loading, since the loader reads it as having no description. The +scan reports that as a `frontmatter-parse` error carrying the YAML message, and +it exits 1 with or without `--strict`, as a load error does. + +`--strict` makes `catalog scan` a gate on the spec as well as on loading. The +text output and `--format json` (one object: `state`, `findings`, +`content_sha256`, `duplicate_of`, for a script or a CI gate) are the same either way. Restrict the scan with `--kind`: diff --git a/dokimi/dokimi/commands/catalog.py b/dokimi/dokimi/commands/catalog.py index 27073033..987444f2 100644 --- a/dokimi/dokimi/commands/catalog.py +++ b/dokimi/dokimi/commands/catalog.py @@ -20,9 +20,10 @@ def _skill_row(skill, review) -> dict: """One skill of the scan report. ``state`` is ``error`` for a load error or an error finding, - ``warn`` for a warning finding only, else ``ok``.""" + ``warn`` for a warning finding only, else ``ok``. A skill that did not load has a review only + when its frontmatter is broken YAML, and that review's finding is its only finding.""" if not skill.loaded: - state, findings = "error", [] + state, findings = "error", (review.findings if review else []) else: state, findings = review.state, review.findings return { @@ -61,11 +62,14 @@ def cmd_catalog_scan(args: argparse.Namespace) -> None: row = _skill_row(skill, review) counts[row["state"]] += 1 report["skills"].append(row) - if row["state"] == "error": + # A load error always fails; a spec error fails only with --strict. + if not skill.loaded or (args.strict and row["state"] == "error"): any_failure = True if not as_json: - print(f"# skills ({len(skills)} declared under {root}; " - f"{counts['ok']} ok, {counts['warn']} warn, {counts['error']} error)") + print( + f"# skills ({len(skills)} declared under {root}; " + f"{counts['ok']} ok, {counts['warn']} warn, {counts['error']} error)" + ) for row in report["skills"]: if row["error"] is not None: state = f"ERROR: {row['error']}" @@ -74,11 +78,15 @@ def cmd_catalog_scan(args: argparse.Namespace) -> None: else: worst = next(f for f in row["findings"] if f["level"] == row["state"]) state = f"{'ERROR' if row['state'] == 'error' else 'warn'}: {worst['message']}" - print(f"{row['name']}\t{row['manifest_status'] or '-'}\t{row['path'] or '-'}\t{state}") + print( + f"{row['name']}\t{row['manifest_status'] or '-'}\t{row['path'] or '-'}\t{state}" + ) for f in row["findings"]: print(f" {f['level']} {f['code']}: {f['message']}") if row["disable_model_invocation"]: - print(" note disable-model-invocation: true (the model is never offered this skill)") + print( + " note disable-model-invocation: true (the model is never offered this skill)" + ) print() if args.kind in ("tasks", "all"): @@ -96,12 +104,16 @@ def cmd_catalog_scan(args: argparse.Namespace) -> None: else: state = f"ERROR: {task.reason}" any_failure = True - report["tasks"].append({ - "name": task.name, "manifest_status": task.status, "track": task.track, - "path": str(task.path) if task.path else None, - "state": "ok" if task.loaded else "skipped" if task.skipped else "error", - "reason": task.reason, - }) + report["tasks"].append( + { + "name": task.name, + "manifest_status": task.status, + "track": task.track, + "path": str(task.path) if task.path else None, + "state": "ok" if task.loaded else "skipped" if task.skipped else "error", + "reason": task.reason, + } + ) if not as_json: print(f"{task.name}\t{task.status or '-'}\t{task.track or '-'}\t{state}") @@ -210,7 +222,15 @@ def register(sub, positive_int) -> None: help="Restrict the scan to skills, tasks, or both (default: all)", ) scan_p.add_argument( - "--format", choices=["text", "json"], default="text", + "--strict", + action="store_true", + help="Exit 1 on a skill spec error (name not the directory name or not kebab-case, " + "description over 1024 characters) as well as on a skill that does not load", + ) + scan_p.add_argument( + "--format", + choices=["text", "json"], + default="text", help="text: one line per entry; json: one object with a row per skill and task (default: text)", ) scan_p.add_argument( diff --git a/dokimi/dokimi/external/catalog.py b/dokimi/dokimi/external/catalog.py index b8b22148..7b0d5336 100644 --- a/dokimi/dokimi/external/catalog.py +++ b/dokimi/dokimi/external/catalog.py @@ -29,7 +29,7 @@ from typing import Optional from dokimi.harnesses.tasks import TaskSpec, discover_tasks, is_task_dir, load_task -from dokimi.skills import Skill, discover_skills, load_skill +from dokimi.skills import Skill, load_skill DEFAULT_TASKS_SUBDIR = "evaluation/harbor/tasks" DEFAULT_SUITES_MANIFEST = "evaluation/harbor/suites.json" @@ -99,16 +99,33 @@ def read_skill_manifest(root: Path) -> list[SkillCatalogEntry]: root = Path(root) manifest_path = root / "skills.yaml" if not manifest_path.is_file(): - return [ - SkillCatalogEntry( - name=skill.name, - status=None, - path=skill.path.parent, - source_root=root, - skill=skill, + # One entry per SKILL.md, as the manifest path gives: a skill that does not + # load is an error row, so one broken skill does not abort the whole scan. + found: list[SkillCatalogEntry] = [] + for skill_file in sorted(root.rglob("SKILL.md")): + try: + skill = load_skill(skill_file) + except (FileNotFoundError, ValueError) as exc: + found.append( + SkillCatalogEntry( + name=skill_file.parent.name, + status=None, + path=skill_file.parent, + source_root=root, + error=str(exc), + ) + ) + continue + found.append( + SkillCatalogEntry( + name=skill.name, + status=None, + path=skill.path.parent, + source_root=root, + skill=skill, + ) ) - for skill in discover_skills(root) - ] + return found import yaml diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py index 76a367a8..002c611b 100644 --- a/dokimi/dokimi/external/validate.py +++ b/dokimi/dokimi/external/validate.py @@ -3,8 +3,11 @@ """Review the skills of an external catalog: spec frontmatter, and identical copies. -Runs after the readers of :mod:`dokimi.external.catalog` and only on skills -that loaded (a load error is already reported as an error by the scan). Checks +Runs after the readers of :mod:`dokimi.external.catalog`. A skill that loaded +gets every check below. A skill that did not load gets one check only: broken +YAML in its frontmatter, reported as ``frontmatter-parse`` with the YAML error, +because the loader reads broken YAML as "no description" and its load error +would otherwise hide the cause. Other load errors stay the scan's own. Checks follow the Agent Skills spec: - ``name`` is the directory name, kebab-case (lowercase letters, digits and @@ -13,6 +16,7 @@ loader falls back to the directory name silently. - ``description`` is at most 1024 characters (``error``). A missing or empty description is a load error, raised by the loader. +- The frontmatter is valid YAML (``frontmatter-parse``, ``error``). Identical copies: a ``SKILL.md`` whose bytes are the same as an earlier skill's is a ``warn`` naming that earlier skill. The model sees only the name and @@ -33,8 +37,10 @@ from dataclasses import dataclass, field from typing import Optional, Sequence +import yaml + from dokimi.skills import Skill -from dokimi.utils import parse_frontmatter +from dokimi.utils import parse_frontmatter_strict from .catalog import SkillCatalogEntry @@ -59,7 +65,9 @@ class SkillReview: findings: list[Finding] = field(default_factory=list) disable_model_invocation: bool = False content_sha256: Optional[str] = None - duplicate_of: Optional[str] = None # the skill directory of the earlier copy, relative to the catalog root + duplicate_of: Optional[str] = ( + None # the skill directory of the earlier copy, relative to the catalog root + ) @property def state(self) -> str: @@ -68,28 +76,67 @@ def state(self) -> str: return "error" if "error" in levels else "warn" if "warn" in levels else "ok" +def _strict_meta(text: str) -> tuple[dict, Optional[Finding]]: + """The frontmatter of ``text``, or a ``frontmatter-parse`` error carrying the YAML message.""" + try: + meta, _ = parse_frontmatter_strict(text) + except yaml.YAMLError as exc: + message = " ".join(str(exc).split()) # the YAML message is several lines; keep one + return {}, Finding( + "error", "frontmatter-parse", f"frontmatter is not valid YAML: {message}" + ) + return meta, None + + def _frontmatter_findings(skill: Skill) -> tuple[list[Finding], bool]: """The spec checks on the frontmatter of ``skill``, and its ``disable-model-invocation`` flag.""" - meta, _ = parse_frontmatter(skill.path.read_text(encoding="utf-8")) + meta, parse_error = _strict_meta(skill.path.read_text(encoding="utf-8")) + if parse_error is not None: + return [parse_error], False dir_name = skill.path.parent.name findings: list[Finding] = [] declared = str(meta.get("name") or "").strip() if not declared: - findings.append(Finding("warn", "name-missing", - f"no name in the frontmatter; the directory name {dir_name!r} is used")) + findings.append( + Finding( + "warn", + "name-missing", + f"no name in the frontmatter; the directory name {dir_name!r} is used", + ) + ) else: if declared != dir_name: - findings.append(Finding("error", "name-mismatch", - f"name {declared!r} is not the directory name {dir_name!r}")) + findings.append( + Finding( + "error", + "name-mismatch", + f"name {declared!r} is not the directory name {dir_name!r}", + ) + ) if len(declared) > NAME_MAX or not NAME_PATTERN.fullmatch(declared): - findings.append(Finding("error", "name-format", - f"name {declared!r} is not kebab-case (lowercase letters, digits, " - f"single hyphens; at most {NAME_MAX} characters)")) + findings.append( + Finding( + "error", + "name-format", + f"name {declared!r} is not kebab-case (lowercase letters, digits, " + f"single hyphens; at most {NAME_MAX} characters)", + ) + ) if len(skill.description) > DESCRIPTION_MAX: - findings.append(Finding("error", "description-too-long", - f"description is {len(skill.description)} characters; the limit is {DESCRIPTION_MAX}")) + findings.append( + Finding( + "error", + "description-too-long", + f"description is {len(skill.description)} characters; the limit is {DESCRIPTION_MAX}", + ) + ) # Read as the skill arm reads it (dokimi.harnesses.skill_treatment.check_skill). - hidden = str(meta.get("disable-model-invocation", "")).strip().lower() in ("1", "true", "yes", "on") + hidden = str(meta.get("disable-model-invocation", "")).strip().lower() in ( + "1", + "true", + "yes", + "on", + ) return findings, hidden @@ -98,17 +145,33 @@ def _skill_digest(skill: Skill) -> str: return hashlib.sha256(skill.path.read_bytes()).hexdigest() +def _unloaded_review(entry: SkillCatalogEntry) -> Optional[SkillReview]: + """A review for an entry that did not load, only when its ``SKILL.md`` has broken YAML; else None.""" + if entry.path is None: + return None + candidate = entry.path / "SKILL.md" if entry.path.is_dir() else entry.path + if candidate.name != "SKILL.md" or not candidate.is_file(): + return None + try: + text = candidate.read_text(encoding="utf-8") + except (OSError, UnicodeDecodeError): + return None # the loader already reported why the skill did not load + _, parse_error = _strict_meta(text) + return None if parse_error is None else SkillReview(findings=[parse_error]) + + def review_skills(entries: Sequence[SkillCatalogEntry]) -> list[Optional[SkillReview]]: - """One :class:`SkillReview` per entry, in order; None for an entry that did not load. + """One :class:`SkillReview` per entry, in order. - The first skill of each identical ``SKILL.md`` is the original; a later copy names it in - ``duplicate_of``. + None for an entry that did not load, unless its frontmatter is broken YAML: then its + review carries the ``frontmatter-parse`` error. The first skill of each identical + ``SKILL.md`` is the original; a later copy names it in ``duplicate_of``. """ reviews: list[Optional[SkillReview]] = [] first_by_digest: dict[str, Skill] = {} for entry in entries: if not entry.loaded or entry.skill is None: - reviews.append(None) + reviews.append(_unloaded_review(entry)) continue skill = entry.skill findings, hidden = _frontmatter_findings(skill) @@ -118,7 +181,12 @@ def review_skills(entries: Sequence[SkillCatalogEntry]) -> list[Optional[SkillRe if original is not skill: where = original.path.parent.relative_to(entry.source_root).as_posix() review.duplicate_of = where - review.findings.append(Finding("warn", "duplicate", - f"SKILL.md is identical to {where}; the same skill is counted twice")) + review.findings.append( + Finding( + "warn", + "duplicate", + f"SKILL.md is identical to {where}; the same skill is counted twice", + ) + ) reviews.append(review) return reviews diff --git a/dokimi/dokimi/utils.py b/dokimi/dokimi/utils.py index 939771ac..291701ed 100644 --- a/dokimi/dokimi/utils.py +++ b/dokimi/dokimi/utils.py @@ -12,7 +12,9 @@ def parse_frontmatter(text: str) -> "Tuple[Dict[str, Any], str]": Supports a leading YAML frontmatter block delimited by ``---`` lines, the same convention used by Claude Code skill files and most static-site - generators. When no frontmatter is present, returns ``({}, text)``. + generators. When no frontmatter is present, returns ``({}, text)``. Frontmatter + that is not valid YAML also returns ``{}``; use :func:`parse_frontmatter_strict` + to see that error. Args: text: Raw file contents. @@ -20,6 +22,23 @@ def parse_frontmatter(text: str) -> "Tuple[Dict[str, Any], str]": Returns: Tuple of (metadata dict, body string with frontmatter stripped). """ + return _split_frontmatter(text, strict=False) + + +def parse_frontmatter_strict(text: str) -> "Tuple[Dict[str, Any], str]": + """Like :func:`parse_frontmatter`, but frontmatter that is not valid YAML raises. + + For a validator that must name the YAML error rather than read the document + as having no metadata. The lenient :func:`parse_frontmatter` is unchanged for + every other caller. + + Raises: + yaml.YAMLError: The frontmatter block is not valid YAML. + """ + return _split_frontmatter(text, strict=True) + + +def _split_frontmatter(text: str, *, strict: bool) -> "Tuple[Dict[str, Any], str]": import yaml if not text: @@ -40,6 +59,8 @@ def parse_frontmatter(text: str) -> "Tuple[Dict[str, Any], str]": try: meta = yaml.safe_load(raw_meta) or {} except yaml.YAMLError: + if strict: + raise meta = {} if not isinstance(meta, dict): meta = {} diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py index f566b3ae..a27fdf39 100644 --- a/dokimi/tests/test_catalog_validate.py +++ b/dokimi/tests/test_catalog_validate.py @@ -9,9 +9,11 @@ from pathlib import Path import pytest +import yaml from dokimi.cli import build_parser from dokimi.external import read_skill_manifest, review_skills +from dokimi.utils import parse_frontmatter, parse_frontmatter_strict def _skill(root: Path, rel: str, name: str | None = None, description: str = "Use for things.", @@ -25,10 +27,12 @@ def _skill(root: Path, rel: str, name: str | None = None, description: str = "Us return skill_dir -def _scan_json(root: Path, capsys, *, kind: str = "skills") -> tuple[dict, int]: +def _scan_json( + root: Path, capsys, *, kind: str = "skills", strict: bool = False +) -> tuple[dict, int]: """Run ``catalog scan --format json``; return the parsed report and the exit code (0 when none raised).""" - args = build_parser().parse_args( - ["catalog", "scan", "--root", str(root), "--kind", kind, "--format", "json"]) + argv = ["catalog", "scan", "--root", str(root), "--kind", kind, "--format", "json"] + args = build_parser().parse_args(argv + (["--strict"] if strict else [])) code = 0 try: args.func(args) @@ -123,11 +127,13 @@ def test_disable_model_invocation_is_reported_as_a_flag_not_a_finding(tmp_path): assert reviews["shown"].disable_model_invocation is False -def test_json_report_carries_the_findings_and_exits_1_on_an_error(tmp_path, capsys): +def test_json_report_carries_the_findings_and_exits_0_on_a_spec_error_without_strict( + tmp_path, capsys +): _skill(tmp_path / "skills", "claude-api", name="Claude API") report, code = _scan_json(tmp_path, capsys) - assert code == 1 + assert code == 0 [row] = report["skills"] assert row["state"] == "error" assert row["error"] is None @@ -135,6 +141,23 @@ def test_json_report_carries_the_findings_and_exits_1_on_an_error(tmp_path, caps assert row["content_sha256"] and len(row["content_sha256"]) == 64 +def test_strict_makes_a_spec_error_exit_1(tmp_path, capsys): + _skill(tmp_path / "skills", "claude-api", name="Claude API") + report, code = _scan_json(tmp_path, capsys, strict=True) + + assert code == 1 + assert report["skills"][0]["state"] == "error" + + +def test_strict_does_not_fail_on_warnings(tmp_path, capsys): + _skill(tmp_path / "plugins", "x", name="x") + _skill(tmp_path / "skills", "x", name="x") + _skill(tmp_path / "skills", "no-name", name=None) + _, code = _scan_json(tmp_path, capsys, strict=True) + + assert code == 0 + + def test_json_report_on_warnings_only_exits_0(tmp_path, capsys): _skill(tmp_path / "plugins", "x", name="x") _skill(tmp_path / "skills", "x", name="x") @@ -163,11 +186,47 @@ def test_json_report_keeps_load_errors_as_error_rows(tmp_path, capsys): def test_text_output_lists_each_finding_under_its_skill(tmp_path, capsys): _skill(tmp_path / "skills", "claude-api", name="Claude API") args = build_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 + args.func(args) # no SystemExit: a spec error is printed, and only --strict makes it fail out = capsys.readouterr().out assert "# skills (1 declared" in out and "1 error" in out assert "ERROR: name 'Claude API' is not the directory name" in out assert " error name-mismatch:" in out assert " error name-format:" in out + + +BARE_COLON = "Use when: the user asks for a Claude API call" + + +def test_frontmatter_with_a_bare_colon_is_a_frontmatter_parse_error(tmp_path): + # ``description: Use when: ...`` is not valid YAML; the lenient parser reads it as no metadata. + _skill(tmp_path / "skills", "claude-api", name="claude-api", description=BARE_COLON) + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.state == "error" + [finding] = review.findings + assert finding.code == "frontmatter-parse" + assert finding.level == "error" + assert finding.message.startswith( + "frontmatter is not valid YAML: mapping values are not allowed" + ) + assert "\n" not in finding.message + + +def test_broken_yaml_is_reported_in_place_of_the_missing_description_error(tmp_path, capsys): + _skill(tmp_path / "skills", "claude-api", name="claude-api", description=BARE_COLON) + report, code = _scan_json(tmp_path, capsys) + + assert code == 1 # a skill that does not load exits 1 with or without --strict + [row] = report["skills"] + assert row["state"] == "error" + assert "missing a 'description'" in row["error"] + assert [f["code"] for f in row["findings"]] == ["frontmatter-parse"] + assert row["content_sha256"] is None and row["duplicate_of"] is None + + +def test_strict_parse_raises_and_the_lenient_parse_still_returns_no_metadata(): + text = f"---\nname: claude-api\ndescription: {BARE_COLON}\n---\nbody\n" + + assert parse_frontmatter(text) == ({}, "body") + with pytest.raises(yaml.YAMLError): + parse_frontmatter_strict(text) From 7e1d17e811c01beda9d31ec9931f377a536d3bdf Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Thu, 22 Oct 2026 12:34:04 +0000 Subject: [PATCH 3/8] Catch OSError when loading a skill so unreadable files become error rows Signed-off-by: Anton Volkov --- dokimi/dokimi/external/catalog.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dokimi/dokimi/external/catalog.py b/dokimi/dokimi/external/catalog.py index 7b0d5336..2c6b6098 100644 --- a/dokimi/dokimi/external/catalog.py +++ b/dokimi/dokimi/external/catalog.py @@ -105,7 +105,7 @@ def read_skill_manifest(root: Path) -> list[SkillCatalogEntry]: for skill_file in sorted(root.rglob("SKILL.md")): try: skill = load_skill(skill_file) - except (FileNotFoundError, ValueError) as exc: + except (OSError, ValueError) as exc: found.append( SkillCatalogEntry( name=skill_file.parent.name, @@ -201,7 +201,7 @@ def failed(error: str, path: Optional[Path] = None) -> SkillCatalogEntry: return failed(f"{source} escapes root: {catalog_path!r}") try: skill = load_skill(path) - except (FileNotFoundError, ValueError) as exc: + except (OSError, ValueError) as exc: return failed(str(exc), path) # The directory is inside the root; a SKILL.md symlinked out of it is not. if not _within(skill.path, root): From 1ed1cfb75ce69803008c18cec17f02697bb0f8dd Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Thu, 22 Oct 2026 12:34:35 +0000 Subject: [PATCH 4/8] Update scan example header and explain the broken-YAML row Signed-off-by: Anton Volkov --- dokimi/docs/external-content.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/dokimi/docs/external-content.md b/dokimi/docs/external-content.md index d12aaf84..e4aa9c8d 100644 --- a/dokimi/docs/external-content.md +++ b/dokimi/docs/external-content.md @@ -89,7 +89,7 @@ to pay to run is half-declared. ```console $ python cli.py catalog scan --root C:/Users/Public/content-repo -# skills (2 declared under C:\Users\Public\content-repo) +# skills (2 declared under C:\Users\Public\content-repo; 1 ok, 0 warn, 1 error) demo-skill published C:\Users\Public\content-repo\skills\demo-skill ok not-written-yet planned C:\Users\Public\content-repo\skills\not-written-yet ERROR: Skill path does not exist: C:\Users\Public\content-repo\skills\not-written-yet @@ -128,7 +128,10 @@ the model is never offered that skill. Frontmatter that is not valid YAML (a bare colon in `description:`, say) stops the skill from loading, since the loader reads it as having no description. The scan reports that as a `frontmatter-parse` error carrying the YAML message, and -it exits 1 with or without `--strict`, as a load error does. +it exits 1 with or without `--strict`, as a load error does. The YAML message is +on the indented `frontmatter-parse` line under the skill (and in its `findings` +in JSON); the row's main line and its JSON `error` field still say the skill is +missing a `description`. `--strict` makes `catalog scan` a gate on the spec as well as on loading. The text output and `--format json` (one object: `state`, `findings`, From 67b683e2244e90df16584c962ff2a836ed70519c Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Thu, 22 Oct 2026 08:06:21 -0700 Subject: [PATCH 5/8] fix(catalog): stringify manifest status in JSON; skip name-mismatch for a root SKILL.md An unquoted YAML date as status crashed catalog scan --format json. A SKILL.md directly in the scan root was compared with the root directory name and failed --strict. Adds the review's mutation-gap tests. Signed-off-by: Nikolay Petrov --- dokimi/dokimi/commands/catalog.py | 4 +- dokimi/dokimi/external/validate.py | 17 ++-- dokimi/tests/test_catalog_validate.py | 114 ++++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 7 deletions(-) diff --git a/dokimi/dokimi/commands/catalog.py b/dokimi/dokimi/commands/catalog.py index 987444f2..54195f85 100644 --- a/dokimi/dokimi/commands/catalog.py +++ b/dokimi/dokimi/commands/catalog.py @@ -28,7 +28,7 @@ def _skill_row(skill, review) -> dict: state, findings = review.state, review.findings return { "name": skill.name, - "manifest_status": skill.status, + "manifest_status": None if skill.status is None else str(skill.status), "path": str(skill.path) if skill.path else None, "state": state, "error": skill.error, @@ -107,7 +107,7 @@ def cmd_catalog_scan(args: argparse.Namespace) -> None: report["tasks"].append( { "name": task.name, - "manifest_status": task.status, + "manifest_status": None if task.status is None else str(task.status), "track": task.track, "path": str(task.path) if task.path else None, "state": "ok" if task.loaded else "skipped" if task.skipped else "error", diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py index 002c611b..c9e068c5 100644 --- a/dokimi/dokimi/external/validate.py +++ b/dokimi/dokimi/external/validate.py @@ -88,8 +88,12 @@ def _strict_meta(text: str) -> tuple[dict, Optional[Finding]]: return meta, None -def _frontmatter_findings(skill: Skill) -> tuple[list[Finding], bool]: - """The spec checks on the frontmatter of ``skill``, and its ``disable-model-invocation`` flag.""" +def _frontmatter_findings(skill: Skill, *, at_root: bool = False) -> tuple[list[Finding], bool]: + """The spec checks on the frontmatter of ``skill``, and its ``disable-model-invocation`` flag. + + ``at_root``: the ``SKILL.md`` sits directly in the scanned root, so its directory name is the + name of a checkout, which says nothing about the skill; the name/directory comparison is skipped. + """ meta, parse_error = _strict_meta(skill.path.read_text(encoding="utf-8")) if parse_error is not None: return [parse_error], False @@ -101,11 +105,12 @@ def _frontmatter_findings(skill: Skill) -> tuple[list[Finding], bool]: Finding( "warn", "name-missing", - f"no name in the frontmatter; the directory name {dir_name!r} is used", + "no name in the frontmatter" + + ("" if at_root else f"; the directory name {dir_name!r} is used"), ) ) else: - if declared != dir_name: + if declared != dir_name and not at_root: findings.append( Finding( "error", @@ -174,7 +179,9 @@ def review_skills(entries: Sequence[SkillCatalogEntry]) -> list[Optional[SkillRe reviews.append(_unloaded_review(entry)) continue skill = entry.skill - findings, hidden = _frontmatter_findings(skill) + findings, hidden = _frontmatter_findings( + skill, at_root=skill.path.parent == entry.source_root + ) review = SkillReview(findings=findings, disable_model_invocation=hidden) review.content_sha256 = _skill_digest(skill) original = first_by_digest.setdefault(review.content_sha256, skill) diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py index a27fdf39..785a7453 100644 --- a/dokimi/tests/test_catalog_validate.py +++ b/dokimi/tests/test_catalog_validate.py @@ -230,3 +230,117 @@ def test_strict_parse_raises_and_the_lenient_parse_still_returns_no_metadata(): assert parse_frontmatter(text) == ({}, "body") with pytest.raises(yaml.YAMLError): parse_frontmatter_strict(text) + + +# --- review follow-up: edges of the name rules, duplicates, state, JSON rows ------------------------- + + +@pytest.mark.parametrize("name", ["a" * 64, "a1-2b"]) +def test_valid_names_matching_the_directory_have_no_finding(tmp_path, name): + _skill(tmp_path / "skills", name, name=name) + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.findings == [] + + +@pytest.mark.parametrize("name", ["a" * 65, "a--b", "a-", "-a", "a_b", "a b"]) +def test_malformed_names_matching_the_directory_only_fail_name_format(tmp_path, name): + _skill(tmp_path / "skills", name, name=f"'{name}'") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert [f.code for f in review.findings] == ["name-format"] + + +def test_same_frontmatter_with_a_different_body_is_not_a_duplicate(tmp_path): + _skill(tmp_path / "plugins", "x", name="x", body="# One\n") + _skill(tmp_path / "skills", "x", name="x", body="# Two\n") + + assert all(r.duplicate_of is None for r in review_skills(read_skill_manifest(tmp_path))) + + +def test_a_trailing_newline_difference_is_not_a_duplicate(tmp_path): + _skill(tmp_path / "plugins", "x", name="x", body="# Body\n") + _skill(tmp_path / "skills", "x", name="x", body="# Body\n\n") + + assert all(r.duplicate_of is None for r in review_skills(read_skill_manifest(tmp_path))) + + +def test_three_identical_copies_all_name_the_first(tmp_path): + for parent in ("a-plugins", "b-plugins", "c-plugins"): + _skill(tmp_path / parent, "x", name="x") + + reviews = review_skills(read_skill_manifest(tmp_path)) + assert reviews[0].duplicate_of is None + assert [r.duplicate_of for r in reviews[1:]] == ["a-plugins/x", "a-plugins/x"] + + +def test_an_error_outranks_a_warning_in_the_state_and_fails_strict(tmp_path, capsys): + _skill(tmp_path / "plugins", "Bad", name="Bad") + _skill(tmp_path / "skills", "Bad", name="Bad") + + reviews = review_skills(read_skill_manifest(tmp_path)) + mirror = reviews[1] + assert {f.level for f in mirror.findings} == {"warn", "error"} + assert mirror.state == "error" + _, code = _scan_json(tmp_path, capsys, strict=True) + assert code == 1 + + +def test_disable_model_invocation_false_does_not_hide_the_skill(tmp_path): + _skill(tmp_path / "skills", "shown", name="shown", extra="disable-model-invocation: false\n") + + [review] = review_skills(read_skill_manifest(tmp_path)) + assert review.disable_model_invocation is False + + +def test_json_report_with_an_unquoted_date_status_does_not_crash(tmp_path, capsys): + _skill(tmp_path / "skills", "a", name="a") + (tmp_path / "skills.yaml").write_text( + "skills:\n - name: a\n status: 2026-01-01\n catalog_path: skills/a\n", + encoding="utf-8", + ) + report, code = _scan_json(tmp_path, capsys) + + assert code == 0 + assert report["skills"][0]["manifest_status"] == "2026-01-01" + + +def test_skill_at_the_scan_root_is_not_compared_with_the_root_directory_name(tmp_path, capsys): + root = tmp_path / "checkout" + root.mkdir() + (root / "SKILL.md").write_text( + "---\nname: z\ndescription: Use for z.\n---\n# z\n", encoding="utf-8" + ) + report, code = _scan_json(root, capsys, strict=True) + + assert code == 0 + assert report["skills"][0]["state"] == "ok" + assert report["skills"][0]["findings"] == [] + + +def test_json_tasks_array_carries_loaded_and_skipped_rows(tmp_path, capsys): + _skill(tmp_path / "skills", "s", name="s") + task = tmp_path / "evaluation" / "harbor" / "tasks" / "real" + for sub in ("environment", "tests", "solution"): + (task / sub).mkdir(parents=True) + (task / "instruction.md").write_text("Do it.", encoding="utf-8") + (task / "task.toml").write_text( + '[metadata]\ndifficulty = "easy"\n\n[agent]\ntimeout_sec = 60\n\n' + "[verifier]\ntimeout_sec = 10\n\n[environment]\n", + encoding="utf-8", + ) + (tmp_path / "skills.yaml").write_text( + 'skills:\n - name: s\n catalog_path: skills/s\n', encoding="utf-8") + (tmp_path / "evaluation" / "harbor" / "suites.json").write_text( + json.dumps({"schema_version": "1.0", "suites": [{"skill": "s", "tasks": [ + {"name": "real", "status": "implemented", "track": "executable"}, + {"name": "later", "status": "planned", "track": "executable"}]}]}), + encoding="utf-8", + ) + report, code = _scan_json(tmp_path, capsys, kind="all") + + assert code == 0 + rows = {r["name"]: r for r in report["tasks"]} + assert rows["real"]["state"] == "ok" and rows["real"]["track"] == "executable" + assert rows["real"]["manifest_status"] == "implemented" + assert rows["later"]["state"] == "skipped" and rows["later"]["reason"] From 9ec812d04118ca38c5ad5f230e0969ec754c5aa6 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Fri, 23 Oct 2026 10:42:30 +0000 Subject: [PATCH 6/8] Note that a root SKILL.md skips the directory-name check in docs Signed-off-by: Anton Volkov --- dokimi/docs/external-content.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dokimi/docs/external-content.md b/dokimi/docs/external-content.md index e4aa9c8d..b49aa339 100644 --- a/dokimi/docs/external-content.md +++ b/dokimi/docs/external-content.md @@ -117,7 +117,7 @@ error — the skill reader has no notion of a not-yet-written entry. Either drop such a skill from `skills.yaml` or expect the non-zero exit. A loaded skill is also checked against the Agent Skills spec. Its `name` must be -its directory name and kebab-case (at most 64 characters), and its `description` +its directory name and kebab-case (at most 64 characters; a `SKILL.md` directly in the scanned root is not compared with the root directory's name), and its `description` at most 1024 characters: a violation is an `error`, printed under the skill. It exits 1 only with `--strict`; without it, the scan reports the error and still exits 0. A `name` missing from the frontmatter, or a `SKILL.md` byte-identical to From a3bb3c41d11870be75ae56a3fd85e4aa1aa4f137 Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Fri, 23 Oct 2026 12:45:34 -0700 Subject: [PATCH 7/8] refactor(catalog): split cmd_catalog_scan and read_skill_manifest under the complexity ratchet Signed-off-by: Nikolay Petrov --- dokimi/dokimi/commands/catalog.py | 148 +++++++++++++++------------- dokimi/dokimi/external/catalog.py | 71 +++++++------ dokimi/scripts/growth_baseline.json | 1 - 3 files changed, 122 insertions(+), 98 deletions(-) diff --git a/dokimi/dokimi/commands/catalog.py b/dokimi/dokimi/commands/catalog.py index 27a982ce..67e36f25 100644 --- a/dokimi/dokimi/commands/catalog.py +++ b/dokimi/dokimi/commands/catalog.py @@ -39,10 +39,86 @@ def _skill_row(skill, review) -> dict: } -def cmd_catalog_scan(args: argparse.Namespace) -> None: - from dokimi.external import read_skill_manifest, read_task_manifest +def _print_skill_row(row: dict) -> None: + if row["error"] is not None: + state = f"ERROR: {row['error']}" + elif row["state"] == "ok": + state = "ok" + else: + worst = next(f for f in row["findings"] if f["level"] == row["state"]) + state = f"{'ERROR' if row['state'] == 'error' else 'warn'}: {worst['message']}" + print(f"{row['name']}\t{row['manifest_status'] or '-'}\t{row['path'] or '-'}\t{state}") + for f in row["findings"]: + print(f" {f['level']} {f['code']}: {f['message']}") + if row["disable_model_invocation"]: + print(" note disable-model-invocation: true (the model is never offered this skill)") + + +def _scan_skills(args: argparse.Namespace, root: Path, report: dict, as_json: bool) -> bool: + """Review the skills under *root* into ``report``; True when the scan must exit 1.""" + from dokimi.external import read_skill_manifest from dokimi.external.validate import review_skills + try: + skills = read_skill_manifest(root) + except ValueError as exc: + raise SystemExit(f"skills.yaml: {exc}") from exc + reviews = review_skills(skills) + counts = {"ok": 0, "warn": 0, "error": 0} + failed = False + for skill, review in zip(skills, reviews, strict=True): + row = _skill_row(skill, review) + counts[row["state"]] += 1 + report["skills"].append(row) + # A load error always fails; a spec error fails only with --strict. + if not skill.loaded or (args.strict and row["state"] == "error"): + failed = True + if not as_json: + print( + f"# skills ({len(skills)} declared under {root}; " + f"{counts['ok']} ok, {counts['warn']} warn, {counts['error']} error)" + ) + for row in report["skills"]: + _print_skill_row(row) + print() + return failed + + +def _task_row(task) -> dict: + return { + "name": task.name, + "manifest_status": None if task.status is None else str(task.status), + "track": task.track, + "path": str(task.path) if task.path else None, + "state": "ok" if task.loaded else "skipped" if task.skipped else "error", + "reason": task.reason, + } + + +def _scan_tasks(args: argparse.Namespace, root: Path, report: dict, as_json: bool) -> bool: + """List the tasks under *root* into ``report``; True when the scan must exit 1.""" + from dokimi.external import read_task_manifest + + try: + tasks = read_task_manifest(root, tasks_subdir=args.tasks_subdir) + except ValueError as exc: + raise SystemExit(f"suites.json: {exc}") from exc + if not as_json: + print(f"# tasks ({len(tasks)} declared under {root})") + failed = False + for task in tasks: + row = _task_row(task) + report["tasks"].append(row) + failed = failed or row["state"] == "error" + if not as_json: + state = {"ok": "ok", "skipped": f"skipped: {task.reason}"}.get( + row["state"], f"ERROR: {task.reason}" + ) + print(f"{task.name}\t{task.status or '-'}\t{task.track or '-'}\t{state}") + return failed + + +def cmd_catalog_scan(args: argparse.Namespace) -> None: root = Path(args.root) if not root.is_dir(): raise SystemExit(f"--root is not a directory: {root}") @@ -50,76 +126,12 @@ def cmd_catalog_scan(args: argparse.Namespace) -> None: as_json = args.format == "json" report: dict = {"root": str(root), "skills": [], "tasks": []} any_failure = False - if args.kind in ("skills", "all"): - try: - skills = read_skill_manifest(root) - except ValueError as exc: - raise SystemExit(f"skills.yaml: {exc}") from exc - reviews = review_skills(skills) - counts = {"ok": 0, "warn": 0, "error": 0} - for skill, review in zip(skills, reviews, strict=True): - row = _skill_row(skill, review) - counts[row["state"]] += 1 - report["skills"].append(row) - # A load error always fails; a spec error fails only with --strict. - if not skill.loaded or (args.strict and row["state"] == "error"): - any_failure = True - if not as_json: - print( - f"# skills ({len(skills)} declared under {root}; " - f"{counts['ok']} ok, {counts['warn']} warn, {counts['error']} error)" - ) - for row in report["skills"]: - if row["error"] is not None: - state = f"ERROR: {row['error']}" - elif row["state"] == "ok": - state = "ok" - else: - worst = next(f for f in row["findings"] if f["level"] == row["state"]) - state = f"{'ERROR' if row['state'] == 'error' else 'warn'}: {worst['message']}" - print( - f"{row['name']}\t{row['manifest_status'] or '-'}\t{row['path'] or '-'}\t{state}" - ) - for f in row["findings"]: - print(f" {f['level']} {f['code']}: {f['message']}") - if row["disable_model_invocation"]: - print( - " note disable-model-invocation: true (the model is never offered this skill)" - ) - print() - + any_failure = _scan_skills(args, root, report, as_json) if args.kind in ("tasks", "all"): - try: - tasks = read_task_manifest(root, tasks_subdir=args.tasks_subdir) - except ValueError as exc: - raise SystemExit(f"suites.json: {exc}") from exc - if not as_json: - print(f"# tasks ({len(tasks)} declared under {root})") - for task in tasks: - if task.loaded: - state = "ok" - elif task.skipped: - state = f"skipped: {task.reason}" - else: - state = f"ERROR: {task.reason}" - any_failure = True - report["tasks"].append( - { - "name": task.name, - "manifest_status": None if task.status is None else str(task.status), - "track": task.track, - "path": str(task.path) if task.path else None, - "state": "ok" if task.loaded else "skipped" if task.skipped else "error", - "reason": task.reason, - } - ) - if not as_json: - print(f"{task.name}\t{task.status or '-'}\t{task.track or '-'}\t{state}") - + any_failure = _scan_tasks(args, root, report, as_json) or any_failure if as_json: print(json.dumps(report, indent=2)) - if any_failure: raise SystemExit(1) diff --git a/dokimi/dokimi/external/catalog.py b/dokimi/dokimi/external/catalog.py index 2c6b6098..92c23ec9 100644 --- a/dokimi/dokimi/external/catalog.py +++ b/dokimi/dokimi/external/catalog.py @@ -93,40 +93,45 @@ def loaded(self) -> bool: return self.task is not None and not self.skipped and not self.failed -def read_skill_manifest(root: Path) -> list[SkillCatalogEntry]: - """Read skills under *root*, preferring a ``skills.yaml`` manifest.""" +def _scan_skill_files(root: Path) -> list[SkillCatalogEntry]: + """One entry per ``SKILL.md`` under *root*, as the manifest path gives. - root = Path(root) - manifest_path = root / "skills.yaml" - if not manifest_path.is_file(): - # One entry per SKILL.md, as the manifest path gives: a skill that does not - # load is an error row, so one broken skill does not abort the whole scan. - found: list[SkillCatalogEntry] = [] - for skill_file in sorted(root.rglob("SKILL.md")): - try: - skill = load_skill(skill_file) - except (OSError, ValueError) as exc: - found.append( - SkillCatalogEntry( - name=skill_file.parent.name, - status=None, - path=skill_file.parent, - source_root=root, - error=str(exc), - ) - ) - continue + A skill that does not load is an error row, so one broken skill does not abort the scan. + """ + found: list[SkillCatalogEntry] = [] + for skill_file in sorted(root.rglob("SKILL.md")): + try: + skill = load_skill(skill_file) + except (OSError, ValueError) as exc: found.append( SkillCatalogEntry( - name=skill.name, + name=skill_file.parent.name, status=None, - path=skill.path.parent, + path=skill_file.parent, source_root=root, - skill=skill, + error=str(exc), ) ) - return found + continue + found.append( + SkillCatalogEntry( + name=skill.name, + status=None, + path=skill.path.parent, + source_root=root, + skill=skill, + ) + ) + return found + + +def _manifest_items(manifest_path: Path) -> tuple[list, bool]: + """The entries of ``skills.yaml`` and whether the file is the list shape. + uxlfoundation/skills nests the entries under ``skills:``, each with a required + catalog_path; intel/skills makes the whole file the list and keeps every skill at + skills/. + """ import yaml try: @@ -134,9 +139,6 @@ def read_skill_manifest(root: Path) -> list[SkillCatalogEntry]: except yaml.YAMLError as exc: raise ValueError(f"invalid skills.yaml at {manifest_path}: {exc}") from exc - # uxlfoundation/skills nests the entries under ``skills:``, each with a - # required catalog_path; intel/skills makes the whole file the list and - # keeps every skill at skills/. if isinstance(data, dict): items = data.get("skills") if items is None: @@ -155,7 +157,18 @@ def read_skill_manifest(root: Path) -> list[SkillCatalogEntry]: f"invalid skills.yaml at {manifest_path}: 'skills' must be a list, " f"got {type(items).__name__}" ) + return items, list_shape + + +def read_skill_manifest(root: Path) -> list[SkillCatalogEntry]: + """Read skills under *root*, preferring a ``skills.yaml`` manifest.""" + + root = Path(root) + manifest_path = root / "skills.yaml" + if not manifest_path.is_file(): + return _scan_skill_files(root) + items, list_shape = _manifest_items(manifest_path) entries: list[SkillCatalogEntry] = [] for index, item in enumerate(items): if not isinstance(item, dict): diff --git a/dokimi/scripts/growth_baseline.json b/dokimi/scripts/growth_baseline.json index 974dd0e6..7f3a9515 100644 --- a/dokimi/scripts/growth_baseline.json +++ b/dokimi/scripts/growth_baseline.json @@ -20,7 +20,6 @@ "dokimi/commands/arms.py::_validate_expected_skills": {"complexity": 11}, "dokimi/commands/benchmark.py::cmd_benchmark_batch": {"complexity": 11}, "dokimi/commands/benchmark.py::cmd_benchmark_preflight": {"complexity": 22}, - "dokimi/commands/catalog.py::cmd_catalog_scan": {"complexity": 12}, "dokimi/commands/catalog.py::cmd_catalog_trigger_set": {"complexity": 18}, "dokimi/commands/evaluate.py::cmd_eval_judge_validity": {"complexity": 29, "rationale": "grew on main in #73 (judge-validity --context stored) from 25 to 29 before the ratchet landed; split it in its own PR"}, "dokimi/commands/judge_reliability.py::cmd_eval_cross_judge": {"complexity": 32, "rationale": "pre-existing when the gate was introduced"}, From 29e767f6911c305ddd9dda15425f8076b5f022fa Mon Sep 17 00:00:00 2001 From: Nikolay Petrov Date: Fri, 23 Oct 2026 12:45:34 -0700 Subject: [PATCH 8/8] fix(catalog): compare skill paths resolved so a symlinked root does not crash the duplicate check; pin root-skill rules Signed-off-by: Nikolay Petrov --- dokimi/dokimi/external/validate.py | 15 +++++- dokimi/tests/test_catalog_validate.py | 74 +++++++++++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/dokimi/dokimi/external/validate.py b/dokimi/dokimi/external/validate.py index c9e068c5..d46ffe37 100644 --- a/dokimi/dokimi/external/validate.py +++ b/dokimi/dokimi/external/validate.py @@ -35,6 +35,7 @@ import hashlib import re from dataclasses import dataclass, field +from pathlib import Path from typing import Optional, Sequence import yaml @@ -165,6 +166,16 @@ def _unloaded_review(entry: SkillCatalogEntry) -> Optional[SkillReview]: return None if parse_error is None else SkillReview(findings=[parse_error]) +def _relative_to_root(path: Path, root: Path) -> str: + """``path`` relative to ``root`` (compared as written, then resolved), else as written.""" + for a, b in ((path, root), (path.resolve(), root.resolve())): + try: + return a.relative_to(b).as_posix() + except ValueError: + continue + return path.as_posix() + + def review_skills(entries: Sequence[SkillCatalogEntry]) -> list[Optional[SkillReview]]: """One :class:`SkillReview` per entry, in order. @@ -180,13 +191,13 @@ def review_skills(entries: Sequence[SkillCatalogEntry]) -> list[Optional[SkillRe continue skill = entry.skill findings, hidden = _frontmatter_findings( - skill, at_root=skill.path.parent == entry.source_root + skill, at_root=skill.path.parent.resolve() == entry.source_root.resolve() ) review = SkillReview(findings=findings, disable_model_invocation=hidden) review.content_sha256 = _skill_digest(skill) original = first_by_digest.setdefault(review.content_sha256, skill) if original is not skill: - where = original.path.parent.relative_to(entry.source_root).as_posix() + where = _relative_to_root(original.path.parent, entry.source_root) review.duplicate_of = where review.findings.append( Finding( diff --git a/dokimi/tests/test_catalog_validate.py b/dokimi/tests/test_catalog_validate.py index 785a7453..9b550032 100644 --- a/dokimi/tests/test_catalog_validate.py +++ b/dokimi/tests/test_catalog_validate.py @@ -344,3 +344,77 @@ def test_json_tasks_array_carries_loaded_and_skipped_rows(tmp_path, capsys): assert rows["real"]["state"] == "ok" and rows["real"]["track"] == "executable" assert rows["real"]["manifest_status"] == "implemented" assert rows["later"]["state"] == "skipped" and rows["later"]["reason"] + + +def _root_skill(root: Path, frontmatter: str) -> None: + root.mkdir(exist_ok=True) + (root / "SKILL.md").write_text(f"---\n{frontmatter}---\n# z\n", encoding="utf-8") + + +def test_root_skill_still_gets_the_name_format_check(tmp_path, capsys): + root = tmp_path / "checkout" + _root_skill(root, "name: Z_z\ndescription: Use for z.\n") + report, _ = _scan_json(root, capsys) + + assert [f["code"] for f in report["skills"][0]["findings"]] == ["name-format"] + + +def test_root_skill_without_a_name_does_not_cite_the_checkout_directory(tmp_path, capsys): + root = tmp_path / "checkout" + _root_skill(root, "description: Use for z.\n") + report, _ = _scan_json(root, capsys) + + [finding] = report["skills"][0]["findings"] + assert finding["code"] == "name-missing" + assert "directory name" not in finding["message"] + assert "checkout" not in finding["message"] + + +def test_a_skill_without_a_manifest_status_has_a_null_status_in_json(tmp_path, capsys): + _skill(tmp_path / "skills", "a", name="a") + report, _ = _scan_json(tmp_path, capsys) + + assert report["skills"][0]["manifest_status"] is None + + +def test_a_task_without_a_manifest_status_has_a_null_status_in_json(tmp_path, capsys): + _skill(tmp_path / "skills", "s", name="s") + (tmp_path / "skills.yaml").write_text( + 'skills:\n - name: s\n catalog_path: skills/s\n', encoding="utf-8") + (tmp_path / "evaluation" / "harbor").mkdir(parents=True) + (tmp_path / "evaluation" / "harbor" / "suites.json").write_text( + json.dumps({"schema_version": "1.0", "suites": [{"skill": "s", "tasks": [ + {"name": "later", "track": "executable"}]}]}), + encoding="utf-8", + ) + report, _ = _scan_json(tmp_path, capsys, kind="tasks") + + assert [t["manifest_status"] for t in report["tasks"]] == [None] + + +def test_a_duplicate_found_through_a_symlinked_root_is_a_row_not_a_traceback(tmp_path, capsys): + real = tmp_path / "real" + for name in ("a", "b"): + _skill(real / "skills", name, name="same") + (real / "skills.yaml").write_text( + f"skills:\n - name: a\n catalog_path: {real / 'skills' / 'a'}\n" + " - name: b\n catalog_path: skills/b\n", encoding="utf-8") + link = tmp_path / "link" + link.symlink_to(real, target_is_directory=True) + report, _ = _scan_json(link, capsys) + + second = next(s for s in report["skills"] if s["duplicate_of"]) + assert second["duplicate_of"].endswith("skills/a") + assert "duplicate" in {f["code"] for f in second["findings"]} + + +def test_a_root_skill_reached_by_an_equivalent_spelling_skips_the_directory_check( + tmp_path, capsys): + root = tmp_path / "checkout" + _root_skill(root, "name: zzz\ndescription: Use for z.\n") + (root / "sub").mkdir() + (root / "skills.yaml").write_text( + "skills:\n - name: zzz\n catalog_path: sub/..\n", encoding="utf-8") + report, _ = _scan_json(root, capsys) + + assert report["skills"][0]["findings"] == []