[napetrov]

Part of #4.

Summary

catalog scan now reviews each loaded skill against the Agent Skills spec, and the report is machine-readable.

 catalog scan --root <catalog>
   read_skill_manifest(root)              # loader, unchanged in shape
+  review_skills(skills)                  # new: external/validate.py
+    name == directory name, kebab-case, <= 64 chars   -> error
+    description <= 1024 chars                          -> error
+    name missing from frontmatter                      -> warn
+    SKILL.md byte-identical to an earlier skill        -> warn (names first copy)
+    disable-model-invocation: true                     -> note, not a finding
+  exit 1 on a load error                  (as before)
+  exit 1 on a spec error only with --strict
+  --format json                           one object: state, findings, content_sha256, duplicate_of

Spec errors are reported and do not change the exit code unless --strict is given, so existing scripts keep their exit status. Broken YAML frontmatter is named (frontmatter-parse, with the YAML message) instead of showing as a missing description. Without a skills.yaml, the scan now reviews every SKILL.md under the root, and a skill that does not load becomes an error row instead of aborting the scan.

Evidence

The new tests live in tests/test_catalog_validate.py (17 tests). The CLI-level tests call the catalog scan command, so they show the old behaviour directly.

before: commands/catalog.py reverted to intel/main, rest of this branch kept
  FAILED test_strict_makes_a_spec_error_exit_1
  FAILED test_strict_does_not_fail_on_warnings
  FAILED test_json_report_on_warnings_only_exits_0
  FAILED test_json_report_keeps_load_errors_as_error_rows
  FAILED test_json_report_carries_the_findings_and_exits_0_on_a_spec_error_without_strict
  FAILED test_text_output_lists_each_finding_under_its_skill
  FAILED test_broken_yaml_is_reported_in_place_of_the_missing_description_error
  7 failed, 10 passed

after: this branch, full suite below (includes tests/test_catalog_validate.py)

Local checks on the rebased head:

uv run ruff check .                              All checks passed!
uv run mypy dokimi                               Success: no issues found in 223 source files
uv run python -m dokimi.schema_check             exit 0
uv run python -m dokimi.config_check             exit 0
uv run pytest -n auto -q                         6896 passed, 22 skipped

Merge Danger

Door: two-way. The new flags and the JSON object are additive, and the exit code without --strict is unchanged for every case that exited 0 before. Reverting the commits restores the old scan.

Blast Radius: low, confined to catalog scan.

Behaviour changes for callers:

• Per-skill error rows. Without --strict, a spec error prints as an ERROR row and the scan still exits 0. Users who want a failing gate must pass --strict.
• Text output. The header now reads # skills (N declared under ...; x ok, y warn, z error), and finding lines are indented under their skill. Parsers of the old header line need updating.
• Scan without skills.yaml. It now enumerates every SKILL.md under the root (rglob), so vendored or mirrored copies appear as rows, and mirrors are reported as warn.
• Broken frontmatter. A SKILL.md with invalid YAML exits 1, with or without --strict, as a load error does.
• No persisted-format or schema change. parse_frontmatter keeps its lenient behaviour; the strict variant is a new function used only by the validator.

Checklist

• Tests added/updated and pytest passes
• ruff, mypy, and schema checks pass
• Docs updated: docs/external-content.md (scan section) and the README step-1 command
• No ad-hoc run artifacts committed
• New code and test files carry the SPDX license header (the changelog fragment follows the existing fragments, which carry none)

🤖 Generated with Claude Code

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 93.90863% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.03%. Comparing base (09be35b) to head (a213659).

Files with missing lines	Patch %	Lines
dokimi/dokimi/external/validate.py	91.91%	5 Missing and 3 partials ⚠️
dokimi/dokimi/commands/catalog.py	94.11%	3 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #163      +/-   ##
==========================================
+ Coverage   92.01%   92.03%   +0.01%     
==========================================
  Files         223      224       +1     
  Lines       27768    27929     +161     
  Branches     4563     4588      +25     
==========================================
+ Hits        25552    25705     +153     
- Misses       1570     1575       +5     
- Partials      646      649       +3     

