diff --git a/.github/workflows/validate_raw_jsons.yml b/.github/workflows/validate_raw_jsons.yml index 9e65fc5..d45059a 100644 --- a/.github/workflows/validate_raw_jsons.yml +++ b/.github/workflows/validate_raw_jsons.yml @@ -2,7 +2,8 @@ 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: @@ -10,6 +11,7 @@ on: - '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 diff --git a/raw_jsons/skill-spelling.json b/raw_jsons/ovos-skill-spelling-openvoiceos.json similarity index 80% rename from raw_jsons/skill-spelling.json rename to raw_jsons/ovos-skill-spelling-openvoiceos.json index 893e563..282c193 100644 --- a/raw_jsons/skill-spelling.json +++ b/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": [] -} \ No newline at end of file +} diff --git a/raw_jsons/skill-easter-eggs.json b/raw_jsons/skill-easter-eggs-jarbasai.json similarity index 92% rename from raw_jsons/skill-easter-eggs.json rename to raw_jsons/skill-easter-eggs-jarbasai.json index 8f1d047..63247e8 100644 --- a/raw_jsons/skill-easter-eggs.json +++ b/raw_jsons/skill-easter-eggs-jarbasai.json @@ -27,5 +27,9 @@ "What would GLaDOS say?", "What would Bender say?" ], - "tags": [] -} \ No newline at end of file + "tags": [ + "easter eggs", + "fun", + "references" + ] +} diff --git a/raw_jsons/skill-audio-recording.json b/raw_jsons/skill-ovos-audio-recording-openvoiceos.json similarity index 87% rename from raw_jsons/skill-audio-recording.json rename to raw_jsons/skill-ovos-audio-recording-openvoiceos.json index a37ef92..b53856d 100644 --- a/raw_jsons/skill-audio-recording.json +++ b/raw_jsons/skill-ovos-audio-recording-openvoiceos.json @@ -19,5 +19,10 @@ "start recording", "new recording named {file_name}" ], - "tags": [] -} \ No newline at end of file + "tags": [ + "recording", + "audio", + "voice memo", + "dictation" + ] +} diff --git a/raw_jsons/skill-wallpapers.json b/raw_jsons/skill-ovos-wallpapers-openvoiceos.json similarity index 92% rename from raw_jsons/skill-wallpapers.json rename to raw_jsons/skill-ovos-wallpapers-openvoiceos.json index 368cfe4..9f539cf 100644 --- a/raw_jsons/skill-wallpapers.json +++ b/raw_jsons/skill-ovos-wallpapers-openvoiceos.json @@ -28,5 +28,10 @@ "show me a picture with dogs", "make that my wallpaper" ], - "tags": [] -} \ No newline at end of file + "tags": [ + "wallpaper", + "background", + "pictures", + "reddit" + ] +} diff --git a/scripts/apply_submission.py b/scripts/apply_submission.py index 6ea446c..15a099a 100644 --- a/scripts/apply_submission.py +++ b/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']: diff --git a/scripts/check_all_raw_jsons.py b/scripts/check_all_raw_jsons.py new file mode 100755 index 0000000..1382349 --- /dev/null +++ b/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())