fix: make the manifest validator agree with the store it validates

.github/workflows/validate_raw_jsons.yml

@@ -2,14 +2,16 @@ name: Validate Skill Entries
 
 # Runs the store's submission validator on every raw_jsons/ entry that a pull
 # request adds or changes, so a pull request that adds a skill gets a check
-# before merge.
+# before merge, and then over the WHOLE directory, so a change to the rules
+# cannot quietly invalidate entries it does not touch.
 
 on:
   pull_request:
     paths:
       - 'raw_jsons/**'
       - 'scripts/apply_submission.py'
       - 'scripts/validate_raw_jsons.py'
+      - 'scripts/check_all_raw_jsons.py'
       - '.github/workflows/validate_raw_jsons.yml'
 
 permissions:
@@ -42,3 +44,6 @@ jobs:
         run: |
           mapfile -t entries < "$RUNNER_TEMP/entries.txt"
           python scripts/validate_raw_jsons.py "${entries[@]}"
+
+      - name: Validate every committed entry
+        run: python scripts/check_all_raw_jsons.py

raw_jsons/skill-spelling.json → raw_jsons/ovos-skill-spelling-openvoiceos.json

@@ -13,10 +13,16 @@
   "icon": "FaBookReader",
   "name": "Spelling",
   "description": "Let OVOS help you spell words. OVOS can spell any word which is understood by speech-to-text. The proper spelling is pronounced on all platforms, as well as displayed by devices such as the Mark 1.",
+  "examples": [
+    "spell hello",
+    "how do you spell serendipity",
+    "what is the spelling of onomatopoeia",
+    "spell the word rhythm"
+  ],
   "category": "Information",
   "tags": [
     "spell",
     "spelling"
   ],
   "images": []
-}
+}

raw_jsons/skill-easter-eggs.json → raw_jsons/skill-easter-eggs-jarbasai.json

@@ -27,5 +27,9 @@
     "What would GLaDOS say?",
     "What would Bender say?"
   ],
-  "tags": []
-}
+  "tags": [
+    "easter eggs",
+    "fun",
+    "references"
+  ]
+}

raw_jsons/skill-audio-recording.json → raw_jsons/skill-ovos-audio-recording-openvoiceos.json

@@ -19,5 +19,10 @@
     "start recording",
     "new recording named {file_name}"
   ],
-  "tags": []
-}
+  "tags": [
+    "recording",
+    "audio",
+    "voice memo",
+    "dictation"
+  ]
+}

raw_jsons/skill-wallpapers.json → raw_jsons/skill-ovos-wallpapers-openvoiceos.json

@@ -28,5 +28,10 @@
     "show me a picture with dogs",
     "make that my wallpaper"
   ],
-  "tags": []
-}
+  "tags": [
+    "wallpaper",
+    "background",
+    "pictures",
+    "reddit"
+  ]
+}

scripts/apply_submission.py

@@ -22,10 +22,17 @@ class ValidationError(Exception):
 
 
 # Required fields with their validators
+# A react-icons component name: a two-or-more letter set prefix (Fa, Md, Si,
+# Ri, Fi, ...) followed by the icon name in CamelCase.
+ICON_NAME_RE = re.compile(r'^[A-Z][a-z]{1,3}[A-Z][A-Za-z0-9]+$')
+
 REQUIRED_FIELDS = {
     'skill_id': {
         'type': str,
-        'pattern': r'^[a-z0-9-]+\.[a-z0-9.-]+$',
+        'pattern': r'^[a-z0-9_-]+\.[a-z0-9._-]+$',
+        # underscores are legal: a skill id is often derived from a Python
+        # package name, and neon_homeassistant_skill.mikejgray is a real
+        # committed entry the old pattern rejected.
         'message': 'skill_id must be lowercase with format: skill-name.domain',
     },
     'source': {
@@ -141,14 +148,27 @@ def validate_submission(data: dict) -> list:
         if data['license'].lower() not in ALLOWED_LICENSES:
             errors.append(f"license must be one of: {', '.join(ALLOWED_LICENSES)}")
 
-    # Validate icon URL if present
+    # Validate the icon if present. The store renders `icon` as a react-icons
+    # component name (SkillCard.tsx resolves it against react-icons, and
+    # IconSelector.tsx offers those names on the submission form), so
+    # "FaMicrophone" and "MdWallpaper" are the convention every committed
+    # entry uses. A URL is still accepted, because entries predating the
+    # marketplace migration used one.
     if 'icon' in data and data['icon']:
-        try:
-            result = urlparse(data['icon'])
-            if not all([result.scheme, result.netloc]):
-                errors.append("icon must be a valid URL")
-        except Exception:
-            errors.append("icon must be a valid URL")
+        icon = data['icon']
+        if not isinstance(icon, str):
+            errors.append("icon must be a string")
+        elif ICON_NAME_RE.match(icon):
+            pass
+        else:
+            try:
+                result = urlparse(icon)
+                if not all([result.scheme, result.netloc]):
+                    errors.append("icon must be a react-icons name "
+                                  "(e.g. FaMicrophone) or a valid URL")
+            except Exception:
+                errors.append("icon must be a react-icons name "
+                              "(e.g. FaMicrophone) or a valid URL")
 
     # Validate images URLs if present
     if 'images' in data and data['images']:

scripts/check_all_raw_jsons.py

@@ -0,0 +1,72 @@
+#!/usr/bin/env python3
+"""Validate EVERY entry in raw_jsons/, not only the ones a pull request changed.
+
+`validate_raw_jsons.py` checks the files a pull request touches. That leaves the
+committed corpus unchecked, and it was: before this script existed all 36
+entries failed `validate_submission`, and nothing said so. Most of those
+failures were the validator's own fault (it demanded a URL in `icon`, while the
+store renders `icon` as a react-icons component name), and the rest were real
+gaps nobody could see behind them.
+
+This script closes that. It runs the same validator over the whole directory, so
+a rule that contradicts the corpus, or an entry that drifts from the rules, is
+one command away from being visible.
+
+KNOWN_FAILURES is the one exemption, and it is asserted to be accurate: an entry
+that starts passing must leave the list, and an entry that starts failing cannot
+hide in it. Exit code 1 on any unexpected failure, and also when the exemption
+list is stale.
+"""
+import json
+import sys
+from pathlib import Path
+
+sys.path.insert(0, str(Path(__file__).parent))
+from apply_submission import validate_submission  # noqa: E402
+
+RAW_JSONS = Path(__file__).parent.parent / 'raw_jsons'
+
+# skill-homeassistant.json ships no `examples`. It is a third-party skill
+# (neon_homeassistant_skill.mikejgray), so the voice phrasings have to come from
+# its maintainer rather than be invented here. Everything else validates.
+KNOWN_FAILURES = {
+    'skill-homeassistant.json': ['examples must be a list with at least 1 voice command'],
+}
+
+
+def main() -> int:
+    actual = {}
+    for path in sorted(RAW_JSONS.glob('*.json')):
+        try:
+            data = json.loads(path.read_text(encoding='utf-8'))
+        except json.JSONDecodeError as exc:
+            actual[path.name] = [f'invalid JSON: {exc}']
+            continue
+        errors = validate_submission(data)
+        if errors:
+            actual[path.name] = errors
+
+    status = 0
+    for name, errors in sorted(actual.items()):
+        if KNOWN_FAILURES.get(name) == errors:
+            continue
+        status = 1
+        if name in KNOWN_FAILURES:
+            print(f'{name}: failures changed')
+            print(f'  expected: {KNOWN_FAILURES[name]}')
+            print(f'  actual:   {errors}')
+        else:
+            print(f'{name}: {errors}')
+
+    for name in sorted(set(KNOWN_FAILURES) - set(actual)):
+        status = 1
+        print(f'{name}: now passes; remove it from KNOWN_FAILURES')
+
+    total = len(list(RAW_JSONS.glob('*.json')))
+    print(f'{total} entries checked, {len(actual)} failing, '
+          f'{len(KNOWN_FAILURES)} exempt')
+    return status
+
+
+if __name__ == '__main__':
+    sys.exit(main())