Files with missing lines	Coverage Δ
dokimi/dokimi/external/__init__.py	100.00% <100.00%> (ø)
dokimi/dokimi/external/catalog.py	97.18% <100.00%> (+0.35%)	⬆️
dokimi/dokimi/utils.py	98.00% <100.00%> (+0.12%)	⬆️
dokimi/dokimi/commands/catalog.py	97.45% <94.11%> (-0.86%)	⬇️
dokimi/dokimi/external/validate.py	91.91% <91.91%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[antonwolfy]

Review at 1868f79. The loader changes and the exit-code split between load errors and --strict hold up. The problems are at the edges: one JSON crash, one false --strict failure, and tests that leave the name rules unpinned. Fix the first two inline comments before merge.

What I ran, on a Linux review host at 1868f79:

• make ci-local: 6,897 tests passed, 21 skipped; the changed-line coverage step, schema-check, the changelog check, the wheel smoke tests and gitleaks pass. The ruff, mypy, deptry, bandit, zizmor, pre-commit, mkdocs and pip-audit steps could not run in my venv: make dev stops at the tools install on this base, which #179 fixed on main. Static checks, mypy and the strict docs build pass on GitHub's CI at this head.
• Reproductions for the code findings, on fixture catalogs, with no model or network involved.
• A mutation run on validate.py and commands/catalog.py: 80 mutants, 48 killed, 32 survive the 17 tests in test_catalog_validate.py (7 of the survivors are equivalent).

Test gaps (the code is correct today; these changes would pass the suite):

1. The name rules beyond one title-case name are not pinned. These mutants of validate.py:116 all pass the suite: > to >= on NAME_MAX, NAME_MAX = 200, the length check removed, a regex that allows a--b, a trailing -?, _ allowed, fullmatch replaced by match. A 65-character name, a--b, a-, a_b would then be ok. Suggestion: a parametrized test where name and directory match, so only name-format fires: "a"*64 and "a1-2b" give no finding; "a"*65, "a--b", "a-", "-a", "a_b", "a b" give ["name-format"].
2. Duplicate detection is not pinned to whole-file bytes and to the first copy. Hashing only the frontmatter, hashing stripped text, and "latest copy wins" all pass. The existing test changes the description, so the frontmatter differs. Suggestion: same frontmatter with a different body is not a duplicate; a trailing-newline difference is not a duplicate; three identical copies all name the first, which has duplicate_of is None.
3. state precedence and disable-model-invocation: false are untested. Swapping the warn and error ranks in SkillReview.state passes, so a mirror with a bad name would be warn and pass --strict. Reading the flag with bool(meta.get(...)) passes, so false would hide a skill. Suggestion: a bad-name skill that is also a mirror expects state == "error" and --strict exit 1; a disable-model-invocation: false skill expects the flag False.
4. The JSON tasks array has no content test. Every JSON test uses --kind skills or asserts report["tasks"] == []. Renaming track or reason, mapping skipped to ok, dropping the append, or printing the # tasks header in JSON mode all pass. Suggestion: one --kind all --format json run on a catalog with a loaded and a skipped task; parse the whole stdout and assert the task rows.

[antonwolfy]

I pushed a commit per nit, up to 1ed1cfb, which fix the two nits from my review:

• 7e1d17e: dokimi/dokimi/external/catalog.py:108: catch OSError instead of FileNotFoundError in the SKILL.md scan and in _skill_entry (:204), keeping ValueError.
• 1ed1cfb: dokimi/docs/external-content.md:92: the example header now shows the ok/warn/error counts, and a sentence at :130 says the YAML message is on the frontmatter-parse line while the row's main line and JSON error still say the description is missing.

Left as is:

• The four test-gap points are not labelled Nit: or Non-blocking: in the review body, so they stay with the author.

make ci-local at 1ed1cfb: 20 passed, 8 failed (all exit 127, tools missing on the host), 7 skipped.

Not approving yet. Still open: the bug and the two risks from my review.

[napetrov]

Test gaps from the review body, all added in 67b683e (tests/test_catalog_validate.py):

1. Name rules: test_valid_names_matching_the_directory_have_no_finding, test_malformed_names_matching_the_directory_only_fail_name_format (65 chars, a--b, a-, -a, a_b, a b).
2. Duplicates: test_same_frontmatter_with_a_different_body_is_not_a_duplicate, test_a_trailing_newline_difference_is_not_a_duplicate, test_three_identical_copies_all_name_the_first.
3. State and flag: test_an_error_outranks_a_warning_in_the_state_and_fails_strict, test_disable_model_invocation_false_does_not_hide_the_skill.
4. JSON tasks: test_json_tasks_array_carries_loaded_and_skipped_rows (--kind all, one loaded and one skipped task).
   I did not re-run the mutation run.

