lint: skill-inventory parity across top-level dirs, skills/ symlinks, CLAUDE.md table, and marketplace.json (#809) (#810)

Closes #809.

check_skill_inventory_parity.py: on-disk <name>/SKILL.md dirs are the
authority; set-equality against skills/ symlinks, the CLAUDE.md Skills
Overview table (exact unfenced H2, GFM header+separator, first-cell
backticked names), and marketplace.json plugins[].skills; "N skills"
count claims on plugin.json / marketplace.json / MODE_REGISTRY.md.
check_spec_consistency.py derives its skill paths from disk at call
time; table-row grammar single-sourced in _skill_lint. 60 mutation
tests; wired into spec-consistency.yml and the pytest manifest.

Dual-track pre-ship: /simplify (4 findings applied), /security-review
(none), codex gpt-5.6-sol xhigh 7 rounds (12 findings fixed, round 7
clean). CHANGELOG also covers #805.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014U5nvjKy84twtB4VYsrex1
This commit is contained in:
Edward Cheng-I Wu
2026-08-31 02:02:28 +08:00
committed by GitHub
parent 37bd060294
commit e8bf858be7
9 changed files with 854 additions and 13 deletions
+10
View File
@@ -90,6 +90,16 @@ jobs:
PYTHONPATH: scripts
run: python3 scripts/check_data_access_level.py
- name: Check skill-inventory parity (#809)
# Top-level <name>/SKILL.md dirs == skills/ symlinks == CLAUDE.md
# Skills Overview rows == marketplace.json plugins[].skills, plus the
# "N skills" count claims in plugin/marketplace descriptions. The pytest
# companion `test_check_skill_inventory_parity.py` runs via the unified
# manifest.
env:
PYTHONPATH: scripts
run: python3 scripts/check_skill_inventory_parity.py
- name: Check instruction-vs-data boundary (#272 guidance layer)
# Drift guard for the retrieved-content instruction/data principle:
# authoritative canonical block in ground_truth_isolation_pattern.md +
+6
View File
@@ -4,8 +4,14 @@ All notable changes to this project will be documented in this file.
## [Unreleased]
### Added
- **Skill-inventory parity lint (#809).** New `scripts/check_skill_inventory_parity.py` takes the top-level `<name>/SKILL.md` directories as the authority and requires set-equality against the three surfaces that package or advertise the inventory: `skills/<name>` symlinks (each must resolve to `../<name>`), the `.claude/CLAUDE.md` Skills Overview table rows, and `.claude-plugin/marketplace.json` `plugins[].skills[]` (`./<name>` form). It also checks that any "N skills" count claim on the three current-state metadata surfaces (`plugin.json` / `marketplace.json` descriptions, `MODE_REGISTRY.md`) equals the number of skills on disk; README and CHANGELOG are out of scope because their release notes carry legitimately frozen historical counts. The table-row grammar moves to `_skill_lint` (`SKILLS_TABLE_ROW_PREFIX` / `SKILLS_TABLE_ROW_FULL`) so this lint and `check_version_consistency.py` agree on what a row is, and a row that names a skill but lacks its `vX.Y.Z` token is reported here rather than silently skipped by the version lint; and `check_spec_consistency.py` now derives its skill list from disk instead of the hardcoded four-path tuple, so a new skill directory is version-policed the moment it exists. Motivation: triage of an external draft PR that added a fifth top-level skill directory showed the existing lints are anchored to the four skills they already know (`check_spec_consistency.py` hardcodes the paths; `check_version_consistency.py` iterates the CLAUDE.md table), so an unpackaged, unlisted skill passed every inventory lint. Wired into `spec-consistency.yml`; 60 mutation tests cover each surface in both directions, dangling / mis-targeted / non-symlink `skills/` entries, malformed manifest entries, and stale count claims.
### Fixed
- **MLA key-rules line in `citation_format_switcher.md` no longer reads "No year in in-text" (#805, by @LeslieLi46).** Docs-only; the line now matches the in-text format documented above it.
- **`check_surface_form_parity` names the broken environment instead of blaming the manifest (#801 follow-up).** With the manifest file present but pyyaml unimportable, `_load_manifest` returned `None`, so the lint failed with "manifest … is present but empty / null / non-mapping" — a misdiagnosis pointing the reader at the wrong file (pyyaml is declared in `requirements-dev.txt`, so only a broken local environment can hit this). The missing-import case is now a distinct `_YamlUnavailableError` and the lint fails naming pyyaml and the `pip install -r requirements-dev.txt` remedy; regression test pins that the message names the environment, not the manifest shape. Also de-enumerated the hand-maintained dependency parenthetical in `docs/SETUP.md` / `docs/SETUP.zh-TW.md` Method line ("PyYAML + jsonschema" was already stale against the manifest it mirrors two lines above); both language files move together.
- **`normalize_cn_title` strips outer wrapper marks only when they enclose the whole title as one balanced unit (#800).** The wrapper strip inherited from #431 (and promoted unchanged by #798/#799) was positional: it removed the first and last characters whenever they matched as a wrapper pair *type*, without checking they belonged to the same bracket pair. `《红楼梦》与《金瓶梅》` — two titles joined in one string — therefore normalized to `红楼梦》与《金瓶梅`, leaving an orphaned `》` mid-key. Matching correctness was never affected (both sides of every comparison pass through the same normalization, and no exploitable asymmetry was found in the #799 security pass), but the mangled key is a semantic anomaly for any future single-sided consumer (display, logging, cache keys, or comparison against an externally-normalized key). Fix adds `_outer_pair_encloses`: the outer marks are stripped only when the interior between them is itself balanced under all six wrapper pairs, so `《围城》` still strips to `围城` and nested balanced interiors (`《基于「ProEXC」的研究》``基于「ProEXC」的研究`) still unwrap, while `《红楼梦》与《金瓶梅》` and `“研究”与“实践”` keep their marks. The interior scan is scoped to the outer pair's **own family**: `` is also the English apostrophe and `”` also appears unpaired, so a family-blind scan read the lone `` in `《Alzheimers病中表达》` as an unbalanced quote and refused to strip a genuine `《…》` wrap — dropping a pair that matched before to exact=False and ratio 0.6818, below the 0.70 floor, which fails the DOI-keyed ratio gate and the title-fallback exact gate at once and is the failure class #798 repaired. Scoping costs the check nothing, since any mark that can orphan the outer pair is by definition of that pair's own family. Both consumers change together — the CJK client re-imports the shared function (#799), pinned behaviorally as well as by identity. Scope of the invariance claim, stated precisely: on the two `has_cjk`-gated paths (`exact_normalized_title`'s third branch and `_similarity`'s CJK fold) every verdict and ratio for titles without Han ideographs is unchanged, pinned by the #799 pre-fix oracles. The client's `_cn_titles_match` is **not** `has_cjk`-gated — it calls `normalize_cn_title` directly — so a mark-carrying title with no Han ideograph can change verdict there (`《Hamlet》and《Macbeth》` no longer matches a pre-mangled `Hamlet》and《Macbeth`); that path is DOI-keyed and Chinese-corpus-only in practice, so the narrowing is accepted rather than gated. Likewise the empty-wrapper guard is a property of the CJK branch specifically (`_cjk_titles_match` requires a non-empty key): `exact_normalized_title("《》", "《》")` remains True through the ungated base-normalization branch, as it did before this change.
+4
View File
@@ -204,6 +204,10 @@ path = "scripts/test_check_risk_register.py"
id = "756-data-access-level-pins"
path = "scripts/test_check_data_access_level.py"
[[pytest]]
id = "809-skill-inventory-parity"
path = "scripts/test_check_skill_inventory_parity.py"
[[pytest]]
id = "v3.10-182-verification-cache"
path = "scripts/test_verification_cache.py"
+13
View File
@@ -29,6 +29,19 @@ from pathlib import Path
import yaml
# One row of the `.claude/CLAUDE.md` § "Skills Overview" table. The first cell is
# the backticked skill directory name followed by its `vX.Y.Z` token. Two forms
# are shared so the lints agree on what a row is:
# PREFIX — name only. check_skill_inventory_parity.py uses it to find every
# row that names a skill, so a row can never hide from the parity
# check by omitting its version.
# FULL — name + version. check_version_consistency.py parses versions with
# it; the parity lint reports any PREFIX row that is not also a FULL
# row, closing the gap where a version-less row is invisible to the
# version lint (it only iterates FULL matches).
SKILLS_TABLE_ROW_PREFIX = r"^\|\s*`([a-z0-9-]+)`"
SKILLS_TABLE_ROW_FULL = SKILLS_TABLE_ROW_PREFIX + r"\s+v([A-Za-z0-9.\-_+]+)\s*\|"
SKIP_DIRS = frozenset(
{"shared", "scripts", "docs", ".git", ".github", "examples", ".local-plans", ".claude"}
)
+355
View File
@@ -0,0 +1,355 @@
#!/usr/bin/env python3
"""Lint: the skill inventory is identical on every surface that lists it (#809).
Triage of an external PR that added a fifth top-level skill directory showed
that the existing inventory lints are anchored to the skills they already
know about: `check_spec_consistency.py` hardcodes four SKILL.md paths,
`check_version_consistency.py` iterates the `.claude/CLAUDE.md` table, and
nothing cross-checks the `skills/` symlink directory or the marketplace
manifest. A skill could therefore exist on disk without being packaged,
listed, or versioned, with every lint green.
This lint takes the set of top-level `<name>/SKILL.md` directories as the
authority (it is what exists) and requires set-equality against the three
surfaces that advertise or package the inventory:
B. `skills/<name>` — one symlink per skill, resolving to `<root>/<name>`
(plugin auto-discovery packages from here);
C. `.claude/CLAUDE.md` § "Skills Overview" table rows (the canonical
inventory other lints iterate);
D. `.claude-plugin/marketplace.json` `plugins[].skills[]` as `./<name>`
(what symlink-blind importers read).
It also checks that any "<N> skills" count claim equals the number of skills
on disk, on the three CURRENT-STATE metadata surfaces that carry one:
`.claude-plugin/plugin.json` description, `marketplace.json` top-level and
per-plugin descriptions, and `MODE_REGISTRY.md`. README and CHANGELOG are deliberately out of scope: they
carry historical release notes whose counts are legitimately frozen at the
time of writing, so a prose-wide grep would flag correct history. A surface
that carries no count makes no claim and is not checked.
Every asymmetric difference is reported in both directions, so a stale row
and an unpackaged directory are both single, named violations.
"""
from __future__ import annotations
import argparse
import json
import re
import sys
from pathlib import Path
from _skill_lint import (
SKILLS_TABLE_ROW_FULL,
SKILLS_TABLE_ROW_PREFIX,
heading_section,
iter_skill_files,
)
SKILLS_DIR = "skills"
CLAUDE_MD = Path(".claude") / "CLAUDE.md"
MARKETPLACE_JSON = Path(".claude-plugin") / "marketplace.json"
PLUGIN_JSON = Path(".claude-plugin") / "plugin.json"
MODE_REGISTRY_MD = Path("MODE_REGISTRY.md")
SKILLS_OVERVIEW_HEADING = "## Skills Overview"
# Row grammar shared with check_version_consistency.py. FULL is what the version
# lint iterates; a row that names a skill but is not FULL is reported here (the
# version lint would silently skip it). PREFIX is kept imported so the shared
# grammar has a visible second consumer.
TABLE_ROW_FULL_RE = re.compile(SKILLS_TABLE_ROW_FULL)
# Any backticked first cell at all, so a row with a non-canonical name (e.g.
# `Ghost-Skill`) is reported instead of being invisible to both lints.
TABLE_ROW_ANY_RE = re.compile(r"^\|\s*`([^`]+)`")
CANONICAL_NAME_RE = re.compile(r"[a-z0-9-]+")
# Markdown table separator row: `|---|:---:|`, `|:-|` etc. (GFM: one or more dashes).
TABLE_SEPARATOR_RE = re.compile(r"^\|\s*:?-+:?\s*(\|\s*:?-+:?\s*)*\|?\s*$")
# Marketplace skill paths are relative, `./<name>`, one segment.
MANIFEST_SKILL_RE = re.compile(r"\./([a-z0-9-]+)") # used with fullmatch: `$` would admit a trailing newline
# A count claim such as "4 skills" / "4 Skills" (word-bounded so "40 skillsets"
# is not one; case-insensitive because these are prose surfaces).
COUNT_CLAIM_RE = re.compile(r"\b(\d+) skills\b", re.IGNORECASE)
def _skills_on_disk(root: Path) -> set[str]:
return {p.parent.name for p in iter_skill_files(root)}
def _skills_dir_entries(root: Path, violations: list[str]) -> set[str]:
skills_dir = root / SKILLS_DIR
if not skills_dir.is_dir():
violations.append(f"{skills_dir}: directory is missing")
return set()
names: set[str] = set()
for entry in sorted(skills_dir.iterdir(), key=lambda p: p.name):
names.add(entry.name)
expected = root / entry.name
if not entry.is_symlink():
violations.append(
f"{entry}: must be a symlink to ../{entry.name}, "
f"found a real {'directory' if entry.is_dir() else 'file'}"
)
continue
try:
target = entry.resolve(strict=True)
except (FileNotFoundError, RuntimeError):
violations.append(f"{entry}: dangling symlink")
continue
if target != expected.resolve():
violations.append(
f"{entry}: symlink resolves to {target}, expected {expected}"
)
return names
def _overview_table_lines(section: str) -> list[str]:
"""The ONE table that immediately follows the heading: skip leading blank
lines, then take the contiguous run of `|`-prefixed lines. A later table,
prose mention, or fenced Markdown sample in the same section is not the
inventory and must not satisfy the parity check."""
lines = iter(section.splitlines())
table: list[str] = []
for line in lines:
if line.strip() == "":
if table:
break
continue
if not line.lstrip().startswith("|"):
break
table.append(line)
return table
def _cell_count(row: str) -> int:
"""Cells in a GFM table row: strip one leading and one trailing pipe,
then split on unescaped pipes."""
inner = row.strip()
inner = inner[1:] if inner.startswith("|") else inner
inner = inner[:-1] if inner.endswith("|") else inner
return len(re.split(r"(?<!\\)\|", inner))
def _claude_table_rows(root: Path, violations: list[str]) -> set[str]:
claude_md = root / CLAUDE_MD
if not claude_md.is_file():
violations.append(f"{claude_md}: file is missing")
return set()
text = claude_md.read_text(encoding="utf-8")
# Exact H2 line at column 0, outside code fences (shared helper): a
# demoted `### Skills Overview` or a copy inside a fenced example is not
# the section.
section = heading_section(text, SKILLS_OVERVIEW_HEADING)
if section is None:
violations.append(
f"{claude_md}: '{SKILLS_OVERVIEW_HEADING}' H2 is missing"
)
return set()
rows: set[str] = set()
table = _overview_table_lines(section)
# A GFM table is header row + separator row + data rows. Both leading
# rows are required (a table without its separator is not a table, and
# the version lint would still read its rows). Every data row must carry
# a backticked skill name in its first cell; one that does not (e.g.
# `| ghost-skill v1.0.0 |`) is reported rather than dropped.
if len(table) < 2 or not TABLE_SEPARATOR_RE.match(table[1]):
violations.append(
f"{claude_md}: '{SKILLS_OVERVIEW_HEADING}' is not followed by a GFM "
f"table (header row then a |---| separator row)"
)
return set()
header_cells, separator_cells = _cell_count(table[0]), _cell_count(table[1])
if header_cells != separator_cells:
# GFM: header and delimiter rows must have the same cell count or
# GitHub does not render a table at all. Data-row width is NOT
# checked: GFM pads/truncates data rows, and this lint reads only
# the first cell, as check_version_consistency.py does.
violations.append(
f"{claude_md}: Skills Overview header has {header_cells} cells but "
f"its separator row has {separator_cells}; GFM requires them equal"
)
return set()
data_rows = table[2:]
for line in data_rows:
any_row = TABLE_ROW_ANY_RE.match(line)
if any_row is None:
violations.append(
f"{claude_md}: Skills Overview data row {line.strip()!r} has no "
f"backticked skill name in its first cell"
)
continue
name = any_row.group(1)
if CANONICAL_NAME_RE.fullmatch(name) is None:
violations.append(
f"{claude_md}: Skills Overview row names {name!r}, which is not "
f"a canonical skill directory name ([a-z0-9-]+); both lints "
f"would otherwise ignore this row"
)
continue
rows.add(name)
if TABLE_ROW_FULL_RE.match(line) is None:
violations.append(
f"{claude_md}: Skills Overview row for '{name}' lacks a "
f"'vX.Y.Z' token after the name (check_version_consistency.py "
f"skips such rows, so the version would go unchecked)"
)
if not rows:
violations.append(
f"{claude_md}: '{SKILLS_OVERVIEW_HEADING}' table has no "
f"backticked skill rows"
)
return rows
def _load_json(path: Path, violations: list[str]) -> dict | None:
if not path.is_file():
violations.append(f"{path}: file is missing")
return None
try:
data = json.loads(path.read_text(encoding="utf-8"))
except json.JSONDecodeError as exc:
violations.append(f"{path}: invalid JSON ({exc.msg} at line {exc.lineno})")
return None
if not isinstance(data, dict):
violations.append(f"{path}: top level must be an object")
return None
return data
def _marketplace_skills(
root: Path, violations: list[str]
) -> tuple[set[str], list[str]]:
"""Return (skill names across all plugins, plugin descriptions)."""
path = root / MARKETPLACE_JSON
data = _load_json(path, violations)
if data is None:
return set(), []
plugins = data.get("plugins")
if not isinstance(plugins, list) or not plugins:
violations.append(f"{path}: 'plugins' must be a non-empty list")
return set(), []
names: set[str] = set()
descriptions: list[str] = []
if isinstance(data.get("description"), str):
descriptions.append(data["description"]) # marketplace-level claim
for index, plugin in enumerate(plugins):
if not isinstance(plugin, dict):
violations.append(f"{path}: plugins[{index}] must be an object")
continue
description = plugin.get("description")
if isinstance(description, str):
descriptions.append(description)
skills = plugin.get("skills")
if not isinstance(skills, list):
violations.append(
f"{path}: plugins[{index}].skills must be a list of './<name>' "
f"paths (symlink-blind importers read this list)"
)
continue
for raw in skills:
match = MANIFEST_SKILL_RE.fullmatch(raw) if isinstance(raw, str) else None
if match is None:
violations.append(
f"{path}: plugins[{index}].skills entry {raw!r} must be "
f"'./<name>' with a single lowercase path segment"
)
continue
names.add(match.group(1))
return names, descriptions
def _check_count_claim(
label: str, text: str, expected: int, violations: list[str]
) -> None:
for claimed in COUNT_CLAIM_RE.findall(text):
if int(claimed) != expected:
violations.append(
f"{label}: claims '{claimed} skills' but {expected} "
f"top-level skill directories exist"
)
def _report_set_diff(
on_disk: set[str], other: set[str], surface: str, violations: list[str]
) -> None:
for name in sorted(on_disk - other):
violations.append(
f"skill '{name}' exists on disk (top-level {name}/SKILL.md) but "
f"is not listed in {surface}"
)
for name in sorted(other - on_disk):
violations.append(
f"{surface} lists skill '{name}' but no top-level {name}/SKILL.md "
f"exists"
)
def run_all_checks(root: Path) -> list[str]:
violations: list[str] = []
on_disk = _skills_on_disk(root)
if not on_disk:
violations.append(
f"{root}: no top-level <name>/SKILL.md found (wrong --path?)"
)
return violations
symlinked = _skills_dir_entries(root, violations)
_report_set_diff(on_disk, symlinked, f"{SKILLS_DIR}/ symlinks", violations)
table = _claude_table_rows(root, violations)
_report_set_diff(
on_disk, table, f"{CLAUDE_MD} Skills Overview table", violations
)
manifest, market_descriptions = _marketplace_skills(root, violations)
_report_set_diff(
on_disk, manifest, f"{MARKETPLACE_JSON} plugins[].skills", violations
)
for description in market_descriptions:
_check_count_claim(
f"{MARKETPLACE_JSON} description", description, len(on_disk), violations
)
plugin = _load_json(root / PLUGIN_JSON, violations)
if plugin is not None:
description = plugin.get("description")
if isinstance(description, str):
_check_count_claim(
f"{PLUGIN_JSON} description", description, len(on_disk), violations
)
registry = root / MODE_REGISTRY_MD
if registry.is_file():
_check_count_claim(
str(MODE_REGISTRY_MD),
registry.read_text(encoding="utf-8"),
len(on_disk),
violations,
)
else:
violations.append(f"{registry}: file is missing")
return violations
def main() -> int:
parser = argparse.ArgumentParser()
parser.add_argument(
"--path",
type=Path,
default=Path(__file__).resolve().parent.parent,
)
args = parser.parse_args()
violations = run_all_checks(args.path)
if violations:
for v in violations:
print(f"ERROR: {v}")
print(f"\n{len(violations)} violation(s) found.", file=sys.stderr)
return 1
print(
"OK: skill inventory is identical across top-level directories, "
"skills/ symlinks, the CLAUDE.md table, and the marketplace manifest."
)
return 0
if __name__ == "__main__":
sys.exit(main())
+14 -9
View File
@@ -13,11 +13,13 @@ if __package__: # Package import in tests.
NON_RELATIVE_LINK_PREFIXES,
extract_link_targets,
)
from ._skill_lint import iter_skill_files
else: # pragma: no cover - exercised by the CLI smoke path
from _markdown_lint_util import (
NON_RELATIVE_LINK_PREFIXES,
extract_link_targets,
)
from _skill_lint import iter_skill_files
ROOT = Path(__file__).resolve().parents[1]
@@ -95,14 +97,17 @@ def check_claude_md() -> None:
expect_absent(rel_path, forbidden)
# All four skills carry the same frontmatter (`version` / `last_updated`) + Version-Info-table
# (`| Skill Version |` / `| Last Updated |`) pair. Pre-#377 only the reviewer was policed.
_SKILL_VERSION_PATHS = (
"academic-pipeline/SKILL.md",
"academic-paper/SKILL.md",
"academic-paper-reviewer/SKILL.md",
"deep-research/SKILL.md",
)
# Every top-level skill carries the same frontmatter (`version` / `last_updated`) +
# Version-Info-table (`| Skill Version |` / `| Last Updated |`) pair. Pre-#377 only the
# reviewer was policed. Derived from disk (#809) rather than hand-listed, so a new skill
# directory is policed the moment it exists; check_skill_inventory_parity.py pins that
# the on-disk set matches every surface that advertises it.
def _skill_version_paths() -> tuple[str, ...]:
"""Read ROOT at call time (tests swap `csc.ROOT` for fixture trees; an
import-time tuple would carry the real checkout's skills into them)."""
return tuple(
f"{skill_md.parent.name}/SKILL.md" for skill_md in iter_skill_files(ROOT)
)
# The single skill whose `version` tracks the suite version. The other three move independently,
# so only this one's date is sanity-checked against the release (CHANGELOG) in #377(b).
@@ -139,7 +144,7 @@ def _parse_skill_version_block(rel_path: str) -> tuple[str, str, str, str] | Non
def check_skill_version_blocks() -> None:
"""#377(a): for ALL FOUR SKILL.md, the frontmatter version/last_updated must match the
Version-Info-table rows (an internal per-file consistency check)."""
for rel_path in _SKILL_VERSION_PATHS:
for rel_path in _skill_version_paths():
parsed = _parse_skill_version_block(rel_path)
if parsed is None:
continue
+2 -4
View File
@@ -59,7 +59,7 @@ from pathlib import Path
import yaml
from _skill_lint import parse_frontmatter, FrontmatterError
from _skill_lint import SKILLS_TABLE_ROW_FULL, parse_frontmatter, FrontmatterError
# Broad token captures: anything that looks like an identifier inside the
@@ -67,9 +67,7 @@ from _skill_lint import parse_frontmatter, FrontmatterError
# token is a canonical semver. Using the regex as a filter (the pre-#169
# pattern) silently dropped invalid tokens and hid the very drift this lint
# is meant to surface; see dual-track review on PR for that class of bug.
TABLE_TOKEN_RE = re.compile(
r"^\|\s*`([a-z0-9-]+)`\s+v([A-Za-z0-9.\-_+]+)\s*\|", re.MULTILINE
)
TABLE_TOKEN_RE = re.compile(SKILLS_TABLE_ROW_FULL, re.MULTILINE)
SUITE_TOKEN_RE = re.compile(
r"^\s*-\s*\*\*Suite version\*\*:\s*([A-Za-z0-9.\-_+]+)", re.MULTILINE
)
@@ -0,0 +1,438 @@
"""Mutation tests for check_skill_inventory_parity.py (#809).
A synthetic root carries two skills on all four surfaces. Each test breaks
exactly one surface in one direction and asserts the lint names it. The
real repository tree is checked last.
"""
from __future__ import annotations
import json
import os
from pathlib import Path
import pytest
from tests.test_helpers import run_skill_linter, write_skill
from check_skill_inventory_parity import run_all_checks
SCRIPT = Path(__file__).resolve().parent / "check_skill_inventory_parity.py"
REPO_ROOT = Path(__file__).resolve().parent.parent
NAMES = ("alpha-skill", "beta-skill")
def _write_table(root: Path, names: tuple[str, ...]) -> None:
rows = "\n".join(f"| `{n}` v1.0.0 | purpose | full |" for n in names)
(root / ".claude").mkdir(exist_ok=True)
(root / ".claude" / "CLAUDE.md").write_text(
"# Test\n\n## Skills Overview\n\n"
"| Skill | Purpose | Key Modes |\n|-------|---------|-----------|\n"
f"{rows}\n\n## Routing Rules\n\n"
"| `not-a-skill` v9.9.9 | a row outside the section | x |\n",
encoding="utf-8",
)
def _write_manifests(
root: Path,
names: tuple[str, ...],
*,
market_entries: list[str] | None = None,
market_desc: str | None = None,
market_top_desc: str | None = None,
plugin_desc: str | None = None,
) -> None:
(root / ".claude-plugin").mkdir(exist_ok=True)
entries = market_entries if market_entries is not None else [f"./{n}" for n in names]
market = {
"name": "test",
"description": market_top_desc if market_top_desc is not None else "test marketplace",
"plugins": [
{
"name": "test",
"description": market_desc if market_desc is not None else f"{len(names)} skills + modes",
"skills": entries,
}
],
}
(root / ".claude-plugin" / "marketplace.json").write_text(
json.dumps(market, indent=2), encoding="utf-8"
)
plugin = {
"name": "test",
"description": plugin_desc if plugin_desc is not None else f"pipeline: {len(names)} skills, 9 modes",
}
(root / ".claude-plugin" / "plugin.json").write_text(
json.dumps(plugin, indent=2), encoding="utf-8"
)
def _write_symlinks(root: Path, names: tuple[str, ...]) -> None:
skills = root / "skills"
skills.mkdir(exist_ok=True)
for n in names:
os.symlink(f"../{n}", skills / n)
def _write_registry(root: Path, text: str) -> None:
(root / "MODE_REGISTRY.md").write_text(text, encoding="utf-8")
def _build_root(root: Path, names: tuple[str, ...] = NAMES) -> None:
for n in names:
write_skill(root, n, f"name: {n}\ndescription: t\nmetadata:\n version: '1'\n")
_write_symlinks(root, names)
_write_table(root, names)
_write_manifests(root, names)
_write_registry(root, f"# Modes\n\n**9 modes** across {len(names)} skills.\n")
def _violations(root: Path) -> list[str]:
return run_all_checks(root)
def _assert_single(violations: list[str], needle: str) -> None:
assert len(violations) == 1, violations
assert needle in violations[0], violations
# ---- baseline ---------------------------------------------------------------
def test_consistent_root_passes(tmp_path: Path) -> None:
_build_root(tmp_path)
assert _violations(tmp_path) == []
def test_cli_exit_codes(tmp_path: Path) -> None:
_build_root(tmp_path)
ok = run_skill_linter(SCRIPT, tmp_path)
assert ok.returncode == 0, ok.stdout + ok.stderr
assert "OK:" in ok.stdout
(tmp_path / "skills" / NAMES[0]).unlink()
bad = run_skill_linter(SCRIPT, tmp_path)
assert bad.returncode == 1
assert "ERROR:" in bad.stdout
assert "1 violation(s) found." in bad.stderr
def test_empty_root_is_a_violation(tmp_path: Path) -> None:
_assert_single(_violations(tmp_path), "no top-level <name>/SKILL.md found")
# ---- surface B: skills/ symlinks -------------------------------------------
def test_unpackaged_skill_dir_fails(tmp_path: Path) -> None:
"""A fifth directory on disk that nothing else knows about (the #807 shape)."""
_build_root(tmp_path)
write_skill(tmp_path, "gamma-skill", "name: gamma-skill\n")
v = _violations(tmp_path)
assert len(v) == 6, v
missing = [line for line in v if "'gamma-skill' exists on disk" in line]
assert len(missing) == 3, v
assert any("skills/ symlinks" in line for line in missing)
assert any("Skills Overview table" in line for line in missing)
assert any("marketplace.json plugins[].skills" in line for line in missing)
# the "2 skills" claims are now stale on every count surface
counts = [line for line in v if "claims '2 skills' but 3" in line]
assert len(counts) == 3, v # marketplace, plugin.json, MODE_REGISTRY.md
def test_missing_symlink_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
(tmp_path / "skills" / NAMES[1]).unlink()
_assert_single(_violations(tmp_path), f"'{NAMES[1]}' exists on disk")
def test_extra_symlink_without_dir_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
os.symlink("../ghost", tmp_path / "skills" / "ghost")
v = _violations(tmp_path)
assert len(v) == 2, v
assert any("dangling symlink" in line for line in v)
assert any("lists skill 'ghost' but no top-level" in line for line in v)
def test_symlink_to_wrong_target_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
link = tmp_path / "skills" / NAMES[0]
link.unlink()
os.symlink(f"../{NAMES[1]}", link)
_assert_single(_violations(tmp_path), "symlink resolves to")
def test_real_directory_instead_of_symlink_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
link = tmp_path / "skills" / NAMES[0]
link.unlink()
link.mkdir()
_assert_single(_violations(tmp_path), "must be a symlink")
def test_missing_skills_dir_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
for n in NAMES:
(tmp_path / "skills" / n).unlink()
(tmp_path / "skills").rmdir()
v = _violations(tmp_path)
assert any("directory is missing" in line for line in v)
assert sum("not listed in skills/ symlinks" in line for line in v) == len(NAMES)
# ---- surface C: CLAUDE.md table --------------------------------------------
def test_missing_table_row_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
_write_table(tmp_path, NAMES[:1])
_assert_single(_violations(tmp_path), f"'{NAMES[1]}' exists on disk")
def test_extra_table_row_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
_write_table(tmp_path, NAMES + ("stale-skill",))
_assert_single(_violations(tmp_path), "lists skill 'stale-skill' but no top-level")
def test_rows_outside_section_are_ignored(tmp_path: Path) -> None:
"""The fixture table carries a `not-a-skill` row under a later heading."""
_build_root(tmp_path)
assert _violations(tmp_path) == []
def _replace_alpha_row(root: Path, row: str) -> None:
md = root / ".claude" / "CLAUDE.md"
text = md.read_text(encoding="utf-8")
md.write_text(text.replace("| `alpha-skill` v1.0.0 | purpose | full |", row), encoding="utf-8")
@pytest.mark.parametrize("row", [
"| `alpha-skill` | no version at all | full |",
"| `alpha-skill` 1.0.0 | version without v | full |",
])
def test_row_without_version_token_fails(tmp_path: Path, row: str) -> None:
"""A row the version lint would skip is still counted as listed AND reported."""
_build_root(tmp_path)
_replace_alpha_row(tmp_path, row)
_assert_single(_violations(tmp_path), "row for 'alpha-skill' lacks a 'vX.Y.Z' token")
@pytest.mark.parametrize("name", ["Ghost-Skill", "ghost_skill", "ghost skill", "ghost.skill"])
def test_non_canonical_row_name_is_reported(tmp_path: Path, name: str) -> None:
"""A stale advertised row with a non-canonical name must not be invisible."""
_build_root(tmp_path)
md = tmp_path / ".claude" / "CLAUDE.md"
text = md.read_text(encoding="utf-8").replace(
"| `beta-skill` v1.0.0 | purpose | full |",
f"| `beta-skill` v1.0.0 | purpose | full |\n| `{name}` v1.0.0 | stale | x |",
)
md.write_text(text, encoding="utf-8")
_assert_single(_violations(tmp_path), f"row names {name!r}, which is not a canonical")
@pytest.mark.parametrize("row", [
"| ghost-skill v1.0.0 | no backticks | x |",
"| **ghost-skill** v1.0.0 | bold instead | x |",
"| | empty first cell | x |",
])
def test_data_row_without_backticked_name_is_reported(tmp_path: Path, row: str) -> None:
_build_root(tmp_path)
md = tmp_path / ".claude" / "CLAUDE.md"
text = md.read_text(encoding="utf-8").replace(
"| `beta-skill` v1.0.0 | purpose | full |\n",
f"| `beta-skill` v1.0.0 | purpose | full |\n{row}\n", 1)
md.write_text(text, encoding="utf-8")
_assert_single(_violations(tmp_path), "has no backticked skill name in its first cell")
@pytest.mark.parametrize("separator", ["|-------|---------|-----------|", "|:--|:-:|--:|", "| --- | --- | --- |"])
def test_header_and_separator_rows_are_not_data_rows(tmp_path: Path, separator: str) -> None:
_build_root(tmp_path)
md = tmp_path / ".claude" / "CLAUDE.md"
md.write_text(md.read_text(encoding="utf-8").replace("|-------|---------|-----------|", separator), encoding="utf-8")
assert _violations(tmp_path) == []
@pytest.mark.parametrize("mutation", ["drop_separator", "drop_header_and_separator"])
def test_table_without_separator_is_not_a_table(tmp_path: Path, mutation: str) -> None:
_build_root(tmp_path)
md = tmp_path / ".claude" / "CLAUDE.md"
text = md.read_text(encoding="utf-8").replace("|-------|---------|-----------|\n", "", 1)
if mutation == "drop_header_and_separator":
text = text.replace("| Skill | Purpose | Key Modes |\n", "", 1)
md.write_text(text, encoding="utf-8")
v = _violations(tmp_path)
assert any("is not followed by a GFM table" in line for line in v), v
@pytest.mark.parametrize("separator", ["|---|", "|---|---|", "|---|---|---|---|"])
def test_separator_width_must_match_header(tmp_path: Path, separator: str) -> None:
_build_root(tmp_path)
md = tmp_path / ".claude" / "CLAUDE.md"
md.write_text(md.read_text(encoding="utf-8").replace("|-------|---------|-----------|", separator), encoding="utf-8")
v = _violations(tmp_path)
assert any("GFM requires them equal" in line for line in v), v
def test_escaped_pipe_in_header_is_one_cell(tmp_path: Path) -> None:
_build_root(tmp_path)
md = tmp_path / ".claude" / "CLAUDE.md"
md.write_text(md.read_text(encoding="utf-8").replace("| Skill | Purpose | Key Modes |", "| Skill | Purpose \\| Role | Key Modes |"), encoding="utf-8")
assert _violations(tmp_path) == []
def test_non_semver_version_token_is_left_to_the_version_lint(tmp_path: Path) -> None:
"""`vNOPE` satisfies the shared FULL grammar; check_version_consistency.py
rejects it as non-semver, so parity stays silent by design."""
_build_root(tmp_path)
_replace_alpha_row(tmp_path, "| `alpha-skill` vNOPE | placeholder | full |")
assert _violations(tmp_path) == []
def test_missing_section_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
(tmp_path / ".claude" / "CLAUDE.md").write_text("# nothing\n", encoding="utf-8")
v = _violations(tmp_path)
assert any("H2 is missing" in line for line in v)
def test_demoted_heading_is_not_the_section(tmp_path: Path) -> None:
_build_root(tmp_path)
md = tmp_path / ".claude" / "CLAUDE.md"
md.write_text(md.read_text(encoding="utf-8").replace("## Skills Overview", "### Skills Overview"), encoding="utf-8")
v = _violations(tmp_path)
assert any("H2 is missing" in line for line in v), v
def test_fenced_heading_copy_does_not_bind(tmp_path: Path) -> None:
"""A fenced example carrying the heading + a ghost row sits BEFORE the real
section; parity must bind to the real H2 and ignore the ghost."""
_build_root(tmp_path)
md = tmp_path / ".claude" / "CLAUDE.md"
fenced = "```markdown\n## Skills Overview\n\n| `ghost` v9.9.9 | x | y |\n```\n\n"
md.write_text(md.read_text(encoding="utf-8").replace("## Skills Overview", fenced + "## Skills Overview", 1), encoding="utf-8")
assert _violations(tmp_path) == []
@pytest.mark.parametrize("trailer", [
"\n| Other | Table |\n|---|---|\n| `ghost` v1.0.0 | second table |\n",
"\n```\n| `ghost` v1.0.0 | fenced sample |\n```\n",
"\nProse mentioning `ghost` v1.0.0 | in a pipe-free line.\n",
])
def test_only_the_first_table_after_the_heading_counts(tmp_path: Path, trailer: str) -> None:
_build_root(tmp_path)
md = tmp_path / ".claude" / "CLAUDE.md"
text = md.read_text(encoding="utf-8").replace("| `beta-skill` v1.0.0 | purpose | full |\n",
"| `beta-skill` v1.0.0 | purpose | full |\n" + trailer, 1)
md.write_text(text, encoding="utf-8")
assert _violations(tmp_path) == []
def test_skill_only_in_a_second_table_is_not_listed(tmp_path: Path) -> None:
_build_root(tmp_path)
md = tmp_path / ".claude" / "CLAUDE.md"
text = md.read_text(encoding="utf-8").replace(
"| `beta-skill` v1.0.0 | purpose | full |\n",
"\n| Other | Table |\n|---|---|\n| `beta-skill` v1.0.0 | moved here |\n", 1)
md.write_text(text, encoding="utf-8")
_assert_single(_violations(tmp_path), "'beta-skill' exists on disk")
def test_missing_claude_md_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
(tmp_path / ".claude" / "CLAUDE.md").unlink()
v = _violations(tmp_path)
assert any("CLAUDE.md: file is missing" in line for line in v)
# ---- surface D: marketplace.json -------------------------------------------
def test_missing_manifest_entry_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
_write_manifests(tmp_path, NAMES, market_entries=[f"./{NAMES[0]}"])
_assert_single(_violations(tmp_path), f"'{NAMES[1]}' exists on disk")
def test_extra_manifest_entry_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
_write_manifests(tmp_path, NAMES, market_entries=[f"./{n}" for n in NAMES] + ["./stale"])
_assert_single(_violations(tmp_path), "lists skill 'stale' but no top-level")
@pytest.mark.parametrize("bad", ["alpha-skill", "skills/alpha-skill", "./Alpha", "./a/b", 7, "./alpha-skill\n", "./alpha-skill "])
def test_malformed_manifest_entry_fails(tmp_path: Path, bad) -> None:
_build_root(tmp_path)
_write_manifests(tmp_path, NAMES, market_entries=[bad, f"./{NAMES[1]}"])
v = _violations(tmp_path)
assert any("must be './<name>'" in line for line in v), v
# the malformed entry does not count as listing alpha-skill
assert any(f"'{NAMES[0]}' exists on disk" in line for line in v), v
def test_invalid_manifest_json_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
(tmp_path / ".claude-plugin" / "marketplace.json").write_text("{", encoding="utf-8")
v = _violations(tmp_path)
assert any("invalid JSON" in line for line in v)
def test_missing_marketplace_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
(tmp_path / ".claude-plugin" / "marketplace.json").unlink()
v = _violations(tmp_path)
assert any("marketplace.json: file is missing" in line for line in v)
# ---- count claims -----------------------------------------------------------
def test_stale_marketplace_count_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
_write_manifests(tmp_path, NAMES, market_desc="4 skills + 27 modes")
_assert_single(_violations(tmp_path), "marketplace.json description: claims '4 skills'")
def test_stale_marketplace_top_level_count_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
_write_manifests(tmp_path, NAMES, market_top_desc="A marketplace of 3 skills")
_assert_single(_violations(tmp_path), "marketplace.json description: claims '3 skills'")
def test_stale_plugin_count_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
_write_manifests(tmp_path, NAMES, plugin_desc="4 skills, 27 modes")
_assert_single(_violations(tmp_path), "plugin.json description: claims '4 skills'")
@pytest.mark.parametrize("desc", ["4 Skills + 27 modes", "4 SKILLS"])
def test_count_claim_is_case_insensitive(tmp_path: Path, desc: str) -> None:
_build_root(tmp_path)
_write_manifests(tmp_path, NAMES, plugin_desc=desc)
_assert_single(_violations(tmp_path), "plugin.json description: claims")
def test_description_without_count_makes_no_claim(tmp_path: Path) -> None:
_build_root(tmp_path)
_write_manifests(tmp_path, NAMES, market_desc="research skills for Claude Code", plugin_desc="40 skillsets")
assert _violations(tmp_path) == []
def test_stale_mode_registry_count_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
_write_registry(tmp_path, "**27 modes** across 4 skills.\n")
_assert_single(_violations(tmp_path), "MODE_REGISTRY.md: claims '4 skills'")
def test_missing_mode_registry_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
(tmp_path / "MODE_REGISTRY.md").unlink()
_assert_single(_violations(tmp_path), "MODE_REGISTRY.md: file is missing")
def test_missing_plugin_json_fails(tmp_path: Path) -> None:
_build_root(tmp_path)
(tmp_path / ".claude-plugin" / "plugin.json").unlink()
_assert_single(_violations(tmp_path), "plugin.json: file is missing")
# ---- real tree --------------------------------------------------------------
def test_real_tree_passes() -> None:
assert run_all_checks(REPO_ROOT) == []
+12
View File
@@ -787,6 +787,18 @@ class TestSkillVersionTableConsistency(unittest.TestCase):
csc.ERRORS.clear()
csc.ERRORS.extend(self._orig_errors)
def test_skill_paths_follow_the_active_root(self) -> None:
"""#809: paths derive from ROOT at call time, so a fixture tree with a
different skill set is policed on ITS skills, never the checkout's."""
with TemporaryDirectory() as tmp:
root = Path(tmp)
csc.ROOT = root
(root / "only-skill").mkdir()
(root / "only-skill" / "SKILL.md").write_text("---\nname: only-skill\n---\n", encoding="utf-8")
self.assertEqual(csc._skill_version_paths(), ("only-skill/SKILL.md",))
csc.check_skill_version_blocks()
self.assertTrue(all(e.startswith("only-skill/SKILL.md:") for e in csc.ERRORS), csc.ERRORS)
def test_all_four_aligned_passes(self) -> None:
"""All four SKILL.md with frontmatter matching their table → no errors."""
with TemporaryDirectory() as tmp: