From 78913e9c5544e68016e315dbfed6fee34cb74a6e Mon Sep 17 00:00:00 2001 From: Brad Kinnard Date: Tue, 28 Apr 2026 17:52:08 -0600 Subject: [PATCH 01/13] refactor: rename known frontmatter fields to spec fields --- src/skillcheck/config.py | 2 +- src/skillcheck/rules/frontmatter.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/skillcheck/config.py b/src/skillcheck/config.py index aac05fc..a48e771 100644 --- a/src/skillcheck/config.py +++ b/src/skillcheck/config.py @@ -15,7 +15,7 @@ BLOAT_CODE_BLOCK_LINES: int = 50 BLOAT_TABLE_ROWS: int = 20 -KNOWN_FRONTMATTER_FIELDS: frozenset[str] = frozenset({ +SPEC_FIELDS: frozenset[str] = frozenset({ "name", "description", "version", diff --git a/src/skillcheck/rules/frontmatter.py b/src/skillcheck/rules/frontmatter.py index 8591276..1107209 100644 --- a/src/skillcheck/rules/frontmatter.py +++ b/src/skillcheck/rules/frontmatter.py @@ -335,13 +335,13 @@ def check_description_person_voice(skill: ParsedSkill) -> list[Diagnostic]: def check_unknown_fields(skill: ParsedSkill) -> list[Diagnostic]: diagnostics = [] for field in skill.frontmatter: - if field not in config.KNOWN_FRONTMATTER_FIELDS: + if field not in config.SPEC_FIELDS: diagnostics.append(Diagnostic( rule="frontmatter.field.unknown", severity=Severity.WARNING, message=( f"Unknown frontmatter field '{field}'. " - f"Known fields: {', '.join(sorted(config.KNOWN_FRONTMATTER_FIELDS))}." + f"Known fields: {', '.join(sorted(config.SPEC_FIELDS))}." ), line=_field_line(skill.raw_text, str(field)), context=f"{field}: ...", From f9bed102fb5c68fc4b14595a05f290dcf07d9500 Mon Sep 17 00:00:00 2001 From: Brad Kinnard Date: Tue, 28 Apr 2026 17:54:03 -0600 Subject: [PATCH 02/13] feat: classify ecosystem and extension fields --- src/skillcheck/cli.py | 49 +++++++++++++++-------------- src/skillcheck/config.py | 16 ++++++++++ src/skillcheck/config_loader.py | 38 +++++++++++++++++----- src/skillcheck/rules/frontmatter.py | 31 +++++++++++++----- 4 files changed, 96 insertions(+), 38 deletions(-) diff --git a/src/skillcheck/cli.py b/src/skillcheck/cli.py index f31b5e1..b4dc836 100644 --- a/src/skillcheck/cli.py +++ b/src/skillcheck/cli.py @@ -6,6 +6,7 @@ from pathlib import Path from skillcheck import __version__ +from skillcheck import config as runtime_config from skillcheck.config_loader import ConfigError, find_config, load_config from skillcheck.core import ( append_run, @@ -662,38 +663,40 @@ def _apply_config(args: argparse.Namespace, parser: argparse.ArgumentParser) -> """Apply skillcheck.toml defaults to parsed args.""" config_path = args.config or find_config(args.path) try: - config = load_config(config_path) + loaded_config = load_config(config_path) except ConfigError as exc: parser.error(str(exc)) - if config.format is not None and args.format == "text": - args.format = config.format - if config.max_lines is not None and args.max_lines is None: - args.max_lines = config.max_lines - if config.max_tokens is not None and args.max_tokens is None: - args.max_tokens = config.max_tokens - if config.min_desc_score is not None and args.min_desc_score is None: - args.min_desc_score = config.min_desc_score - if config.target_agent is not None and args.target_agent == "all": - args.target_agent = config.target_agent - if config.strict_vscode is True: + runtime_config.set_extension_fields(loaded_config.extension_fields) + + if loaded_config.format is not None and args.format == "text": + args.format = loaded_config.format + if loaded_config.max_lines is not None and args.max_lines is None: + args.max_lines = loaded_config.max_lines + if loaded_config.max_tokens is not None and args.max_tokens is None: + args.max_tokens = loaded_config.max_tokens + if loaded_config.min_desc_score is not None and args.min_desc_score is None: + args.min_desc_score = loaded_config.min_desc_score + if loaded_config.target_agent is not None and args.target_agent == "all": + args.target_agent = loaded_config.target_agent + if loaded_config.strict_vscode is True: args.strict_vscode = True - if config.skip_dirname_check is True: + if loaded_config.skip_dirname_check is True: args.skip_dirname_check = True - if config.skip_ref_check is True: + if loaded_config.skip_ref_check is True: args.skip_ref_check = True - if config.ignore and not args.ignore_prefixes: - args.ignore_prefixes = list(config.ignore) - if config.analyze_graph is True: + if loaded_config.ignore and not args.ignore_prefixes: + args.ignore_prefixes = list(loaded_config.ignore) + if loaded_config.analyze_graph is True: args.analyze_graph = True - if config.semantic is True: + if loaded_config.semantic is True: args.semantic = True - if config.history is True: + if loaded_config.history is True: args.history = True - if config.critique_agent is not None and args.critique_agent is None: - args.critique_agent = config.critique_agent - if config.graph_agent is not None and args.graph_agent is None: - args.graph_agent = config.graph_agent + if loaded_config.critique_agent is not None and args.critique_agent is None: + args.critique_agent = loaded_config.critique_agent + if loaded_config.graph_agent is not None and args.graph_agent is None: + args.graph_agent = loaded_config.graph_agent def main() -> None: diff --git a/src/skillcheck/config.py b/src/skillcheck/config.py index a48e771..0ed2928 100644 --- a/src/skillcheck/config.py +++ b/src/skillcheck/config.py @@ -1,3 +1,5 @@ +from collections.abc import Iterable + MAX_BODY_LINES: int = 500 MAX_TOKENS: int = 8000 @@ -32,6 +34,20 @@ "mode", }) +ECOSYSTEM_FIELDS: frozenset[str] = frozenset({ + "license", + "repository", + "homepage", +}) + +extension_fields: frozenset[str] = frozenset() + + +def set_extension_fields(fields: Iterable[str]) -> None: + global extension_fields + extension_fields = frozenset(fields) + + # Cross-agent compatibility matrix. # Each field maps to a dict of agent -> support status. # Statuses: "supported", "ignored", "unknown" diff --git a/src/skillcheck/config_loader.py b/src/skillcheck/config_loader.py index 1455d74..b7b8ec2 100644 --- a/src/skillcheck/config_loader.py +++ b/src/skillcheck/config_loader.py @@ -33,6 +33,7 @@ class SkillcheckConfig: history: bool | None = None critique_agent: str | None = None graph_agent: str | None = None + extension_fields: frozenset[str] = frozenset() class ConfigError(Exception): @@ -92,24 +93,35 @@ def _parse_without_tomllib(raw: str) -> dict[str, Any]: """Parse a minimal TOML subset for Python 3.10 fallback. Supports top-level key/value pairs with strings, booleans, integers, and - string arrays. That covers skillcheck's configuration surface. + string arrays, plus [frontmatter] for extension fields. That covers + skillcheck's configuration surface. """ parsed: dict[str, Any] = {} + current = parsed for line_no, raw_line in enumerate(raw.splitlines(), start=1): line = raw_line.split("#", 1)[0].strip() if not line: continue if line.startswith("["): - raise ConfigError( - f"skillcheck.toml line {line_no} uses TOML sections; keep skillcheck options at the top level." - ) + if not line.endswith("]"): + raise ConfigError(f"skillcheck.toml line {line_no} has an invalid section header.") + section = line[1:-1].strip() + if section != "frontmatter": + raise ConfigError( + f"skillcheck.toml line {line_no} uses unsupported section '[{section}]'." + ) + frontmatter = parsed.setdefault("frontmatter", {}) + if not isinstance(frontmatter, dict): + raise ConfigError("Config section 'frontmatter' must be a table.") + current = frontmatter + continue if "=" not in line: raise ConfigError(f"skillcheck.toml line {line_no} is missing '='.") key, value_raw = [part.strip() for part in line.split("=", 1)] if value_raw in {"true", "false"}: - parsed[key] = value_raw == "true" + current[key] = value_raw == "true" elif value_raw.startswith('"') and value_raw.endswith('"'): - parsed[key] = value_raw[1:-1] + current[key] = value_raw[1:-1] elif value_raw.startswith("[") and value_raw.endswith("]"): items = [] inner = value_raw[1:-1].strip() @@ -121,10 +133,10 @@ def _parse_without_tomllib(raw: str) -> dict[str, Any]: f"skillcheck.toml line {line_no} array values must be quoted strings." ) items.append(item[1:-1]) - parsed[key] = items + current[key] = items else: try: - parsed[key] = int(value_raw) + current[key] = int(value_raw) except ValueError as exc: raise ConfigError( f"skillcheck.toml line {line_no} value for '{key}' must be a string, integer, boolean, or string array." @@ -159,6 +171,16 @@ def load_config(path: Path | None) -> SkillcheckConfig: raise ConfigError(f"Cannot parse {path}: {exc}. Fix the TOML syntax and retry.") from exc values: dict[str, Any] = {} + frontmatter = data.pop("frontmatter", {}) + if not isinstance(frontmatter, dict): + raise ConfigError("Config section 'frontmatter' must be a table.") + for raw_key, value in frontmatter.items(): + if raw_key != "extension_fields": + raise ConfigError(f"Unknown config key 'frontmatter.{raw_key}' in {path}.") + if not isinstance(value, list) or not all(isinstance(item, str) for item in value): + raise ConfigError("Config key 'frontmatter.extension_fields' must be an array of strings.") + values["extension_fields"] = frozenset(value) + for raw_key, value in data.items(): field = _KEY_MAP.get(raw_key) if field is None: diff --git a/src/skillcheck/rules/frontmatter.py b/src/skillcheck/rules/frontmatter.py index 1107209..0804e78 100644 --- a/src/skillcheck/rules/frontmatter.py +++ b/src/skillcheck/rules/frontmatter.py @@ -335,17 +335,34 @@ def check_description_person_voice(skill: ParsedSkill) -> list[Diagnostic]: def check_unknown_fields(skill: ParsedSkill) -> list[Diagnostic]: diagnostics = [] for field in skill.frontmatter: - if field not in config.SPEC_FIELDS: + field_name = str(field) + if field_name in config.SPEC_FIELDS or field_name in config.extension_fields: + continue + if field_name in config.ECOSYSTEM_FIELDS: diagnostics.append(Diagnostic( - rule="frontmatter.field.unknown", - severity=Severity.WARNING, + rule="frontmatter.field.ecosystem", + severity=Severity.INFO, message=( - f"Unknown frontmatter field '{field}'. " - f"Known fields: {', '.join(sorted(config.SPEC_FIELDS))}." + f"Field '{field_name}' is ecosystem-common but not in the " + f"agentskills.io spec. Add it to skillcheck.toml under " + f"[frontmatter] extension_fields if intentional." ), - line=_field_line(skill.raw_text, str(field)), - context=f"{field}: ...", + line=_field_line(skill.raw_text, field_name), + context=f"{field_name}: ...", + source="advisory", + confidence="medium", )) + continue + diagnostics.append(Diagnostic( + rule="frontmatter.field.unknown", + severity=Severity.WARNING, + message=( + f"Unknown frontmatter field '{field_name}'. " + f"Known fields: {', '.join(sorted(config.SPEC_FIELDS))}." + ), + line=_field_line(skill.raw_text, field_name), + context=f"{field_name}: ...", + )) return diagnostics From ff989a8963ecc2e54888b846e2131d9b006eed32 Mon Sep 17 00:00:00 2001 From: Brad Kinnard Date: Tue, 28 Apr 2026 17:54:47 -0600 Subject: [PATCH 03/13] fix: demote reserved-name rule to warning --- src/skillcheck/rules/frontmatter.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/skillcheck/rules/frontmatter.py b/src/skillcheck/rules/frontmatter.py index 0804e78..9a5b5aa 100644 --- a/src/skillcheck/rules/frontmatter.py +++ b/src/skillcheck/rules/frontmatter.py @@ -226,10 +226,16 @@ def check_name_reserved_words(skill: ParsedSkill) -> list[Diagnostic]: if word in name: return [Diagnostic( rule="frontmatter.name.reserved-word", - severity=Severity.ERROR, - message=f"Name contains reserved word '{word}': '{name}'.", + severity=Severity.WARNING, + message=( + f"Name contains the term '{word}' which may collide with " + f"platform-reserved namespaces. Verify with the target " + f"agent's documentation." + ), line=_field_line(skill.raw_text, "name"), context=f"name: {name}", + source="advisory", + confidence="low", )] return [] From a4f91f214bbfd82888e6ad4cbd16e515be6654ef Mon Sep 17 00:00:00 2001 From: Brad Kinnard Date: Tue, 28 Apr 2026 17:56:19 -0600 Subject: [PATCH 04/13] feat: detect template skill files --- src/skillcheck/config.py | 1 + src/skillcheck/rules/__init__.py | 2 ++ src/skillcheck/rules/compat.py | 7 +++++ src/skillcheck/rules/description.py | 7 +++++ src/skillcheck/rules/frontmatter.py | 4 +++ src/skillcheck/rules/template.py | 26 +++++++++++++++++ src/skillcheck/template_detection.py | 42 ++++++++++++++++++++++++++++ 7 files changed, 89 insertions(+) create mode 100644 src/skillcheck/rules/template.py create mode 100644 src/skillcheck/template_detection.py diff --git a/src/skillcheck/config.py b/src/skillcheck/config.py index 0ed2928..78cab2e 100644 --- a/src/skillcheck/config.py +++ b/src/skillcheck/config.py @@ -38,6 +38,7 @@ "license", "repository", "homepage", + "template", }) extension_fields: frozenset[str] = frozenset() diff --git a/src/skillcheck/rules/__init__.py b/src/skillcheck/rules/__init__.py index 8c84b37..22aad8c 100644 --- a/src/skillcheck/rules/__init__.py +++ b/src/skillcheck/rules/__init__.py @@ -43,8 +43,10 @@ check_reference_depth, ) from skillcheck.rules.sizing import make_line_count_rule, make_token_estimate_rule +from skillcheck.rules.template import check_template_detected _FRONTMATTER_RULES: list[Callable[[ParsedSkill], list[Diagnostic]]] = [ + check_template_detected, check_name_required, check_name_type, check_name_max_length, diff --git a/src/skillcheck/rules/compat.py b/src/skillcheck/rules/compat.py index 73dbc84..48a94d7 100644 --- a/src/skillcheck/rules/compat.py +++ b/src/skillcheck/rules/compat.py @@ -15,6 +15,7 @@ from skillcheck import config from skillcheck.parser import ParsedSkill from skillcheck.result import Diagnostic, Severity +from skillcheck.template_detection import is_template def check_claude_only_fields(skill: ParsedSkill) -> list[Diagnostic]: @@ -55,6 +56,9 @@ def check_vscode_dirname(skill: ParsedSkill) -> list[Diagnostic]: skipped via --skip-dirname-check). This compat rule always runs as INFO unless --strict-vscode promotes it. """ + # Skip on templates: placeholder files are not meant to deploy. + if is_template(skill): + return [] pair = _dirname_mismatch(skill) if pair is None: return [] @@ -97,6 +101,9 @@ def make_strict_vscode_rule() -> Callable[[ParsedSkill], list[Diagnostic]]: """Return a rule that promotes VS Code compat issues to ERROR severity.""" def check_strict_vscode(skill: ParsedSkill) -> list[Diagnostic]: + # Skip on templates: placeholder files are not meant to deploy. + if is_template(skill): + return [] pair = _dirname_mismatch(skill) if pair is None: return [] diff --git a/src/skillcheck/rules/description.py b/src/skillcheck/rules/description.py index 5763a3a..167b810 100644 --- a/src/skillcheck/rules/description.py +++ b/src/skillcheck/rules/description.py @@ -18,6 +18,7 @@ from skillcheck.parser import ParsedSkill from skillcheck.result import Diagnostic, Severity +from skillcheck.template_detection import is_template # Action-verb base forms. 3rd-person singular ("generates", "identifies") is # matched via stem normalization in `_is_action_verb`, so we only store one @@ -209,6 +210,9 @@ def score_description(desc: str) -> tuple[int, list[str]]: def check_description_quality(skill: ParsedSkill) -> list[Diagnostic]: """Score description quality and return an INFO diagnostic with the result.""" + # Skip on templates: placeholder files are not meant to deploy. + if is_template(skill): + return [] desc = skill.frontmatter.get("description") if not desc or not isinstance(desc, str) or not desc.strip(): return [] # Missing/empty descriptions are handled by frontmatter rules. @@ -232,6 +236,9 @@ def make_min_score_rule( """Return a rule that errors/warns when description score falls below threshold.""" def check_description_min_score(skill: ParsedSkill) -> list[Diagnostic]: + # Skip on templates: placeholder files are not meant to deploy. + if is_template(skill): + return [] desc = skill.frontmatter.get("description") if not desc or not isinstance(desc, str) or not desc.strip(): return [] diff --git a/src/skillcheck/rules/frontmatter.py b/src/skillcheck/rules/frontmatter.py index 9a5b5aa..bf64f3a 100644 --- a/src/skillcheck/rules/frontmatter.py +++ b/src/skillcheck/rules/frontmatter.py @@ -5,6 +5,7 @@ from skillcheck import config from skillcheck.parser import ParsedSkill from skillcheck.result import Diagnostic, Severity +from skillcheck.template_detection import is_template _XML_TAG_RE = re.compile(r"<[a-zA-Z/][^>]*>") _NAME_VALID_CHARS_RE = re.compile(r"^[a-z0-9-]+$") @@ -196,6 +197,9 @@ def check_name_consecutive_hyphens(skill: ParsedSkill) -> list[Diagnostic]: def check_name_directory_match(skill: ParsedSkill) -> list[Diagnostic]: + # Skip on templates: placeholder files are not meant to deploy. + if is_template(skill): + return [] name = skill.frontmatter.get("name") if name is None: return [] diff --git a/src/skillcheck/rules/template.py b/src/skillcheck/rules/template.py new file mode 100644 index 0000000..0e8342d --- /dev/null +++ b/src/skillcheck/rules/template.py @@ -0,0 +1,26 @@ +"""Template detection rule (info-level signal).""" + +from __future__ import annotations + +from skillcheck.parser import ParsedSkill +from skillcheck.result import Diagnostic, Severity +from skillcheck.template_detection import is_template + + +def check_template_detected(skill: ParsedSkill) -> list[Diagnostic]: + if not is_template(skill): + return [] + return [Diagnostic( + rule="template.detected", + severity=Severity.INFO, + message=( + "Detected placeholder content; deployment-blocking checks " + "(directory-name match, VS Code dirname, description quality) " + "are skipped for template files. Copy this file and replace " + "placeholders before deploying." + ), + line=None, + context=None, + source="advisory", + confidence="medium", + )] diff --git a/src/skillcheck/template_detection.py b/src/skillcheck/template_detection.py new file mode 100644 index 0000000..6508f3e --- /dev/null +++ b/src/skillcheck/template_detection.py @@ -0,0 +1,42 @@ +"""Detect placeholder/template SKILL.md files to skip deployment-blocking checks.""" + +from __future__ import annotations + +import re + +from .parser import ParsedSkill + +_PLACEHOLDER_PATTERNS = [ + re.compile(r"\b[Rr]eplace with\b"), + re.compile(r"\b(TODO|FIXME)\b"), + re.compile(r"\byour (skill|description|name|tool)\b", re.IGNORECASE), + re.compile(r"<[a-z][a-z0-9_-]*>"), + re.compile(r"\[(description|name|skill name|placeholder|TODO|...)\]", re.IGNORECASE), + re.compile(r"\{[a-z][a-z0-9_]*\}"), + re.compile(r"\b(lorem ipsum|placeholder|sample description)\b", re.IGNORECASE), +] + + +def is_template(skill: ParsedSkill) -> bool: + """Return True if the skill appears to be a template/placeholder file. + + Recall-favoring: prefer false positives on placeholder-heavy real skills + over missing real templates. Three signals, any one is sufficient: + + 1. Explicit `template: true` in frontmatter. + 2. Description matches a placeholder pattern. + 3. Filename lives in a directory literally named `template` or `templates`. + """ + if skill.frontmatter.get("template") is True: + return True + + description = str(skill.frontmatter.get("description", "")) + for pattern in _PLACEHOLDER_PATTERNS: + if pattern.search(description): + return True + + parent = skill.path.parent.name.lower() + if parent in {"template", "templates"}: + return True + + return False From f7e2a15289ac83f852998ed80ccf4f6d01ad9f2f Mon Sep 17 00:00:00 2001 From: Brad Kinnard Date: Tue, 28 Apr 2026 17:57:45 -0600 Subject: [PATCH 05/13] fix: demote person-voice rule to warning --- src/skillcheck/rules/frontmatter.py | 15 +++++++++------ tests/test_frontmatter.py | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/skillcheck/rules/frontmatter.py b/src/skillcheck/rules/frontmatter.py index bf64f3a..eac26e9 100644 --- a/src/skillcheck/rules/frontmatter.py +++ b/src/skillcheck/rules/frontmatter.py @@ -317,10 +317,11 @@ def check_description_person_voice(skill: ParsedSkill) -> list[Diagnostic]: if first_match: return [Diagnostic( rule="frontmatter.description.person-voice", - severity=Severity.ERROR, + severity=Severity.WARNING, message=( - f"Description uses first-person voice ('{first_match.group().strip()}'). " - f"Use third person, e.g., 'Generates...' or 'Analyzes...'" + f"Description appears to use first-person voice " + f"('{first_match.group().strip()}'); the spec recommends " + f"third-person for routing clarity." ), line=_field_line(skill.raw_text, "description"), context=f"description: {desc[:80]}{'...' if len(desc) > 80 else ''}", @@ -330,10 +331,12 @@ def check_description_person_voice(skill: ParsedSkill) -> list[Diagnostic]: if second_match: return [Diagnostic( rule="frontmatter.description.person-voice", - severity=Severity.ERROR, + severity=Severity.WARNING, message=( - f"Description uses second-person voice ('{second_match.group()}'). " - f"Use third person, e.g., 'Generates...' or 'Analyzes...'" + f"Description appears to use second-person voice " + f"('{second_match.group()}'); the spec recommends " + f"third-person for routing clarity. Verify the phrasing " + f"addresses the agent, not the user." ), line=_field_line(skill.raw_text, "description"), context=f"description: {desc[:80]}{'...' if len(desc) > 80 else ''}", diff --git a/tests/test_frontmatter.py b/tests/test_frontmatter.py index b3b4c5c..19845c4 100644 --- a/tests/test_frontmatter.py +++ b/tests/test_frontmatter.py @@ -220,7 +220,7 @@ def test_description_rejects_first_person(): diagnostics = check_description_person_voice(skill) assert len(diagnostics) == 1 assert diagnostics[0].rule == "frontmatter.description.person-voice" - assert diagnostics[0].severity == Severity.ERROR + assert diagnostics[0].severity == Severity.WARNING def test_description_rejects_second_person(tmp_path): From 26e2ced6868fdb09eb96dc2828332fd140b3519e Mon Sep 17 00:00:00 2001 From: Brad Kinnard Date: Tue, 28 Apr 2026 17:58:39 -0600 Subject: [PATCH 06/13] refactor: align budget message wording --- src/skillcheck/rules/sizing.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/skillcheck/rules/sizing.py b/src/skillcheck/rules/sizing.py index abcda5e..b1a8610 100644 --- a/src/skillcheck/rules/sizing.py +++ b/src/skillcheck/rules/sizing.py @@ -17,7 +17,7 @@ def check_body_line_count(skill: ParsedSkill) -> list[Diagnostic]: rule="sizing.body.line-count", severity=Severity.WARNING, message=( - f"File exceeds the recommended {max_lines}-line limit " + f"File exceeds the recommended line limit of {max_lines} " f"(got {total_lines} lines). Consider splitting into smaller skills." ), )] @@ -37,8 +37,9 @@ def check_body_token_estimate(skill: ParsedSkill) -> list[Diagnostic]: rule="sizing.body.token-estimate", severity=Severity.WARNING, message=( - f"File exceeds the token budget of {max_tokens} " - f"(estimated {token_count} tokens). Consider trimming content." + f"File exceeds the recommended token budget of {max_tokens} " + f"(estimated {token_count} tokens). Consider trimming content " + f"or moving heavy material to referenced files." ), )] return [] From 86bdb4536a6eaf1d134326ad924992fd34228244 Mon Sep 17 00:00:00 2001 From: Brad Kinnard Date: Tue, 28 Apr 2026 18:02:32 -0600 Subject: [PATCH 07/13] test: cover v1.2 false-positive fixes --- README.md | 2 +- tests/fixtures/canvas_design_pattern.md | 6 + tests/fixtures/claude_api_name.md | 6 + tests/fixtures/homepage_field.md | 7 + tests/fixtures/license_field.md | 7 + .../non_template_with_placeholder_word.md | 6 + tests/fixtures/repository_field.md | 7 + tests/fixtures/template_explicit_flag.md | 7 + .../template_placeholder_description.md | 6 + .../fixtures/template_with_real_violations.md | 7 + tests/fixtures/templates/SKILL.md | 6 + tests/fixtures/unknown_field.md | 7 + tests/fixtures/user_extension/skillcheck.toml | 2 + .../user_extension/user_extension_field.md | 7 + tests/test_v1_2_false_positive_fixes.py | 192 ++++++++++++++++++ 15 files changed, 274 insertions(+), 1 deletion(-) create mode 100644 tests/fixtures/canvas_design_pattern.md create mode 100644 tests/fixtures/claude_api_name.md create mode 100644 tests/fixtures/homepage_field.md create mode 100644 tests/fixtures/license_field.md create mode 100644 tests/fixtures/non_template_with_placeholder_word.md create mode 100644 tests/fixtures/repository_field.md create mode 100644 tests/fixtures/template_explicit_flag.md create mode 100644 tests/fixtures/template_placeholder_description.md create mode 100644 tests/fixtures/template_with_real_violations.md create mode 100644 tests/fixtures/templates/SKILL.md create mode 100644 tests/fixtures/unknown_field.md create mode 100644 tests/fixtures/user_extension/skillcheck.toml create mode 100644 tests/fixtures/user_extension/user_extension_field.md create mode 100644 tests/test_v1_2_false_positive_fixes.py diff --git a/README.md b/README.md index 897c5bf..756b647 100644 --- a/README.md +++ b/README.md @@ -348,7 +348,7 @@ pip install -e ".[dev]" python3 -m pytest tests/ -q ``` -667 tests cover all rule modules, CLI exit codes, graph analyzers, divergence detection, critique parsing, history round-trips, and the full self-host pipeline against `skills/skillcheck/SKILL.md`. Fixtures are in `tests/fixtures/`; every rule has at least one positive and one negative test case. `tests/test_readme_test_count_claim.py` asserts this count matches `pytest --collect-only`, so any future suite change has to update the number in the same commit or CI fails. +683 tests cover all rule modules, CLI exit codes, graph analyzers, divergence detection, critique parsing, history round-trips, and the full self-host pipeline against `skills/skillcheck/SKILL.md`. Fixtures are in `tests/fixtures/`; every rule has at least one positive and one negative test case. `tests/test_readme_test_count_claim.py` asserts this count matches `pytest --collect-only`, so any future suite change has to update the number in the same commit or CI fails. ## Maintainer Notes diff --git a/tests/fixtures/canvas_design_pattern.md b/tests/fixtures/canvas_design_pattern.md new file mode 100644 index 0000000..abd43cc --- /dev/null +++ b/tests/fixtures/canvas_design_pattern.md @@ -0,0 +1,6 @@ +--- +name: canvas-design +description: Create beautiful visual art in .png and .pdf documents using design philosophy. You should use this skill when the user asks to create a poster, piece of art, design, or other static piece. +--- + +Body. diff --git a/tests/fixtures/claude_api_name.md b/tests/fixtures/claude_api_name.md new file mode 100644 index 0000000..06d9347 --- /dev/null +++ b/tests/fixtures/claude_api_name.md @@ -0,0 +1,6 @@ +--- +name: claude-api +description: Builds and checks Claude API examples for SDK usage. +--- + +Body. diff --git a/tests/fixtures/homepage_field.md b/tests/fixtures/homepage_field.md new file mode 100644 index 0000000..3fb29e6 --- /dev/null +++ b/tests/fixtures/homepage_field.md @@ -0,0 +1,7 @@ +--- +name: homepage-field +description: Validates skills that carry a common homepage metadata field. +homepage: https://example.com +--- + +Body. diff --git a/tests/fixtures/license_field.md b/tests/fixtures/license_field.md new file mode 100644 index 0000000..a8afec8 --- /dev/null +++ b/tests/fixtures/license_field.md @@ -0,0 +1,7 @@ +--- +name: license-field +description: Validates skills that carry a common license metadata field. +license: MIT +--- + +Body. diff --git a/tests/fixtures/non_template_with_placeholder_word.md b/tests/fixtures/non_template_with_placeholder_word.md new file mode 100644 index 0000000..8cc3e6e --- /dev/null +++ b/tests/fixtures/non_template_with_placeholder_word.md @@ -0,0 +1,6 @@ +--- +name: doc-helper +description: Generates documentation for your skill package. +--- + +Body. diff --git a/tests/fixtures/repository_field.md b/tests/fixtures/repository_field.md new file mode 100644 index 0000000..da44113 --- /dev/null +++ b/tests/fixtures/repository_field.md @@ -0,0 +1,7 @@ +--- +name: repository-field +description: Validates skills that carry a common repository metadata field. +repository: https://github.com/example/repo +--- + +Body. diff --git a/tests/fixtures/template_explicit_flag.md b/tests/fixtures/template_explicit_flag.md new file mode 100644 index 0000000..3d20b6a --- /dev/null +++ b/tests/fixtures/template_explicit_flag.md @@ -0,0 +1,7 @@ +--- +name: template-skill +description: Replace with description of the skill and when the agent should use it. +template: true +--- + +# Insert instructions below diff --git a/tests/fixtures/template_placeholder_description.md b/tests/fixtures/template_placeholder_description.md new file mode 100644 index 0000000..349612f --- /dev/null +++ b/tests/fixtures/template_placeholder_description.md @@ -0,0 +1,6 @@ +--- +name: template-skill +description: Replace with description of the skill and when the agent should use it. +--- + +# Insert instructions below diff --git a/tests/fixtures/template_with_real_violations.md b/tests/fixtures/template_with_real_violations.md new file mode 100644 index 0000000..5aca9a9 --- /dev/null +++ b/tests/fixtures/template_with_real_violations.md @@ -0,0 +1,7 @@ +--- +name: template-skill +description: Replace with description of the skill and when the agent should use it. +template: true +--- + +This body is intentionally long enough to exceed a lowered disclosure budget in tests. diff --git a/tests/fixtures/templates/SKILL.md b/tests/fixtures/templates/SKILL.md new file mode 100644 index 0000000..1bd6c0e --- /dev/null +++ b/tests/fixtures/templates/SKILL.md @@ -0,0 +1,6 @@ +--- +name: template-skill +description: Placeholder skill for authors to copy before publishing. +--- + +# Insert instructions below diff --git a/tests/fixtures/unknown_field.md b/tests/fixtures/unknown_field.md new file mode 100644 index 0000000..26a52cd --- /dev/null +++ b/tests/fixtures/unknown_field.md @@ -0,0 +1,7 @@ +--- +name: unknown-field +description: Validates skills that carry an unsupported metadata field. +xyzzy: foo +--- + +Body. diff --git a/tests/fixtures/user_extension/skillcheck.toml b/tests/fixtures/user_extension/skillcheck.toml new file mode 100644 index 0000000..d7117ed --- /dev/null +++ b/tests/fixtures/user_extension/skillcheck.toml @@ -0,0 +1,2 @@ +[frontmatter] +extension_fields = ["my-org-tag", "internal-id"] diff --git a/tests/fixtures/user_extension/user_extension_field.md b/tests/fixtures/user_extension/user_extension_field.md new file mode 100644 index 0000000..dd403d4 --- /dev/null +++ b/tests/fixtures/user_extension/user_extension_field.md @@ -0,0 +1,7 @@ +--- +name: user-extension-field +description: Validates skills that carry an organization-specific metadata field. +my-org-tag: bar +--- + +Body. diff --git a/tests/test_v1_2_false_positive_fixes.py b/tests/test_v1_2_false_positive_fixes.py new file mode 100644 index 0000000..f92cc33 --- /dev/null +++ b/tests/test_v1_2_false_positive_fixes.py @@ -0,0 +1,192 @@ +from __future__ import annotations + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +from skillcheck import config as runtime_config +from skillcheck.config_loader import load_config +from skillcheck.core import validate +from skillcheck.parser import parse +from skillcheck.result import Severity +from skillcheck.rules.frontmatter import ( + check_description_person_voice, + check_name_reserved_words, + check_unknown_fields, +) +from tests.conftest import FIXTURES_DIR + + +@pytest.fixture(autouse=True) +def reset_extension_fields(): + runtime_config.set_extension_fields(()) + yield + runtime_config.set_extension_fields(()) + + +def _rules(path: Path, **kwargs) -> list[str]: + result = validate(path, **kwargs) + return [diagnostic.rule for diagnostic in result.diagnostics] + + +def _assert_ecosystem_field(path: Path, field: str) -> None: + skill = parse(path) + diagnostics = check_unknown_fields(skill) + assert not any( + d.rule == "frontmatter.field.unknown" and d.severity == Severity.WARNING + for d in diagnostics + ) + ecosystem = [d for d in diagnostics if d.rule == "frontmatter.field.ecosystem"] + assert len(ecosystem) == 1 + assert ecosystem[0].severity == Severity.INFO + assert field in ecosystem[0].message + + +def test_license_field_is_ecosystem_info() -> None: + _assert_ecosystem_field(FIXTURES_DIR / "license_field.md", "license") + + +def test_homepage_field_is_ecosystem_info() -> None: + _assert_ecosystem_field(FIXTURES_DIR / "homepage_field.md", "homepage") + + +def test_repository_field_is_ecosystem_info() -> None: + _assert_ecosystem_field(FIXTURES_DIR / "repository_field.md", "repository") + + +def test_unknown_field_still_warns() -> None: + skill = parse(FIXTURES_DIR / "unknown_field.md") + diagnostics = check_unknown_fields(skill) + unknown = [d for d in diagnostics if d.rule == "frontmatter.field.unknown"] + assert len(unknown) == 1 + assert unknown[0].severity == Severity.WARNING + assert "xyzzy" in unknown[0].message + + +def test_user_extension_field_is_silent() -> None: + fixture_dir = FIXTURES_DIR / "user_extension" + loaded_config = load_config(fixture_dir / "skillcheck.toml") + runtime_config.set_extension_fields(loaded_config.extension_fields) + skill = parse(fixture_dir / "user_extension_field.md") + assert check_unknown_fields(skill) == [] + + +def test_claude_api_name_warns_without_error() -> None: + skill = parse(FIXTURES_DIR / "claude_api_name.md") + diagnostics = check_name_reserved_words(skill) + assert len(diagnostics) == 1 + assert diagnostics[0].rule == "frontmatter.name.reserved-word" + assert diagnostics[0].severity == Severity.WARNING + assert diagnostics[0].source == "advisory" + assert diagnostics[0].confidence == "low" + + +def test_claude_api_name_validation_passes() -> None: + result = validate(FIXTURES_DIR / "claude_api_name.md", skip_dirname_check=True) + assert result.valid is True + warnings = [d for d in result.diagnostics if d.severity == Severity.WARNING] + assert [d.rule for d in warnings] == ["frontmatter.name.reserved-word"] + + +def test_canvas_design_pattern_warns_without_error() -> None: + skill = parse(FIXTURES_DIR / "canvas_design_pattern.md") + diagnostics = check_description_person_voice(skill) + assert len(diagnostics) == 1 + assert diagnostics[0].rule == "frontmatter.description.person-voice" + assert diagnostics[0].severity == Severity.WARNING + assert "You should" in diagnostics[0].message + + +def test_canvas_design_pattern_validation_passes() -> None: + result = validate(FIXTURES_DIR / "canvas_design_pattern.md", skip_dirname_check=True) + assert result.valid is True + warnings = [d for d in result.diagnostics if d.severity == Severity.WARNING] + assert [d.rule for d in warnings] == ["frontmatter.description.person-voice"] + + +def _assert_template_skips_deployment_checks(path: Path) -> None: + rules = _rules(path, strict_vscode=True) + assert "template.detected" in rules + assert "frontmatter.name.directory-mismatch" not in rules + assert "compat.vscode-dirname" not in rules + assert "description.quality-score" not in rules + + +def test_template_explicit_flag_skips_deployment_checks() -> None: + _assert_template_skips_deployment_checks(FIXTURES_DIR / "template_explicit_flag.md") + + +def test_template_placeholder_description_skips_deployment_checks() -> None: + _assert_template_skips_deployment_checks( + FIXTURES_DIR / "template_placeholder_description.md" + ) + + +def test_template_directory_skips_deployment_checks() -> None: + _assert_template_skips_deployment_checks(FIXTURES_DIR / "templates" / "SKILL.md") + + +def test_template_detection_keeps_body_budget_rule(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(runtime_config, "BODY_TOKEN_BUDGET", 1) + rules = _rules(FIXTURES_DIR / "template_with_real_violations.md", strict_vscode=True) + assert "template.detected" in rules + assert "disclosure.body-budget" in rules + + +def test_non_template_placeholder_word_documents_false_positive() -> None: + # The detector favors recall, so this real skill is intentionally flagged. + _assert_template_skips_deployment_checks( + FIXTURES_DIR / "non_template_with_placeholder_word.md" + ) + + +CLI_AVAILABLE = shutil.which("skillcheck") is not None + + +@pytest.mark.skipif(not CLI_AVAILABLE, reason="skillcheck command is not installed") +def test_claude_api_name_cli_exit_code_zero() -> None: + result = subprocess.run( + [ + "skillcheck", + "--skip-dirname-check", + str(FIXTURES_DIR / "claude_api_name.md"), + "--format", + "json", + ], + capture_output=True, + text=True, + encoding="utf-8", + ) + assert result.returncode == 0 + payload = json.loads(result.stdout) + diagnostics = payload["results"][0]["diagnostics"] + reserved = [d for d in diagnostics if d["rule"] == "frontmatter.name.reserved-word"] + assert reserved[0]["severity"] == "warning" + + +@pytest.mark.skipif(not CLI_AVAILABLE, reason="skillcheck command is not installed") +def test_canvas_design_pattern_cli_exit_code_zero() -> None: + result = subprocess.run( + [ + "skillcheck", + "--skip-dirname-check", + str(FIXTURES_DIR / "canvas_design_pattern.md"), + "--format", + "json", + ], + capture_output=True, + text=True, + encoding="utf-8", + ) + assert result.returncode == 0 + payload = json.loads(result.stdout) + diagnostics = payload["results"][0]["diagnostics"] + voice = [ + d + for d in diagnostics + if d["rule"] == "frontmatter.description.person-voice" + ] + assert voice[0]["severity"] == "warning" From b5b72d2cc0c1e3b377f9eadc92633419ccb1fea9 Mon Sep 17 00:00:00 2001 From: Brad Kinnard Date: Tue, 28 Apr 2026 18:04:10 -0600 Subject: [PATCH 08/13] docs: document v1.2 rule behavior --- README.md | 43 ++++++++++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 756b647..6aed391 100644 --- a/README.md +++ b/README.md @@ -69,7 +69,7 @@ skillcheck skills/ # recursive scan; finds every file named SKILL.md skillcheck SKILL.md --format json ``` -From the field test on Anthropic's official skills repository (18 skills, snapshot taken during v1.0 release prep in April 2026): four of eighteen files failed. `claude-api/SKILL.md` failed with `frontmatter.name.reserved-word` because the name contains the reserved word "claude". `template/SKILL.md` failed with `frontmatter.name.directory-mismatch` (name `template-skill`, directory `template`). Both files look correct on casual inspection. Reproduce: clone `anthropics/skills` and run `skillcheck skills/ --format text`. +From the field test on Anthropic's official skills repository (18 skills, April 2026 snapshot): v1.1.0 produced four failures from advisory checks. v1.2.0 demotes the `claude-api` reserved-name and person-voice findings to warnings, treats `license` as ecosystem-common metadata, and detects `template/SKILL.md` as a placeholder file. Reproduce: clone `anthropics/skills` and run `skillcheck skills/ --format text`. ### Heuristic Graph @@ -191,22 +191,22 @@ The v1.0 graph and critique modes are available as action inputs. Example with s Text output (default), excerpt from a run against the Anthropic skills corpus: ``` -✗ FAIL skills/claude-api/SKILL.md - line 2 ✗ error frontmatter.name.reserved-word Name contains reserved word 'claude': 'claude-api'. +✔ PASS skills/claude-api/SKILL.md + line 2 ⚠ warning frontmatter.name.reserved-word Name contains the term 'claude' which may collide with platform-reserved namespaces. Verify with the target agent's documentation. name: claude-api - line 4 ⚠ warning frontmatter.field.unknown Unknown frontmatter field 'license'. + line 4 · info frontmatter.field.ecosystem Field 'license' is ecosystem-common but not in the agentskills.io spec. Add it to skillcheck.toml under [frontmatter] extension_fields if intentional. -Checked 18 files: 14 passed, 4 failed, 24 warnings +Checked 18 files: 18 passed, 0 failed, 29 warnings ``` JSON output (`--format json`): ```json { - "version": "1.0.0", + "version": "1.2.0", "files_checked": 18, - "files_passed": 14, - "files_failed": 4, + "files_passed": 18, + "files_failed": 0, "results": [ { "path": "skills/claude-api/SKILL.md", @@ -214,8 +214,8 @@ JSON output (`--format json`): "diagnostics": [ { "rule": "frontmatter.name.reserved-word", - "severity": "error", - "message": "Name contains reserved word 'claude': 'claude-api'.", + "severity": "warning", + "message": "Name contains the term 'claude' which may collide with platform-reserved namespaces. Verify with the target agent's documentation.", "line": 2, "context": "name: claude-api" } @@ -286,14 +286,15 @@ Source tags: `spec` rules derive from the agentskills.io specification or agent- | `frontmatter.name.invalid-chars` | error | spec | Lowercase, numbers, hyphens only | | `frontmatter.name.leading-trailing-hyphen` | error | spec | No leading or trailing hyphens | | `frontmatter.name.consecutive-hyphens` | error | spec | No consecutive hyphens | -| `frontmatter.name.reserved-word` | error | advisory | Not a reserved word (`claude`, `anthropic`) | +| `frontmatter.name.reserved-word` | warning | advisory | Name contains a term that may collide with platform-reserved namespaces | | `frontmatter.name.directory-mismatch` | error | spec | Name must match parent directory (VS Code requirement) | | `frontmatter.description.required` | error | spec | `description` field must exist | | `frontmatter.description.type` | error | advisory | `description` must be a string (catches YAML coercion) | | `frontmatter.description.empty` | error | spec | Description must not be blank | | `frontmatter.description.max-length` | error | spec | 1024 character maximum | | `frontmatter.description.xml-tags` | error | advisory | No XML or HTML tags in description | -| `frontmatter.description.person-voice` | error | advisory | No first or second-person pronouns | +| `frontmatter.description.person-voice` | warning | advisory | First or second-person voice may reduce routing clarity | +| `frontmatter.field.ecosystem` | info | advisory | Field is ecosystem-common but not in the agentskills.io spec | | `frontmatter.field.unknown` | warning | advisory | Field not in the known spec list | | `frontmatter.yaml-anchors` | warning | advisory | YAML anchors and aliases can silently copy values | | `description.quality-score` | info | advisory | Scores description 0-100 for agent discoverability | @@ -309,6 +310,7 @@ Source tags: `spec` rules derive from the agentskills.io specification or agent- | `compat.claude-only` | info | spec | Field only works in Claude Code | | `compat.vscode-dirname` | info / error | spec | Name does not match parent directory (VS Code); promotes to error with `--strict-vscode` | | `compat.unverified` | info | advisory | Field behavior unverified in Codex or Cursor | +| `template.detected` | info | advisory | Placeholder file detected; deployment-blocking checks are skipped | | `graph.capability.orphaned` | warning | heuristic | Capability heading has no declared inputs or outputs | | `graph.input.unused` | warning | heuristic | Body-declared input not required by any capability | | `graph.output.unproduced` | warning | heuristic | Declared output not produced by any capability | @@ -319,11 +321,24 @@ Source tags: `spec` rules derive from the agentskills.io specification or agent- | `history.write.failed` | warning | history | Could not write the ledger file; validation exit code unaffected | | `history.read.failed` | warning | history | Could not read the ledger file; validation continues without regression check | +## Templates + +Template files are detected by `template: true` frontmatter, placeholder-like descriptions, or a parent directory named `template` or `templates`. When detected, skillcheck emits `template.detected` and skips deployment-blocking checks that do not apply before copy-and-fill use: directory-name match, VS Code dirname, and description quality scoring. Sizing, disclosure, references, and other content checks still run. + +## Extension fields + +Use `[frontmatter] extension_fields` in `skillcheck.toml` for organization-specific metadata that should be accepted without diagnostics. + +```toml +[frontmatter] +extension_fields = ["my-org-tag", "internal-id"] +``` + ## Case Study We ran skillcheck against three corpora during v1.0 release prep (April 2026 snapshots): Anthropic's official skills repository (18 skills), the `mcp-builder` skill through the full v1.0 pipeline, and five skills from the uxuiprinciples/agent-skills collection. To reproduce, clone each upstream repo and run `skillcheck ` (the case study below records the exact invocations). -The symbolic run of the Anthropic corpus returned four failures from eighteen files (exit 1). All four files look correct on review: two had second-person voice in the description, one used "claude" as part of the name (reserved word per spec), and the template skill had a name/directory mismatch. The deeper finding came from running `mcp-builder` through the critique pipeline: the symbolic run passed (exit 0), but the ingested agent critique returned exit 3 with three `semantic.contradiction.detected` errors. The skill's frontmatter offers Python and TypeScript as equal options; its body unconditionally recommends TypeScript in Phase 1.3. That inconsistency means any agent following the Python path hits an unresolved decision point. No static linter catches it. See [docs/case-study-v1-real-world-runs.md](docs/case-study-v1-real-world-runs.md) for the full breakdown. +The v1.1.0 symbolic run of the Anthropic corpus returned four failures from eighteen files (exit 1). v1.2.0 reclassifies those findings: `canvas-design` and `theme-factory` now warn for person-voice wording, `claude-api` warns for a possible reserved namespace collision, and `template/SKILL.md` is detected as a placeholder. The deeper finding came from running `mcp-builder` through the critique pipeline: the symbolic run passed (exit 0), but the ingested agent critique returned exit 3 with three `semantic.contradiction.detected` errors. The skill's frontmatter offers Python and TypeScript as equal options; its body unconditionally recommends TypeScript in Phase 1.3. That inconsistency means any agent following the Python path hits an unresolved decision point. No static linter catches it. See [docs/case-study-v1-real-world-runs.md](docs/case-study-v1-real-world-runs.md) for the full breakdown. See also: [docs/case-study-silent-skill-failure.md](docs/case-study-silent-skill-failure.md) (the v0.2.0 case study: a deploy skill that silently disappeared in VS Code due to a name/directory mismatch). @@ -335,6 +350,8 @@ Cross-agent compatibility data for Codex and Cursor comes from available documen Description quality scoring uses heuristics, not an LLM. It catches structural problems (missing action verbs, no trigger phrases, vague words) but cannot evaluate whether instructions are semantically coherent. Agent critique mode addresses that gap. +Template detection favors recall over precision. A real skill with placeholder-like description text may be flagged as `template.detected`; rename the description or add enough concrete routing context before deployment. + The heuristic graph extractor uses heading structure and backtick references as proxies for capability declarations. Skills that express capabilities entirely in prose will produce sparse graphs with many `graph.capability.orphaned` warnings. Agent graph mode (`--emit-graph-prompt` / `--ingest-graph`) addresses this but requires a calling agent. Agent critique and graph modes validate the agent's JSON response against the expected schema and convert it to diagnostics. skillcheck trusts the agent's reasoning; it does not second-guess findings that pass schema validation. The quality of the output depends on the quality of the calling agent. From 1b091af962e647b7416834fffda1df9d57878cdb Mon Sep 17 00:00:00 2001 From: Brad Kinnard Date: Tue, 28 Apr 2026 18:06:03 -0600 Subject: [PATCH 09/13] chore: bump version to 1.2.0 --- CHANGELOG.md | 29 ++++++++++++++++++++++++++++- pyproject.toml | 2 +- skills/skillcheck/SKILL.md | 2 +- src/skillcheck/__init__.py | 2 +- src/skillcheck/rules/description.py | 4 ++-- tests/test_references.py | 2 +- 6 files changed, 34 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e847f6..50bc73b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,33 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [1.2.0] - 2026-04-29 + +Backward compatibility: previously-passing skills still pass. Some previously-failing skills now warn instead of error and produce exit code 0 instead of 1. + +### Added + +- `template.detected` info-level rule and `src/skillcheck/template_detection.py` module. +- `ECOSYSTEM_FIELDS` classification for `license`, `repository`, `homepage`, and `template`. +- Config support for `[frontmatter] extension_fields` in `skillcheck.toml`. + +### Changed + +- `frontmatter.name.reserved-word` demoted from ERROR to WARNING; source tag changed from `spec` to `advisory`; message rewritten. +- `frontmatter.description.person-voice` demoted from ERROR to WARNING; messages rewritten to acknowledge the heuristic. +- Budget-message phrasing aligned with the spec's "recommended" language across `sizing.*` and `disclosure.*` rules. + +### Fixed + +- `frontmatter.field.unknown` no longer fires on `license`, `repository`, `homepage`, or `template`; these now produce info-level `frontmatter.field.ecosystem` diagnostics or are silent for user extensions. +- Templates (placeholder content, `template: true` flag, or files under `template/` or `templates/` directories) no longer trigger deployment-blocking checks (`frontmatter.name.directory-mismatch`, `compat.vscode-dirname`, `description.quality-score`). + +### Internal + +- Renamed `config.KNOWN_FRONTMATTER_FIELDS` to `config.SPEC_FIELDS`. +- New `template.detected` rule wired into `rules/__init__.py`. +- New fixture set under `tests/fixtures/` covering ecosystem fields, user extensions, template detection, and demoted severities. + ## [1.1.0] - 2026-04-28 External audit against v1.0.1 surfaced eight repo defects ranging from documentation drift to a CI-confusing exit-code conflation. v1.1.0 ships fixes for all eight, reverses one v1.0.1 behavior change that turned out wrong, and tightens the description scorer's vague-word rubric. The minor bump is driven by the exit-code semantics change (now distinguishes warning-only from input error) and the new `--warnings-as-errors` flag. @@ -24,7 +51,7 @@ External audit against v1.0.1 surfaced eight repo defects ranging from documenta ### Changed - `action.yml` install step pins `skillcheck>=1.0.1` so consumers fail loudly on unpublished v1 features instead of silently running v0.2.0. -- Description scorer rubric documented and tightened: dropped `comprehensive`, `robust`, and `flexible` from `_VAGUE_WORDS` because each can describe a concrete attribute when qualified ("comprehensive coverage of N file formats", "robust against malformed input"). The inclusion rubric is now documented inline. Verified against `anthropics/skills` (17 SKILL.md files): zero score changes, because none of those skills use the dropped words. The rubric edit is a no-op against the current corpus; the new regression tests are forward-looking guards against scoring drift if the list is ever re-expanded. +- Description scorer rubric documented and tightened: dropped `comprehensive`, `flexible`, and the malformed-input term from `_VAGUE_WORDS` because each can describe a concrete attribute when qualified ("comprehensive coverage of N file formats", "handles malformed input"). The inclusion rubric is now documented inline. Verified against `anthropics/skills` (17 SKILL.md files): zero score changes, because none of those skills use the dropped words. The rubric edit is a no-op against the current corpus; the new regression tests are forward-looking guards against scoring drift if the list is ever re-expanded. - Description scorer verb matching: collapsed `_ACTION_VERBS` from 86 entries (base + 3rd-person duplicates) to 42 base forms. Added `_is_action_verb()` to handle stem normalization across `-s`, `-es`, and `-ies` endings. Adding a new verb now only requires the base form. - README test count bumped from 663 to 667 to include the drift-guard test, two description-scorer regression tests, and the `--warnings-as-errors` test. - README field-test citations: replaced seven gitignored `runs/...` path references with the exact `skillcheck` commands needed to reproduce each finding. Readers can now verify the claims without access to private artifacts. diff --git a/pyproject.toml b/pyproject.toml index 3cc8edf..7df73f8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "skillcheck" -version = "1.1.0" +version = "1.2.0" description = "Cross-agent skill quality gate for SKILL.md files conforming to the agentskills.io specification" readme = "README.md" license = { text = "MIT" } diff --git a/skills/skillcheck/SKILL.md b/skills/skillcheck/SKILL.md index e6a66b4..ae0345c 100644 --- a/skills/skillcheck/SKILL.md +++ b/skills/skillcheck/SKILL.md @@ -1,7 +1,7 @@ --- name: skillcheck description: Validates and scores SKILL.md files against the agentskills.io specification; use when linting skills for cross-agent compatibility, description quality, or capability graph structure. -version: "1.1.0" +version: "1.2.0" author: brad --- diff --git a/src/skillcheck/__init__.py b/src/skillcheck/__init__.py index 451acd1..ce14f27 100644 --- a/src/skillcheck/__init__.py +++ b/src/skillcheck/__init__.py @@ -2,7 +2,7 @@ from skillcheck.parser import ParsedSkill, ParseError from skillcheck.result import Diagnostic, Severity, ValidationResult -__version__ = "1.1.0" +__version__ = "1.2.0" __all__ = [ "validate", diff --git a/src/skillcheck/rules/description.py b/src/skillcheck/rules/description.py index 167b810..8f727e1 100644 --- a/src/skillcheck/rules/description.py +++ b/src/skillcheck/rules/description.py @@ -70,8 +70,8 @@ def _is_action_verb(word: str) -> bool: # Rubric for inclusion: a word qualifies as vague filler only if it adds # little or no domain signal in the contexts where it typically appears in # SKILL.md descriptions. Words that *can* describe a concrete attribute when -# qualified ("comprehensive coverage of N file formats", "robust against -# malformed input", "flexible CLI grammar") are deliberately excluded, even +# qualified ("comprehensive coverage of N file formats", "handles malformed +# input", "flexible CLI grammar") are deliberately excluded, even # if they often appear as marketing fluff. The cost of false positives is # high here: a real description that uses the word once gets penalized. _VAGUE_WORDS = frozenset({ diff --git a/tests/test_references.py b/tests/test_references.py index a47d9b5..6032507 100644 --- a/tests/test_references.py +++ b/tests/test_references.py @@ -8,7 +8,7 @@ _WINDOWS = sys.platform == "win32" _skip_symlink = pytest.mark.skipif( _WINDOWS, - reason="os.symlink requires developer mode / elevated privileges on Windows", + reason="os.symlink requires developer mode or admin privileges on Windows", ) from skillcheck.parser import parse From 5e7b413c8280bfbbada3f9f33bc85bdf9e168d56 Mon Sep 17 00:00:00 2001 From: Brad Kinnard Date: Tue, 28 Apr 2026 18:08:38 -0600 Subject: [PATCH 10/13] refactor: split frontmatter rule modules --- src/skillcheck/rules/frontmatter.py | 459 ++---------------- src/skillcheck/rules/frontmatter_common.py | 18 + .../rules/frontmatter_description.py | 138 ++++++ src/skillcheck/rules/frontmatter_fields.py | 83 ++++ src/skillcheck/rules/frontmatter_name.py | 179 +++++++ 5 files changed, 458 insertions(+), 419 deletions(-) create mode 100644 src/skillcheck/rules/frontmatter_common.py create mode 100644 src/skillcheck/rules/frontmatter_description.py create mode 100644 src/skillcheck/rules/frontmatter_fields.py create mode 100644 src/skillcheck/rules/frontmatter_name.py diff --git a/src/skillcheck/rules/frontmatter.py b/src/skillcheck/rules/frontmatter.py index eac26e9..0c5bee3 100644 --- a/src/skillcheck/rules/frontmatter.py +++ b/src/skillcheck/rules/frontmatter.py @@ -1,424 +1,45 @@ from __future__ import annotations -import re - -from skillcheck import config -from skillcheck.parser import ParsedSkill -from skillcheck.result import Diagnostic, Severity -from skillcheck.template_detection import is_template - -_XML_TAG_RE = re.compile(r"<[a-zA-Z/][^>]*>") -_NAME_VALID_CHARS_RE = re.compile(r"^[a-z0-9-]+$") - -# YAML anchor (&name) and alias (*name) patterns. -# Anchors define a reusable value; aliases reference it. When safe_load -# resolves ``description: *anchor``, the description silently becomes -# whatever the anchor pointed to, a subtle semantic trap that bypasses -# all downstream description-quality checks. -_YAML_ANCHOR_RE = re.compile(r"&([A-Za-z_][A-Za-z0-9_-]*)") -_YAML_ALIAS_RE = re.compile(r"\*([A-Za-z_][A-Za-z0-9_-]*)") - -# First-person patterns: "I can", "I will", "I'm", "My approach", etc. -# Catches subject "I" at sentence start, "I" before a verb, and possessive "My". -_FIRST_PERSON_RE = re.compile( - r"(?:(?:^|(?<=\.\s))I\b)" - r"|\bI (?:can|will|am|do|have|would|should|need|shall|won't|didn't|don't)\b" - r"|\bMy\b", - re.MULTILINE, +from skillcheck.rules.frontmatter_common import _field_line +from skillcheck.rules.frontmatter_description import ( + check_description_max_length, + check_description_no_xml_tags, + check_description_non_empty, + check_description_person_voice, + check_description_required, + check_description_type, ) - -# Second-person patterns: "You can", "you will", "you should", etc. -_SECOND_PERSON_RE = re.compile( - r"\b[Yy]ou (?:can|will|should|must|need|are|have|do|get|use)\b", +from skillcheck.rules.frontmatter_fields import ( + check_unknown_fields, + check_yaml_anchors, +) +from skillcheck.rules.frontmatter_name import ( + check_name_charset, + check_name_consecutive_hyphens, + check_name_directory_match, + check_name_leading_trailing_hyphen, + check_name_max_length, + check_name_required, + check_name_reserved_words, + check_name_type, ) - -def _field_line(raw_text: str, field: str) -> int | None: - """Return the 1-based line number where a frontmatter field appears. - - Only searches within the frontmatter block to avoid false positives from - body content that happens to start with a field name. - """ - lines = raw_text.splitlines() - if not lines or lines[0].strip() != "---": - return None - # Scan only until the closing --- delimiter. - for i, line in enumerate(lines[1:], 2): - if line.strip() == "---": - break - if line.lstrip().startswith(f"{field}:"): - return i - return None - - -def check_name_required(skill: ParsedSkill) -> list[Diagnostic]: - if skill.frontmatter.get("name") is None: - return [Diagnostic( - rule="frontmatter.name.required", - severity=Severity.ERROR, - message="Required field 'name' is missing from frontmatter.", - )] - return [] - - -def check_name_type(skill: ParsedSkill) -> list[Diagnostic]: - """Ensure ``name`` is a string, not a YAML-coerced boolean/number/null. - - ``yaml.safe_load`` silently converts bare values like ``true``, ``123``, - and ``null`` into Python ``bool``, ``int``, and ``None``. When this - happens, every downstream rule sees corrupted data (e.g., ``str(True)`` - becomes ``'True'`` and then fails charset checks for uppercase ``T``). - """ - name = skill.frontmatter.get("name") - if name is None: - return [] # handled by check_name_required - if isinstance(name, str): - return [] - yaml_type = type(name).__name__ - return [Diagnostic( - rule="frontmatter.name.type", - severity=Severity.ERROR, - message=( - f"Field 'name' must be a string but YAML parsed it as {yaml_type} " - f"({name!r}). Quote the value: name: \"{name}\"" - ), - line=_field_line(skill.raw_text, "name"), - )] - - -def check_description_type(skill: ParsedSkill) -> list[Diagnostic]: - """Ensure ``description`` is a string, not a YAML-coerced type.""" - desc = skill.frontmatter.get("description") - if desc is None: - return [] # handled by check_description_required - if isinstance(desc, str): - return [] - yaml_type = type(desc).__name__ - return [Diagnostic( - rule="frontmatter.description.type", - severity=Severity.ERROR, - message=( - f"Field 'description' must be a string but YAML parsed it as {yaml_type} " - f"({desc!r}). Quote the value: description: \"{desc}\"" - ), - line=_field_line(skill.raw_text, "description"), - )] - - -def check_name_max_length(skill: ParsedSkill) -> list[Diagnostic]: - name = skill.frontmatter.get("name") - if name is None: - return [] - name = str(name) - if len(name) > config.NAME_MAX_LENGTH: - return [Diagnostic( - rule="frontmatter.name.max-length", - severity=Severity.ERROR, - message=( - f"Name exceeds {config.NAME_MAX_LENGTH} characters " - f"(got {len(name)}): '{name}'" - ), - line=_field_line(skill.raw_text, "name"), - context=f"name: {name}", - )] - return [] - - -def check_name_charset(skill: ParsedSkill) -> list[Diagnostic]: - name = skill.frontmatter.get("name") - if name is None: - return [] - name = str(name) - if not name: - return [Diagnostic( - rule="frontmatter.name.invalid-chars", - severity=Severity.ERROR, - message="Name is empty. Use lowercase letters, numbers, and hyphens only.", - line=_field_line(skill.raw_text, "name"), - )] - if not _NAME_VALID_CHARS_RE.match(name): - invalid = sorted(set(c for c in name if not re.match(r"[a-z0-9-]", c))) - return [Diagnostic( - rule="frontmatter.name.invalid-chars", - severity=Severity.ERROR, - message=( - f"Name contains invalid characters {invalid}: '{name}'. " - f"Use lowercase letters, numbers, and hyphens only." - ), - line=_field_line(skill.raw_text, "name"), - context=f"name: {name}", - )] - return [] - - -def check_name_leading_trailing_hyphen(skill: ParsedSkill) -> list[Diagnostic]: - name = skill.frontmatter.get("name") - if name is None: - return [] - name = str(name) - if not name: - return [] - issues = [] - if name.startswith("-"): - issues.append("starts with a hyphen") - if name.endswith("-"): - issues.append("ends with a hyphen") - if issues: - return [Diagnostic( - rule="frontmatter.name.leading-trailing-hyphen", - severity=Severity.ERROR, - message=( - f"Name {' and '.join(issues)}: '{name}'. " - f"Hyphens are only allowed between characters." - ), - line=_field_line(skill.raw_text, "name"), - context=f"name: {name}", - )] - return [] - - -def check_name_consecutive_hyphens(skill: ParsedSkill) -> list[Diagnostic]: - name = skill.frontmatter.get("name") - if name is None: - return [] - name = str(name) - if "--" in name: - return [Diagnostic( - rule="frontmatter.name.consecutive-hyphens", - severity=Severity.ERROR, - message=( - f"Name contains consecutive hyphens: '{name}'. " - f"Use a single hyphen between words." - ), - line=_field_line(skill.raw_text, "name"), - context=f"name: {name}", - )] - return [] - - -def check_name_directory_match(skill: ParsedSkill) -> list[Diagnostic]: - # Skip on templates: placeholder files are not meant to deploy. - if is_template(skill): - return [] - name = skill.frontmatter.get("name") - if name is None: - return [] - name = str(name) - if not name: - return [] - parent_dir = skill.path.parent.name - if parent_dir and parent_dir != name: - return [Diagnostic( - rule="frontmatter.name.directory-mismatch", - severity=Severity.ERROR, - message=( - f"Name '{name}' does not match parent directory '{parent_dir}'. " - f"VS Code requires these to match or the skill will not load." - ), - line=_field_line(skill.raw_text, "name"), - context=f"name: {name} | directory: {parent_dir}", - )] - return [] - - -def check_name_reserved_words(skill: ParsedSkill) -> list[Diagnostic]: - name = skill.frontmatter.get("name") - if name is None: - return [] - name = str(name) - for word in ("anthropic", "claude"): - if word in name: - return [Diagnostic( - rule="frontmatter.name.reserved-word", - severity=Severity.WARNING, - message=( - f"Name contains the term '{word}' which may collide with " - f"platform-reserved namespaces. Verify with the target " - f"agent's documentation." - ), - line=_field_line(skill.raw_text, "name"), - context=f"name: {name}", - source="advisory", - confidence="low", - )] - return [] - - -def check_description_required(skill: ParsedSkill) -> list[Diagnostic]: - if "description" not in skill.frontmatter: - return [Diagnostic( - rule="frontmatter.description.required", - severity=Severity.ERROR, - message="Required field 'description' is missing from frontmatter.", - )] - return [] - - -def check_description_non_empty(skill: ParsedSkill) -> list[Diagnostic]: - if "description" not in skill.frontmatter: - return [] # already covered by check_description_required - desc = skill.frontmatter.get("description") - # Treat null (description:) and whitespace-only strings as empty. - if not desc or (isinstance(desc, str) and not desc.strip()): - return [Diagnostic( - rule="frontmatter.description.empty", - severity=Severity.ERROR, - message="Description is empty. Provide a meaningful description of the skill.", - line=_field_line(skill.raw_text, "description"), - context="description: (empty)", - )] - return [] - - -def check_description_max_length(skill: ParsedSkill) -> list[Diagnostic]: - desc = skill.frontmatter.get("description") - if not desc: - return [] - desc = str(desc) - if len(desc) > config.DESCRIPTION_MAX_LENGTH: - return [Diagnostic( - rule="frontmatter.description.max-length", - severity=Severity.ERROR, - message=( - f"Description exceeds {config.DESCRIPTION_MAX_LENGTH} characters " - f"(got {len(desc)})." - ), - line=_field_line(skill.raw_text, "description"), - )] - return [] - - -def check_description_no_xml_tags(skill: ParsedSkill) -> list[Diagnostic]: - desc = skill.frontmatter.get("description") - if not desc: - return [] - desc = str(desc) - tags_found = _XML_TAG_RE.findall(desc) - if tags_found: - return [Diagnostic( - rule="frontmatter.description.xml-tags", - severity=Severity.ERROR, - message=( - f"Description contains XML tags: {tags_found}. " - f"Remove markup from the description." - ), - line=_field_line(skill.raw_text, "description"), - )] - return [] - - -def check_description_person_voice(skill: ParsedSkill) -> list[Diagnostic]: - desc = skill.frontmatter.get("description") - if not desc: - return [] - desc = str(desc) - - first_match = _FIRST_PERSON_RE.search(desc) - if first_match: - return [Diagnostic( - rule="frontmatter.description.person-voice", - severity=Severity.WARNING, - message=( - f"Description appears to use first-person voice " - f"('{first_match.group().strip()}'); the spec recommends " - f"third-person for routing clarity." - ), - line=_field_line(skill.raw_text, "description"), - context=f"description: {desc[:80]}{'...' if len(desc) > 80 else ''}", - )] - - second_match = _SECOND_PERSON_RE.search(desc) - if second_match: - return [Diagnostic( - rule="frontmatter.description.person-voice", - severity=Severity.WARNING, - message=( - f"Description appears to use second-person voice " - f"('{second_match.group()}'); the spec recommends " - f"third-person for routing clarity. Verify the phrasing " - f"addresses the agent, not the user." - ), - line=_field_line(skill.raw_text, "description"), - context=f"description: {desc[:80]}{'...' if len(desc) > 80 else ''}", - )] - - return [] - - -def check_unknown_fields(skill: ParsedSkill) -> list[Diagnostic]: - diagnostics = [] - for field in skill.frontmatter: - field_name = str(field) - if field_name in config.SPEC_FIELDS or field_name in config.extension_fields: - continue - if field_name in config.ECOSYSTEM_FIELDS: - diagnostics.append(Diagnostic( - rule="frontmatter.field.ecosystem", - severity=Severity.INFO, - message=( - f"Field '{field_name}' is ecosystem-common but not in the " - f"agentskills.io spec. Add it to skillcheck.toml under " - f"[frontmatter] extension_fields if intentional." - ), - line=_field_line(skill.raw_text, field_name), - context=f"{field_name}: ...", - source="advisory", - confidence="medium", - )) - continue - diagnostics.append(Diagnostic( - rule="frontmatter.field.unknown", - severity=Severity.WARNING, - message=( - f"Unknown frontmatter field '{field_name}'. " - f"Known fields: {', '.join(sorted(config.SPEC_FIELDS))}." - ), - line=_field_line(skill.raw_text, field_name), - context=f"{field_name}: ...", - )) - return diagnostics - - -def _extract_frontmatter_raw(raw_text: str) -> str: - """Return the raw frontmatter text between ``---`` delimiters.""" - lines = raw_text.splitlines() - if not lines or lines[0].strip() != "---": - return "" - fm_lines: list[str] = [] - for line in lines[1:]: - if line.strip() == "---": - break - fm_lines.append(line) - return "\n".join(fm_lines) - - -def check_yaml_anchors(skill: ParsedSkill) -> list[Diagnostic]: - """Warn when YAML anchors or aliases are used in frontmatter. - - ``yaml.safe_load`` silently resolves anchors/aliases, which can cause - a field like ``description: *name_anchor`` to inherit the name value. - This bypasses description-quality checks and is almost always a mistake - in SKILL.md files. - """ - fm_raw = _extract_frontmatter_raw(skill.raw_text) - if not fm_raw: - return [] - - diagnostics: list[Diagnostic] = [] - - anchors = _YAML_ANCHOR_RE.findall(fm_raw) - aliases = _YAML_ALIAS_RE.findall(fm_raw) - - if anchors or aliases: - names = sorted(set(anchors + aliases)) - diagnostics.append(Diagnostic( - rule="frontmatter.yaml-anchors", - severity=Severity.WARNING, - message=( - f"YAML anchors/aliases detected in frontmatter ({', '.join(names)}). " - f"Anchors silently copy values between fields, which can bypass " - f"validation. Use explicit values instead." - ), - )) - - return diagnostics +__all__ = [ + "_field_line", + "check_description_max_length", + "check_description_no_xml_tags", + "check_description_non_empty", + "check_description_person_voice", + "check_description_required", + "check_description_type", + "check_name_charset", + "check_name_consecutive_hyphens", + "check_name_directory_match", + "check_name_leading_trailing_hyphen", + "check_name_max_length", + "check_name_required", + "check_name_reserved_words", + "check_name_type", + "check_unknown_fields", + "check_yaml_anchors", +] diff --git a/src/skillcheck/rules/frontmatter_common.py b/src/skillcheck/rules/frontmatter_common.py new file mode 100644 index 0000000..5a9480b --- /dev/null +++ b/src/skillcheck/rules/frontmatter_common.py @@ -0,0 +1,18 @@ +from __future__ import annotations + + +def _field_line(raw_text: str, field: str) -> int | None: + """Return the 1-based line number where a frontmatter field appears. + + Only searches within the frontmatter block to avoid false positives from + body content that happens to start with a field name. + """ + lines = raw_text.splitlines() + if not lines or lines[0].strip() != "---": + return None + for i, line in enumerate(lines[1:], 2): + if line.strip() == "---": + break + if line.lstrip().startswith(f"{field}:"): + return i + return None diff --git a/src/skillcheck/rules/frontmatter_description.py b/src/skillcheck/rules/frontmatter_description.py new file mode 100644 index 0000000..6a3564c --- /dev/null +++ b/src/skillcheck/rules/frontmatter_description.py @@ -0,0 +1,138 @@ +from __future__ import annotations + +import re + +from skillcheck import config +from skillcheck.parser import ParsedSkill +from skillcheck.result import Diagnostic, Severity +from skillcheck.rules.frontmatter_common import _field_line + +_XML_TAG_RE = re.compile(r"<[a-zA-Z/][^>]*>") +_FIRST_PERSON_RE = re.compile( + r"(?:(?:^|(?<=\.\s))I\b)" + r"|\bI (?:can|will|am|do|have|would|should|need|shall|won't|didn't|don't)\b" + r"|\bMy\b", + re.MULTILINE, +) +_SECOND_PERSON_RE = re.compile( + r"\b[Yy]ou (?:can|will|should|must|need|are|have|do|get|use)\b", +) + + +def check_description_type(skill: ParsedSkill) -> list[Diagnostic]: + """Ensure ``description`` is a string, not a YAML-coerced type.""" + desc = skill.frontmatter.get("description") + if desc is None: + return [] + if isinstance(desc, str): + return [] + yaml_type = type(desc).__name__ + return [Diagnostic( + rule="frontmatter.description.type", + severity=Severity.ERROR, + message=( + f"Field 'description' must be a string but YAML parsed it as {yaml_type} " + f"({desc!r}). Quote the value: description: \"{desc}\"" + ), + line=_field_line(skill.raw_text, "description"), + )] + + +def check_description_required(skill: ParsedSkill) -> list[Diagnostic]: + if "description" not in skill.frontmatter: + return [Diagnostic( + rule="frontmatter.description.required", + severity=Severity.ERROR, + message="Required field 'description' is missing from frontmatter.", + )] + return [] + + +def check_description_non_empty(skill: ParsedSkill) -> list[Diagnostic]: + if "description" not in skill.frontmatter: + return [] + desc = skill.frontmatter.get("description") + if not desc or (isinstance(desc, str) and not desc.strip()): + return [Diagnostic( + rule="frontmatter.description.empty", + severity=Severity.ERROR, + message="Description is empty. Provide a meaningful description of the skill.", + line=_field_line(skill.raw_text, "description"), + context="description: (empty)", + )] + return [] + + +def check_description_max_length(skill: ParsedSkill) -> list[Diagnostic]: + desc = skill.frontmatter.get("description") + if not desc: + return [] + desc = str(desc) + if len(desc) > config.DESCRIPTION_MAX_LENGTH: + return [Diagnostic( + rule="frontmatter.description.max-length", + severity=Severity.ERROR, + message=( + f"Description exceeds {config.DESCRIPTION_MAX_LENGTH} characters " + f"(got {len(desc)})." + ), + line=_field_line(skill.raw_text, "description"), + )] + return [] + + +def check_description_no_xml_tags(skill: ParsedSkill) -> list[Diagnostic]: + desc = skill.frontmatter.get("description") + if not desc: + return [] + desc = str(desc) + tags_found = _XML_TAG_RE.findall(desc) + if tags_found: + return [Diagnostic( + rule="frontmatter.description.xml-tags", + severity=Severity.ERROR, + message=( + f"Description contains XML tags: {tags_found}. " + f"Remove markup from the description." + ), + line=_field_line(skill.raw_text, "description"), + )] + return [] + + +def check_description_person_voice(skill: ParsedSkill) -> list[Diagnostic]: + desc = skill.frontmatter.get("description") + if not desc: + return [] + desc = str(desc) + + first_match = _FIRST_PERSON_RE.search(desc) + if first_match: + return [Diagnostic( + rule="frontmatter.description.person-voice", + severity=Severity.WARNING, + message=( + f"Description appears to use first-person voice " + f"('{first_match.group().strip()}'); the spec recommends " + f"third-person for routing clarity." + ), + line=_field_line(skill.raw_text, "description"), + context=f"description: {desc[:80]}{'...' if len(desc) > 80 else ''}", + )] + + second_match = _SECOND_PERSON_RE.search(desc) + if second_match: + return [Diagnostic( + rule="frontmatter.description.person-voice", + severity=Severity.WARNING, + message=( + f"Description appears to use second-person voice " + f"('{second_match.group()}'); the spec recommends " + f"third-person for routing clarity. Verify the phrasing " + f"addresses the agent, not the user." + ), + line=_field_line(skill.raw_text, "description"), + context=f"description: {desc[:80]}{'...' if len(desc) > 80 else ''}", + )] + + return [] diff --git a/src/skillcheck/rules/frontmatter_fields.py b/src/skillcheck/rules/frontmatter_fields.py new file mode 100644 index 0000000..3a89689 --- /dev/null +++ b/src/skillcheck/rules/frontmatter_fields.py @@ -0,0 +1,83 @@ +from __future__ import annotations + +import re + +from skillcheck import config +from skillcheck.parser import ParsedSkill +from skillcheck.result import Diagnostic, Severity +from skillcheck.rules.frontmatter_common import _field_line + +_YAML_ANCHOR_RE = re.compile(r"&([A-Za-z_][A-Za-z0-9_-]*)") +_YAML_ALIAS_RE = re.compile(r"\*([A-Za-z_][A-Za-z0-9_-]*)") + + +def check_unknown_fields(skill: ParsedSkill) -> list[Diagnostic]: + diagnostics = [] + for field in skill.frontmatter: + field_name = str(field) + if field_name in config.SPEC_FIELDS or field_name in config.extension_fields: + continue + if field_name in config.ECOSYSTEM_FIELDS: + diagnostics.append(Diagnostic( + rule="frontmatter.field.ecosystem", + severity=Severity.INFO, + message=( + f"Field '{field_name}' is ecosystem-common but not in the " + f"agentskills.io spec. Add it to skillcheck.toml under " + f"[frontmatter] extension_fields if intentional." + ), + line=_field_line(skill.raw_text, field_name), + context=f"{field_name}: ...", + source="advisory", + confidence="medium", + )) + continue + diagnostics.append(Diagnostic( + rule="frontmatter.field.unknown", + severity=Severity.WARNING, + message=( + f"Unknown frontmatter field '{field_name}'. " + f"Known fields: {', '.join(sorted(config.SPEC_FIELDS))}." + ), + line=_field_line(skill.raw_text, field_name), + context=f"{field_name}: ...", + )) + return diagnostics + + +def _extract_frontmatter_raw(raw_text: str) -> str: + """Return the raw frontmatter text between ``---`` delimiters.""" + lines = raw_text.splitlines() + if not lines or lines[0].strip() != "---": + return "" + fm_lines: list[str] = [] + for line in lines[1:]: + if line.strip() == "---": + break + fm_lines.append(line) + return "\n".join(fm_lines) + + +def check_yaml_anchors(skill: ParsedSkill) -> list[Diagnostic]: + """Warn when YAML anchors or aliases are used in frontmatter.""" + fm_raw = _extract_frontmatter_raw(skill.raw_text) + if not fm_raw: + return [] + + diagnostics: list[Diagnostic] = [] + anchors = _YAML_ANCHOR_RE.findall(fm_raw) + aliases = _YAML_ALIAS_RE.findall(fm_raw) + + if anchors or aliases: + names = sorted(set(anchors + aliases)) + diagnostics.append(Diagnostic( + rule="frontmatter.yaml-anchors", + severity=Severity.WARNING, + message=( + f"YAML anchors/aliases detected in frontmatter ({', '.join(names)}). " + f"Anchors silently copy values between fields, which can bypass " + f"validation. Use explicit values instead." + ), + )) + + return diagnostics diff --git a/src/skillcheck/rules/frontmatter_name.py b/src/skillcheck/rules/frontmatter_name.py new file mode 100644 index 0000000..85f76b4 --- /dev/null +++ b/src/skillcheck/rules/frontmatter_name.py @@ -0,0 +1,179 @@ +from __future__ import annotations + +import re + +from skillcheck import config +from skillcheck.parser import ParsedSkill +from skillcheck.result import Diagnostic, Severity +from skillcheck.rules.frontmatter_common import _field_line +from skillcheck.template_detection import is_template + +_NAME_VALID_CHARS_RE = re.compile(r"^[a-z0-9-]+$") + + +def check_name_required(skill: ParsedSkill) -> list[Diagnostic]: + if skill.frontmatter.get("name") is None: + return [Diagnostic( + rule="frontmatter.name.required", + severity=Severity.ERROR, + message="Required field 'name' is missing from frontmatter.", + )] + return [] + + +def check_name_type(skill: ParsedSkill) -> list[Diagnostic]: + """Ensure ``name`` is a string, not a YAML-coerced boolean/number/null.""" + name = skill.frontmatter.get("name") + if name is None: + return [] + if isinstance(name, str): + return [] + yaml_type = type(name).__name__ + return [Diagnostic( + rule="frontmatter.name.type", + severity=Severity.ERROR, + message=( + f"Field 'name' must be a string but YAML parsed it as {yaml_type} " + f"({name!r}). Quote the value: name: \"{name}\"" + ), + line=_field_line(skill.raw_text, "name"), + )] + + +def check_name_max_length(skill: ParsedSkill) -> list[Diagnostic]: + name = skill.frontmatter.get("name") + if name is None: + return [] + name = str(name) + if len(name) > config.NAME_MAX_LENGTH: + return [Diagnostic( + rule="frontmatter.name.max-length", + severity=Severity.ERROR, + message=( + f"Name exceeds {config.NAME_MAX_LENGTH} characters " + f"(got {len(name)}): '{name}'" + ), + line=_field_line(skill.raw_text, "name"), + context=f"name: {name}", + )] + return [] + + +def check_name_charset(skill: ParsedSkill) -> list[Diagnostic]: + name = skill.frontmatter.get("name") + if name is None: + return [] + name = str(name) + if not name: + return [Diagnostic( + rule="frontmatter.name.invalid-chars", + severity=Severity.ERROR, + message="Name is empty. Use lowercase letters, numbers, and hyphens only.", + line=_field_line(skill.raw_text, "name"), + )] + if not _NAME_VALID_CHARS_RE.match(name): + invalid = sorted(set(c for c in name if not re.match(r"[a-z0-9-]", c))) + return [Diagnostic( + rule="frontmatter.name.invalid-chars", + severity=Severity.ERROR, + message=( + f"Name contains invalid characters {invalid}: '{name}'. " + f"Use lowercase letters, numbers, and hyphens only." + ), + line=_field_line(skill.raw_text, "name"), + context=f"name: {name}", + )] + return [] + + +def check_name_leading_trailing_hyphen(skill: ParsedSkill) -> list[Diagnostic]: + name = skill.frontmatter.get("name") + if name is None: + return [] + name = str(name) + if not name: + return [] + issues = [] + if name.startswith("-"): + issues.append("starts with a hyphen") + if name.endswith("-"): + issues.append("ends with a hyphen") + if issues: + return [Diagnostic( + rule="frontmatter.name.leading-trailing-hyphen", + severity=Severity.ERROR, + message=( + f"Name {' and '.join(issues)}: '{name}'. " + f"Hyphens are only allowed between characters." + ), + line=_field_line(skill.raw_text, "name"), + context=f"name: {name}", + )] + return [] + + +def check_name_consecutive_hyphens(skill: ParsedSkill) -> list[Diagnostic]: + name = skill.frontmatter.get("name") + if name is None: + return [] + name = str(name) + if "--" in name: + return [Diagnostic( + rule="frontmatter.name.consecutive-hyphens", + severity=Severity.ERROR, + message=( + f"Name contains consecutive hyphens: '{name}'. " + f"Use a single hyphen between words." + ), + line=_field_line(skill.raw_text, "name"), + context=f"name: {name}", + )] + return [] + + +def check_name_directory_match(skill: ParsedSkill) -> list[Diagnostic]: + # Skip on templates: placeholder files are not meant to deploy. + if is_template(skill): + return [] + name = skill.frontmatter.get("name") + if name is None: + return [] + name = str(name) + if not name: + return [] + parent_dir = skill.path.parent.name + if parent_dir and parent_dir != name: + return [Diagnostic( + rule="frontmatter.name.directory-mismatch", + severity=Severity.ERROR, + message=( + f"Name '{name}' does not match parent directory '{parent_dir}'. " + f"VS Code requires these to match or the skill will not load." + ), + line=_field_line(skill.raw_text, "name"), + context=f"name: {name} | directory: {parent_dir}", + )] + return [] + + +def check_name_reserved_words(skill: ParsedSkill) -> list[Diagnostic]: + name = skill.frontmatter.get("name") + if name is None: + return [] + name = str(name) + for word in ("anthropic", "claude"): + if word in name: + return [Diagnostic( + rule="frontmatter.name.reserved-word", + severity=Severity.WARNING, + message=( + f"Name contains the term '{word}' which may collide with " + f"platform-reserved namespaces. Verify with the target " + f"agent's documentation." + ), + line=_field_line(skill.raw_text, "name"), + context=f"name: {name}", + source="advisory", + confidence="low", + )] + return [] From 54e1c251ea5eeb6f324561a0c58c990f009e0505 Mon Sep 17 00:00:00 2001 From: Brad Kinnard Date: Tue, 28 Apr 2026 18:09:29 -0600 Subject: [PATCH 11/13] docs: note frontmatter module split --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50bc73b..41419ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ Backward compatibility: previously-passing skills still pass. Some previously-fa - Renamed `config.KNOWN_FRONTMATTER_FIELDS` to `config.SPEC_FIELDS`. - New `template.detected` rule wired into `rules/__init__.py`. +- Frontmatter rule implementation split into smaller modules while preserving `skillcheck.rules.frontmatter` imports. - New fixture set under `tests/fixtures/` covering ecosystem fields, user extensions, template detection, and demoted severities. ## [1.1.0] - 2026-04-28 From 859220bfa545ebf2cd14d2f1f015b36e5a5f10b2 Mon Sep 17 00:00:00 2001 From: Brad Kinnard Date: Tue, 28 Apr 2026 18:12:23 -0600 Subject: [PATCH 12/13] chore: add root self-validation skill --- CHANGELOG.md | 1 + SKILL.md | 44 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 SKILL.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 41419ea..5c229c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ Backward compatibility: previously-passing skills still pass. Some previously-fa - Renamed `config.KNOWN_FRONTMATTER_FIELDS` to `config.SPEC_FIELDS`. - New `template.detected` rule wired into `rules/__init__.py`. - Frontmatter rule implementation split into smaller modules while preserving `skillcheck.rules.frontmatter` imports. +- Root `SKILL.md` restored so `skillcheck SKILL.md` self-validation works from the repository root. - New fixture set under `tests/fixtures/` covering ecosystem fields, user extensions, template detection, and demoted severities. ## [1.1.0] - 2026-04-28 diff --git a/SKILL.md b/SKILL.md new file mode 100644 index 0000000..698856d --- /dev/null +++ b/SKILL.md @@ -0,0 +1,44 @@ +--- +name: skillcheck +description: Validates SKILL.md files for spec-facing structure, sizing, references, cross-agent compatibility, and agent-authored critique or graph diagnostics. +version: "1.2.0" +author: brad +--- + +Use this skill when validating a `SKILL.md` file or a directory of skill files with the `skillcheck` CLI. + +## Validate + +Run the default symbolic checks: + +```bash +skillcheck SKILL.md +skillcheck skills/ --format json +``` + +The report includes errors, warnings, and info diagnostics. Exit code 0 means no errors. Exit code 1 means at least one error, or warnings with `--warnings-as-errors`. Exit code 2 means input or argument error. Exit code 3 means symbolic validation passed but ingested agent critique found semantic errors. + +## Agent workflows + +For semantic self-critique, emit a prompt and ingest the returned JSON: + +```bash +skillcheck SKILL.md --emit-critique-prompt > prompt.txt +skillcheck SKILL.md --ingest-critique response.json +``` + +For capability graph work, use: + +```bash +skillcheck SKILL.md --analyze-graph +skillcheck SKILL.md --emit-graph --format json +``` + +## Configuration + +`skillcheck.toml` can set CLI defaults and frontmatter extension fields: + +```toml +[frontmatter] +extension_fields = ["my-org-tag", "internal-id"] +``` From 182f9c4ca5b534f33c69ac64e7d2d58f5c123853 Mon Sep 17 00:00:00 2001 From: Brad Kinnard Date: Tue, 28 Apr 2026 18:19:42 -0600 Subject: [PATCH 13/13] refactor: tidy frontmatter rule imports --- src/skillcheck/rules/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/skillcheck/rules/__init__.py b/src/skillcheck/rules/__init__.py index 22aad8c..2e2c67e 100644 --- a/src/skillcheck/rules/__init__.py +++ b/src/skillcheck/rules/__init__.py @@ -26,6 +26,7 @@ check_description_non_empty, check_description_person_voice, check_description_required, + check_description_type, check_name_charset, check_name_consecutive_hyphens, check_name_directory_match, @@ -34,7 +35,6 @@ check_name_required, check_name_reserved_words, check_name_type, - check_description_type, check_unknown_fields, check_yaml_anchors, )