From 9d3648a2bf010336c7edd888df556aec33ed591b Mon Sep 17 00:00:00 2001 From: JarbasAi Date: Fri, 2 Oct 2026 07:50:32 +0100 Subject: [PATCH] fix: make the store's manifest validator agree with the store it validates Every one of the 36 committed entries failed validate_submission, and nothing reported it: the workflow only checks the entries a pull request touches, and validate_raw_jsons.py says so in its own docstring ("most of them do not pass it"). Most of those failures were the validator's, not the data's. icon was required to be a URL. 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 every committed entry holds "FaMicrophone", "MdWallpaper" and the like. The rule rejected the convention the store itself uses, and would have rejected any correct new submission. A react-icons name is now accepted, and a URL still is, because entries predating the marketplace migration used one. The skill_id pattern rejected underscores, but neon_homeassistant_skill.mikejgray is a real committed entry and a skill id is often derived from a Python package name. Widened rather than renaming a third party's skill id to fit a pattern. That left four real gaps the icon error had been hiding: skill-spelling no examples added four spelling phrasings skill-wallpapers empty tags wallpaper, background, pictures, reddit skill-audio-recording empty tags recording, audio, voice memo, dictation skill-easter-eggs empty tags easter eggs, fun, references 36 of 36 failing before, 1 of 36 after. The one left is skill-homeassistant.json, which ships no examples: it is a third-party skill, so the voice phrasings belong to its maintainer rather than being invented here. scripts/check_all_raw_jsons.py runs the same validator over the WHOLE directory, so a rule that contradicts the corpus is one command away from being visible instead of hiding behind a per-PR diff. Its KNOWN_FAILURES holds that single entry and is asserted accurate in both directions: an entry that starts failing cannot hide in it, and an entry that starts passing must leave it. Four entries predate the per-entry file-name rule, which requires a file's name to equal its skill_id lowercased with dots replaced by dashes, and failed CI on that rule rather than on their content. Renamed, content untouched: skill-audio-recording.json -> skill-ovos-audio-recording-openvoiceos.json skill-easter-eggs.json -> skill-easter-eggs-jarbasai.json skill-spelling.json -> ovos-skill-spelling-openvoiceos.json skill-wallpapers.json -> skill-ovos-wallpapers-openvoiceos.json The workflow now runs the whole-corpus check after the per-entry check. Co-Authored-By: Claude Opus 5 --- .github/workflows/validate_raw_jsons.yml | 7 +- ...n => ovos-skill-spelling-openvoiceos.json} | 8 ++- ...s.json => skill-easter-eggs-jarbasai.json} | 8 ++- ...ill-ovos-audio-recording-openvoiceos.json} | 9 ++- ...=> skill-ovos-wallpapers-openvoiceos.json} | 9 ++- scripts/apply_submission.py | 36 +++++++--- scripts/check_all_raw_jsons.py | 72 +++++++++++++++++++ 7 files changed, 133 insertions(+), 16 deletions(-) rename raw_jsons/{skill-spelling.json => ovos-skill-spelling-openvoiceos.json} (80%) rename raw_jsons/{skill-easter-eggs.json => skill-easter-eggs-jarbasai.json} (92%) rename raw_jsons/{skill-audio-recording.json => skill-ovos-audio-recording-openvoiceos.json} (87%) rename raw_jsons/{skill-wallpapers.json => skill-ovos-wallpapers-openvoiceos.json} (92%) create mode 100755 scripts/check_all_raw_jsons.py 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())