[antonwolfy]

Review at 67b683e. Both earlier threads are fixed and tested. One new crash in the duplicate check is worth fixing before merge, plus a test gap and a doc nit, three points in all, all inline.

What I ran, on a Linux review host at 67b683e:

• make ci-local: 27 passed, 1 failed, 7 skipped. 6,933 tests passed, 1 skipped; the changed-line coverage step, schema-check, ruff, mypy, deptry, bandit, zizmor, the strict docs build, the wheel smoke tests and gitleaks pass. The one failure is pip-audit on the lock files, which this PR does not touch.
• Reproductions for the code finding, on fixture catalogs, with no model or network involved.
• A mutation run on validate.py and commands/catalog.py: 56 mutants, 41 killed, 15 survive. The name-rule, duplicate, state and --strict mutants are all killed now. The survivors that matter are in the test-gap comment.

Checked, no issues

• JSON status: date, float and bool status values come out as strings in the skill and task rows, and --format json stdout parses as one object with and without --strict.
• Root SKILL.md: with a plain scan and with catalog_path: ., name-mismatch is skipped and name-format still applies.
• Exit codes: a non-UTF-8 file, a missing description and broken YAML each exit 1 as a load error. A spec error alone exits 0 without --strict.

[antonwolfy]

I pushed a commit per nit, up to 9ec812d, which fix the nit from my review:

• 9ec812d: dokimi/docs/external-content.md:120: added that a SKILL.md directly in the scanned root is not compared with the root directory's name.

make ci-local at 9ec812d: 27 passed, 1 failed (pip-audit, same as at the reviewed head), 7 skipped.

GitHub's Tests fail at 9ec812d on every Python version, in test_growth_ratchet.py: commands/catalog.py::cmd_catalog_scan has complexity 21 against its baseline of 12, and external/catalog.py::read_skill_manifest has 11 with no baseline entry (the cap is 10). The commit changes only external-content.md. The ratchet came to main (#93) after this PR's earlier green run, and the PR's changes to those two functions take them past it. Splitting the two functions makes Tests green again.

Not approving yet. Still open: the bug and the risk from my review.

[napetrov]

Tests were red on the growth ratchet (cmd_catalog_scan complexity 21 vs baseline 12, read_skill_manifest 11 vs cap 10). a3bb3c4 splits both into helpers (behaviour unchanged, the catalog tests pass) and drops the cmd_catalog_scan baseline entry. The branch was also merged with main in 5ec24f7.

[github-actions]

CLI interface change detected

This PR changes dokimi's user-facing CLI surface (commands and/or options):

~ `dokimi catalog scan` parameters changed:
    + added option `--format`
    + added option `--strict`

Updated on every push; the cli-interface-change label is on while the diff is non-empty.

[xaleryb]

• Ran: the full suite, ruff, mypy and bandit on 29e767f merged with current main, plus catalog scan on anthropics/skills and small fixtures.
• Semantic conflict with fix(skills): discovery skips template dirs and counts a mirror once #167: a manifest-less catalog scan lists template/SKILL.md again. On anthropics/skills that gives 20 rows, including template-skill ERROR name-mismatch, so --strict fails on the scaffold. Expected: 19 rows. The duplicate warning also lands on skills/foo, the copy discover_skills keeps, when it should land on plugin/skills/foo.
• A symlinked SKILL.md outside --root with broken YAML: one line of the outside file appears in the text and JSON frontmatter-parse message. Expected: no content from outside the root.
• Pushed:
  • f3c8fbc: skips TEMPLATE_DIR_NAMES and makes the copy with the fewest path parts the original (docs and the fragment are updated).
  • 0b92036: adds a containment check before _unloaded_review reads the file.
  • 4e295ca: tests pinning the default --kind all exit code and the OSError error row.
  • a213659: merges main, since CI on this head never ran with fix(skills): discovery skips template dirs and counts a mirror once #167.
• For the author: for a skills.yaml catalog, the duplicate original is now the copy nearest the root, not the first one declared.

[xaleryb]

Approved at a213659. Every earlier point re-run at this head passes, no must-fix or mid item is open, and CI is green.