feat(catalog): review skills against the Agent Skills spec, add --strict and --format json

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 <catalog> --kind skills` | free |
+| 1. Does it load? | `dokimi catalog scan --root <catalog> --kind skills [--strict]` | free |
 | 2. Is it ever picked? | `dokimi novelty routing --artifact skill:<dir> --catalog <catalog> --queries queries.json [--model …]` | free without `--model` |
 | 3. What will step 4 cost? | `dokimi novelty analyze --artifact skill:<dir> --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 |

dokimi/changelog.d/20261021_150000_napetrov_catalog-scan-spec-review.md

@@ -0,0 +1,24 @@
+### 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 --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.
+
+### Fixed
+
+- **`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.

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
 
@@ -109,12 +109,34 @@ 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; 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
+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.
+
+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. 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`,
+`content_sha256`, `duplicate_of`, for a script or a CI gate) are the same either way.
+
 Restrict the scan with `--kind`:
 
 ```console

dokimi/dokimi/commands/catalog.py

@@ -13,51 +13,125 @@
 from __future__ import annotations
 
 import argparse
+import json
 import sys
 from pathlib import Path
 
 
-def cmd_catalog_scan(args: argparse.Namespace) -> None:
-    from dokimi.external import read_skill_manifest, read_task_manifest
+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``. 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", (review.findings if review else [])
+    else:
+        state, findings = review.state, review.findings
+    return {
+        "name": skill.name,
+        "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,
+        "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 _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}")
 
+    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
-        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:
-                any_failure = True
-        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
-        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
-            track = task.track or "-"
-            status = task.status or "-"
-            print(f"{task.name}\t{status}\t{track}\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)
 
@@ -160,6 +234,18 @@ 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(
+        "--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(
         "--tasks-subdir", default="evaluation/harbor/tasks",
         help="Task directory relative to --root (default: evaluation/harbor/tasks, "

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",
 ]

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"
@@ -93,33 +93,52 @@ 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():
-        return [
+    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_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
+
+
+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/<name>.
+    """
     import yaml
 
     try:
         data = yaml.safe_load(manifest_path.read_text(encoding="utf-8")) or {}
     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/<name>.
     if isinstance(data, dict):
         items = data.get("skills")
         if items is None:
@@ -138,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):
@@ -184,7 +214,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):