fix: validate skill names and duplicate frontmatter keys

CHANGELOG.md

@@ -6,6 +6,10 @@ All notable changes to this project are documented here.
 
 ## [Unreleased]
 
+### Fixed
+
+- Require explicit skill names matching their directories and reject duplicate frontmatter keys, including mixed quoted/unquoted keys, while retaining required names in every generated distribution (#259).
+
 ### Added
 
 - Add machine-readable `--json` output to `avoid-ai-writing-gate` and expose `pass`, `total-findings`, and `failed-files` step outputs in the GitHub Action (#252).

scripts/flatten-skill.test.js

@@ -7,6 +7,7 @@ const {flatten} = require('./flatten-skill');
 const root = path.resolve(__dirname, '..');
 const normalized = p => fs.readFileSync(p, 'utf8').replace(/\r\n/g, '\n');
 assert.equal(flatten(root), normalized(path.join(root, 'SKILL.full.md')), 'Flattened artifact must equal canonical content');
+assert.match(flatten(root).split('\n---\n')[0], /^name: avoid-ai-writing$/m, 'Portable skill retains its required name');
 assert.ok(normalized(path.join(root, 'SKILL.md')).split('\n').length < 500, 'Entry skill stays below 500 lines');
 // Both single-file targets must carry the same portable instructions.
 const portableBody = file => normalized(path.join(root, file))

scripts/validate-openai-plugin.py

@@ -12,9 +12,6 @@
 
 SEMVER = re.compile(r"^\d+\.\d+\.\d+(?:-[0-9A-Za-z.-]+)?(?:\+[0-9A-Za-z.-]+)?$")
 FRONTMATTER = re.compile(r"\A---\s*\n(.*?)\n---\s*\n(.*)\Z", re.S)
-# The top-level `metadata` key only: `metadata:` at column 0 followed by
-# whitespace or end of line, so `metadata:extra:` (a different plain key) is kept.
-METADATA_KEY = re.compile(r"metadata:(?:\s|$)")
 TOP_LEVEL_INCLUDE_FILES = ("OPENAI_PLUGIN.md", "NOTICE.md", "PRIVACY.md", "TERMS.md", "SUPPORT.md", "LICENSE")
 CANONICAL_PROJECT_URL = "https://github.com/conorbronsdon/avoid-ai-writing"
 MAX_SVG_BYTES = 256 * 1024
@@ -30,13 +27,44 @@ def parse_frontmatter(path: Path):
     if not match:
         return {}, ""
     meta = {}
-    for line in match.group(1).splitlines():
-        if ":" not in line or line.startswith((" ", "\t")):
+    for key, value in frontmatter_entries(match.group(1)):
+        if key is None:
             continue
-        key, value = line.split(":", 1)
-        meta[key.strip()] = value.strip().strip('"').strip("'")
+        parsed = parse_supported_yaml_scalar(value)
+        meta[key] = parsed if parsed is not None else value.strip()
     return meta, match.group(2).strip()
 
+def frontmatter_inner(text: str) -> str | None:
+    match = FRONTMATTER.match(text)
+    return match.group(1) if match else None
+
+
+def frontmatter_entries(inner: str):
+    """Normalize supported scalar keys; yield None for unsupported top-level syntax."""
+    for line in inner.splitlines():
+        if not line or line[0] in (" ", "\t", "#"):
+            continue
+        match = re.fullmatch(r'''("(?:\\.|[^"\\])*"|'(?:''|[^'])*'|[A-Za-z_][A-Za-z0-9_-]*)[ \t]*:(.*)''', line)
+        if match:
+            # JSON-style quoted YAML keys permit a value directly after ':'.
+            if match.group(2) and not match.group(2)[0].isspace() and match.group(1)[0] not in ("'", '"'):
+                yield None, ""
+                continue
+            yield parse_supported_yaml_scalar(match.group(1)), match.group(2) or ""
+        else:
+            yield None, ""
+
+
+def duplicate_top_level_frontmatter_keys(inner: str) -> list[str]:
+    """Return repeated keys within one frontmatter mapping, not across copies."""
+    counts: dict[str, int] = {}
+    for key, _ in frontmatter_entries(inner):
+        if key is None:
+            continue
+        counts[key] = counts.get(key, 0) + 1
+    return sorted(key for key, count in counts.items() if count > 1)
+
+
 def strip_frontmatter_metadata(text: str) -> str:
     """Drop the top-level `metadata` block from SKILL.md frontmatter, byte-exact otherwise.
 
@@ -52,7 +80,7 @@ def strip_frontmatter_metadata(text: str) -> str:
     inner = match.group(1)
     kept, skip = [], False
     for line in inner.splitlines(keepends=True):
-        if METADATA_KEY.match(line):
+        if any(key == "metadata" for key, _ in frontmatter_entries(line)):
             skip = True
             continue
         if skip and (line[:1] in (" ", "\t", "#") or line.strip() == ""):
@@ -74,7 +102,7 @@ def strip_frontmatter_metadata(text: str) -> str:
 
 def frontmatter_has_metadata(path: Path) -> bool:
     match = FRONTMATTER.match(path.read_text(encoding="utf-8"))
-    return bool(match) and any(METADATA_KEY.match(line) for line in match.group(1).split("\n"))
+    return bool(match) and any(key == "metadata" for key, _ in frontmatter_entries(match.group(1)))
 
 
 def safe_rel(value: str) -> bool:
@@ -503,9 +531,20 @@ def validate(root: Path):
             errors.append(f"{skill_dir}: missing SKILL.md")
             continue
         meta, body = parse_frontmatter(skill_path)
-        name, desc = meta.get("name", ""), meta.get("description", "")
+        inner = frontmatter_inner(skill_path.read_text(encoding="utf-8"))
+        if inner:
+            if any(key is None for key, _ in frontmatter_entries(inner)):
+                errors.append(f"{skill_path}: unsupported top-level frontmatter key syntax")
+            for key in duplicate_top_level_frontmatter_keys(inner):
+                errors.append(f"{skill_path}: duplicate frontmatter key: {key}")
+        name = meta.get("name", "")
+        desc = meta.get("description", "")
         if not name or not desc or not body:
             errors.append(f"{skill_path}: name, description, and body are required")
+        if meta.get("name") and meta.get("name") != skill_dir.name:
+            errors.append(
+                f"{skill_path}: frontmatter name {meta.get('name')!r} must match directory {skill_dir.name!r}"
+            )
         if frontmatter_has_metadata(skill_path):
             errors.append(
                 f"{skill_path}: `metadata` in SKILL.md frontmatter is rejected by the OpenAI plugin portal; "
@@ -526,7 +565,16 @@ def validate(root: Path):
         if not openai_copy.is_file():
             errors.append("skills/avoid-ai-writing/SKILL.md missing; cannot check drift from root SKILL.md")
         elif strip_frontmatter_metadata(canonical.read_bytes().decode("utf-8")).encode("utf-8") != openai_copy.read_bytes():
-            errors.append("skills/avoid-ai-writing/SKILL.md drifted from root SKILL.md (expected: root minus the frontmatter `metadata` block)")
+            errors.append(
+                "skills/avoid-ai-writing/SKILL.md drifted from root SKILL.md "
+                "(expected: root minus the frontmatter `metadata` block)"
+            )
+        canonical_inner = frontmatter_inner(canonical.read_text(encoding="utf-8"))
+        if canonical_inner:
+            if any(key is None for key, _ in frontmatter_entries(canonical_inner)):
+                errors.append(f"{canonical}: unsupported top-level frontmatter key syntax")
+            for key in duplicate_top_level_frontmatter_keys(canonical_inner):
+                errors.append(f"{canonical}: duplicate frontmatter key: {key}")
         meta, _ = parse_frontmatter(canonical)
         if meta.get("version") != version:
             errors.append(f"canonical SKILL.md version {meta.get('version')!r} does not match manifest {version!r}")
@@ -685,7 +733,7 @@ def main():
     parser.add_argument(
         "--strip-frontmatter-metadata",
         metavar="SKILL_MD",
-        help="print SKILL_MD with the frontmatter `metadata` block removed (used by sync-plugin-skill.sh) and exit",
+        help="print SKILL_MD with the frontmatter `metadata` block removed and exit",
     )
     args = parser.parse_args()
     if args.strip_frontmatter_metadata:

scripts/validate-openai-plugin.test.py

@@ -95,6 +95,48 @@ def validation_errors_for(*, manifest=None, tests=None, listing=None, pack=None)
         return errors
 
 
+def skill_name_errors(replacement: str):
+    with tempfile.TemporaryDirectory() as temp_dir:
+        root = Path(temp_dir)
+        make_valid_plugin_root(root)
+        skill = root / "skills" / "voice-preserving-rewriter" / "SKILL.md"
+        text = skill.read_text(encoding="utf-8")
+        original = "name: voice-preserving-rewriter"
+        assert original in text
+        skill.write_text(text.replace(original, replacement, 1), encoding="utf-8")
+        return MODULE.validate(root)[0]
+
+
+for key in ("name", '"name"', "'name'", r'"\u006eame"'):
+    assert skill_name_errors(f'{key}: "voice-preserving-rewriter" # identity') == []
+    errors = skill_name_errors(f"{key}: wrong-name")
+    assert any("must match directory" in error for error in errors), errors
+    errors = skill_name_errors(f"name: voice-preserving-rewriter\n{key}: voice-preserving-rewriter")
+    assert any("duplicate frontmatter key: name" in error for error in errors), errors
+
+for replacement in ("", "name:", "name: ''", '"name": ""'):
+    errors = skill_name_errors(replacement)
+    assert any("name, description, and body are required" in error for error in errors), errors
+
+assert skill_name_errors('"name":"voice-preserving-rewriter"') == []
+errors = skill_name_errors('name: voice-preserving-rewriter\n"name" :"wrong-name"')
+assert any("duplicate frontmatter key: name" in error for error in errors), errors
+assert any("must match directory" in error for error in errors), errors
+
+for key_line in (r'"\x6eame": wrong-name', '? name\n: wrong-name'):
+    errors = skill_name_errors('name: voice-preserving-rewriter\n' + key_line)
+    assert any("unsupported top-level frontmatter key syntax" in error for error in errors), errors
+
+errors = skill_name_errors('"name": false-positive-reviewer')
+assert any("duplicate skill name 'false-positive-reviewer'" in error for error in errors), errors
+assert MODULE.duplicate_top_level_frontmatter_keys(
+    'name: first\nmetadata:\n  name: nested\n  name: nested-again\n# name: comment\n'
+) == []
+assert MODULE.duplicate_top_level_frontmatter_keys(
+    'description: first\n"description": second\n'
+) == ["description"]
+
+
 assert errors_for("  products: [CHAT, CODEX]\n") == []
 assert errors_for('  products: ["CHAT", "CODEX"]\n') == []
 assert errors_for("  products: ['CHAT', 'CODEX']\n") == []
@@ -245,18 +287,24 @@ def validation_errors_for(*, manifest=None, tests=None, listing=None, pack=None)
 with tempfile.TemporaryDirectory() as temp_dir:
     root = Path(temp_dir)
     make_valid_plugin_root(root)
-    skill_dir = root / "skills" / "missing-name"
-    skill_dir.mkdir()
+    skill_dir = root / "skills" / "avoid-ai-writing"
     skill_path = skill_dir / "SKILL.md"
     skill_path.write_text(
-        "---\ndescription: Fixture without a name\n---\n# Missing name\n\nTest body.\n",
+        "---\nname: avoid-ai-writing\n---\n# Missing description\n\nTest body.\n",
         encoding="utf-8",
     )
-    agents_dir = skill_dir / "agents"
-    agents_dir.mkdir()
-    (agents_dir / "openai.yaml").write_text(PREFIX + "  products: [CHAT]\n", encoding="utf-8")
     errors, _, _ = MODULE.validate(root)
-    assert errors == [f"{skill_path}: name, description, and body are required"]
+    assert f"{skill_path}: name, description, and body are required" in errors
+
+with tempfile.TemporaryDirectory() as temp_dir:
+    root = Path(temp_dir)
+    make_valid_plugin_root(root)
+    skill_dir = root / "skills" / "avoid-ai-writing"
+    skill_path = skill_dir / "SKILL.md"
+    text = skill_path.read_text(encoding="utf-8")
+    skill_path.write_text(text.replace("name: avoid-ai-writing\n", "", 1), encoding="utf-8")
+    errors, _, _ = MODULE.validate(root)
+    assert any("name, description, and body are required" in error for error in errors), errors
 
 for payload in ("[]", "null"):
     with tempfile.TemporaryDirectory() as temp_dir:
@@ -367,13 +415,14 @@ def validation_errors_for(*, manifest=None, tests=None, listing=None, pack=None)
         assert any("symlink not allowed in plugin surface: LICENSE" in error for error in errors)
 
 # The OpenAI copy of the canonical skill must drop the frontmatter `metadata`
-# block (the portal rejects it) and otherwise match root SKILL.md exactly.
+# block (the portal rejects it) and preserve the required `name` field.
 ROOT_SKILL = (REPO_ROOT / "SKILL.md").read_text(encoding="utf-8")
 assert "\nmetadata:\n" in ROOT_SKILL, "fixture assumption: root SKILL.md carries a metadata block"
 STRIPPED = MODULE.strip_frontmatter_metadata(ROOT_SKILL)
 ROOT_HEAD, ROOT_BODY = ROOT_SKILL.split("\n---\n", 1)
 STRIPPED_HEAD, STRIPPED_BODY = STRIPPED.split("\n---\n", 1)
 assert "metadata:" not in STRIPPED_HEAD
+assert "name: avoid-ai-writing" in STRIPPED_HEAD
 assert "\nversion:" in STRIPPED_HEAD and "\nlicense:" in STRIPPED_HEAD and "\ncompatibility:" in STRIPPED_HEAD
 assert STRIPPED_BODY == ROOT_BODY, "body must be untouched"
 assert MODULE.strip_frontmatter_metadata("no frontmatter\nmetadata:\n  x: y\n") == "no frontmatter\nmetadata:\n  x: y\n"
@@ -389,18 +438,20 @@ def validation_errors_for(*, manifest=None, tests=None, listing=None, pack=None)
 # a column-zero comment inside the block belongs to it; `metadata:extra` is a different key and stays
 assert STRIP("---\nname: x\nmetadata:\n  author: y\n# note\n  repository: z\nlicense: MIT\n---\nBody\n") == "---\nname: x\nlicense: MIT\n---\nBody\n"
 assert STRIP("---\nname: x\nmetadata:extra: keep\n---\nBody\n") == "---\nname: x\nmetadata:extra: keep\n---\nBody\n"
+assert STRIP('---\nname: x\n"metadata":\n  author: y\n---\nBody\n') == '---\nname: x\n---\nBody\n'
 # missing closing delimiter: not a frontmatter, untouched
 assert STRIP("---\nname: x\nmetadata:\n  author: y\nBody\n") == "---\nname: x\nmetadata:\n  author: y\nBody\n"
-# the CLI path sync-plugin-skill.sh uses must be byte-exact with the function
+# sync-plugin-skill.sh uses metadata-only stripping and keeps `name`
 import subprocess
-cli = subprocess.run(
+metadata_only = subprocess.run(
     [sys.executable, str(MODULE_PATH), "--strip-frontmatter-metadata", str(REPO_ROOT / "SKILL.md")],
     capture_output=True, check=True,
 )
-assert cli.stdout == STRIPPED.encode("utf-8"), "CLI output differs from strip_frontmatter_metadata"
+assert metadata_only.stdout == MODULE.strip_frontmatter_metadata(ROOT_SKILL).encode("utf-8")
 assert STRIPPED == (REPO_ROOT / "skills" / "avoid-ai-writing" / "SKILL.md").read_text(encoding="utf-8"), (
     "run bash scripts/sync-plugin-skill.sh; the OpenAI copy is out of date"
 )
+assert "name: avoid-ai-writing" in (REPO_ROOT / "SKILL.full.md").read_text(encoding="utf-8").split("\n---\n", 1)[0]
 
 with tempfile.TemporaryDirectory() as temp_dir:
     root = Path(temp_dir)