diff --git a/.github/workflows/test-plugins.yml b/.github/workflows/test-plugins.yml index d9e4e8d3..4c7cbc9e 100644 --- a/.github/workflows/test-plugins.yml +++ b/.github/workflows/test-plugins.yml @@ -97,6 +97,27 @@ jobs: done exit $fail + # The floor the store and loader actually read is versions[0]. Requiring + # the new spelling only on plugins that are already being bumped lets the + # migration finish organically, instead of rewriting 42 manifests and + # pushing 42 store updates for a field rename with no behaviour change. + - name: Check manifest version fields on changed plugins + if: steps.changed.outputs.ids != '' && github.event.inputs.all != 'true' + working-directory: plugins-repo + env: + IDS: ${{ steps.changed.outputs.ids }} + run: | + for pid in $IDS; do + case "$pid" in + '' | *[!a-z0-9._-]*) echo "::error::invalid plugin id '$pid'"; exit 1 ;; + esac + done + python scripts/check_manifest_version_fields.py $IDS + # The gate's own regression suite -- its failure mode is silent, + # so a manifest that looks migrated but resolves to no floor must + # not slip through. + python scripts/test_check_manifest_version_fields.py + - name: Validate manifests against schema if: steps.changed.outputs.ids != '' env: diff --git a/plugins/baseball-scoreboard/manifest.json b/plugins/baseball-scoreboard/manifest.json index 960359d8..bcbb72e0 100644 --- a/plugins/baseball-scoreboard/manifest.json +++ b/plugins/baseball-scoreboard/manifest.json @@ -33,7 +33,7 @@ { "released": "2026-08-03", "version": "1.21.1", - "ledmatrix_min": "2.0.0" + "ledmatrix_min_version": "2.0.0" }, { "released": "2026-08-02", diff --git a/plugins/football-scoreboard/manifest.json b/plugins/football-scoreboard/manifest.json index 0c1ed8ca..7cbea4f9 100644 --- a/plugins/football-scoreboard/manifest.json +++ b/plugins/football-scoreboard/manifest.json @@ -27,7 +27,7 @@ { "released": "2026-08-03", "version": "2.10.1", - "ledmatrix_min": "2.0.0" + "ledmatrix_min_version": "2.0.0" }, { "released": "2026-08-02", diff --git a/scripts/check_manifest_version_fields.py b/scripts/check_manifest_version_fields.py new file mode 100644 index 00000000..bb758cb8 --- /dev/null +++ b/scripts/check_manifest_version_fields.py @@ -0,0 +1,156 @@ +#!/usr/bin/env python3 +"""Require the *newest* versions[] entry to use `ledmatrix_min_version`. + +Run over changed plugins in CI, or over everything with `--all`: + + python scripts/check_manifest_version_fields.py baseball-scoreboard news + python scripts/check_manifest_version_fields.py --all # audit, non-gating + +## Why only the newest entry + +`PluginStoreManager` and `PluginLoader` resolve a plugin's floor through +`src/plugin_system/compatibility.py:declared_min_version`, which reads +`versions[0]` and accepts either spelling. So the deprecated `ledmatrix_min` +costs nothing functionally, and the store's own deprecation check is +warnings-only and reachable only from the sideload path. + +Rewriting all 42 manifests to the new spelling would therefore change no +behaviour while forcing 42 version bumps — 42 store updates pushed to every +user for a field rename. Instead this gate asks each plugin to migrate the one +entry that is actually read, at a moment when it is already being bumped for +other reasons. The migration completes as plugins release, and cannot slide +backwards. + +Historical entries are left alone: nothing reads them, and rewriting shipped +release records to satisfy a linter is worse than the inconsistency. +""" + +from __future__ import annotations + +import argparse +import json +import sys +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +PLUGINS = REPO_ROOT / "plugins" + +NEW = "ledmatrix_min_version" +OLD = "ledmatrix_min" + + +def _declares_floor_elsewhere(manifest: dict) -> bool: + """Does the manifest declare a floor above the `versions[]` array? + + The core's `compatibility.declared_min_version` checks, in order: + top-level `min_ledmatrix_version`, then `requires.min_ledmatrix_version`, + and only then `versions[0]`. Four published plugins — flights, leaderboard, + music and stocks — use the top-level form, so demanding the key in + `versions[0]` unconditionally would fail manifests that are already + correct, and push them into declaring the floor twice. + """ + if manifest.get("min_ledmatrix_version"): + return True + requires = manifest.get("requires") + return isinstance(requires, dict) and bool( + requires.get("min_ledmatrix_version")) + + +def check_plugin(plugin_id: str) -> list[str]: + """Problems with this plugin's newest version entry (empty when fine).""" + path = PLUGINS / plugin_id / "manifest.json" + if not path.exists(): + return [] # not a monorepo plugin; nothing to say + + try: + manifest = json.loads(path.read_text(encoding="utf-8")) + except (OSError, ValueError) as e: + return [f"{plugin_id}: manifest.json could not be read ({e})"] + + versions = [v for v in (manifest.get("versions") or []) if isinstance(v, dict)] + if not versions: + return [] + + head = versions[0] + version_label = head.get("version", "?") + problems: list[str] = [] + + # Values, not key presence. The core resolves the floor with + # `head.get(NEW) or head.get(OLD)`, so an empty string or null under either + # key is no floor at all -- and a gate that accepted the key while the core + # saw nothing would pass exactly the manifests it exists to catch. + # `_declares_floor_elsewhere` already worked this way; this brings the + # versions[0] check into line with it. + new_value = head.get(NEW) + old_value = head.get(OLD) + + if not new_value and old_value: + problems.append( + f"{plugin_id}: versions[0] ({version_label}) uses the deprecated " + f"'{OLD}'. Rename it to '{NEW}' — this is the entry the store and " + f"loader actually read. Older entries can stay as they are." + ) + elif not new_value and not _declares_floor_elsewhere(manifest): + # Neither spelling in versions[0], and nothing above it either. The + # core's declared_min_version() then resolves to None, so the install + # gate has no floor to enforce and the plugin can reach a core that + # cannot run it — the exact failure the B6 sunset turns fatal. + problems.append( + f"{plugin_id}: versions[0] ({version_label}) declares no minimum " + f"core version, and neither does the manifest above it. Add " + f"'{NEW}' so the install gate has a floor to enforce." + ) + + # compatible_versions is required by the core's manifest schema and is the + # only field that can express an upper bound; a missing one means the gate + # has nothing authoritative to evaluate. + compatible = manifest.get("compatible_versions") + if not isinstance(compatible, list) or not compatible: + problems.append( + f"{plugin_id}: 'compatible_versions' is missing or empty. The core " + f"manifest schema requires it, and it is the field the install " + f"gate evaluates for upper bounds." + ) + + return problems + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("plugin_ids", nargs="*", help="Plugin ids to check") + parser.add_argument("--all", action="store_true", + help="Audit every plugin; reports without failing") + args = parser.parse_args() + + if args.all: + ids = sorted(p.name for p in PLUGINS.iterdir() + if (p / "manifest.json").exists()) + else: + ids = args.plugin_ids + if not ids: + print("No plugins to check.") + return 0 + + problems = [p for pid in ids for p in check_plugin(pid)] + + if not problems: + print(f"OK: {len(ids)} plugin(s) checked, newest version entries are current.") + return 0 + + for problem in problems: + print(f" - {problem}", file=sys.stderr) + + if args.all: + print(f"\n{len(problems)} plugin(s) still to migrate — audit only, " + f"not failing. They migrate at their next version bump.", + file=sys.stderr) + return 0 + + print(f"\n{len(problems)} problem(s). These plugins are being changed " + f"anyway, so the fix is a one-line rename in the entry you just " + f"added.", file=sys.stderr) + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/test_check_manifest_version_fields.py b/scripts/test_check_manifest_version_fields.py new file mode 100644 index 00000000..e6374a37 --- /dev/null +++ b/scripts/test_check_manifest_version_fields.py @@ -0,0 +1,124 @@ +#!/usr/bin/env python3 +"""Regression tests for the manifest version-field gate. + +The gate exists to guarantee the install gate has a floor to enforce. Its +failure mode is silent: a manifest that *looks* migrated but resolves to no +floor passes here, then reaches a core that cannot run it — which the B6 +sunset turns from a degraded plugin into one that will not load at all. + +So these pin the two things that are easy to get wrong: + +- **Values, not key presence.** The core resolves the floor with + ``head.get(NEW) or head.get(OLD)``. An empty string or ``null`` under either + key is no floor, and a gate checking only for the key would accept exactly + the manifests it exists to catch. +- **The floor may legitimately live above ``versions[]``.** Four published + plugins declare a top-level ``min_ledmatrix_version``, which the core checks + *first*. Demanding the key inside ``versions[0]`` would fail manifests that + are already correct. + +Exit codes follow the convention in `run_plugin_tests.py`: 0 pass, 1 fail. +""" + +import json +import shutil +import sys +import tempfile +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +import check_manifest_version_fields as gate # noqa: E402 + +BASE = {"id": "p", "compatible_versions": [">=2.0.0"]} + +CLEAN, DEPRECATED, MISSING = "clean", "deprecated", "missing-floor" + + +def classify(manifest): + """Run the gate over a synthetic manifest and bucket the outcome.""" + root = Path(tempfile.mkdtemp()) + (root / "p").mkdir() + (root / "p" / "manifest.json").write_text(json.dumps(manifest)) + original = gate.PLUGINS + gate.PLUGINS = root + try: + problems = [p for p in gate.check_plugin("p") + if "compatible_versions" not in p] + finally: + gate.PLUGINS = original + shutil.rmtree(root, ignore_errors=True) + + if not problems: + return CLEAN + if "deprecated" in problems[0]: + return DEPRECATED + if "declares no minimum" in problems[0]: + return MISSING + return f"unexpected: {problems[0]}" + + +def entry(**kwargs): + return {**BASE, "versions": [{"version": "1.0.0", **kwargs}]} + + +CASES = [ + # (label, manifest, expected) + ("new spelling with a value", + entry(ledmatrix_min_version="2.0.0"), CLEAN), + ("deprecated spelling with a value", + entry(ledmatrix_min="2.0.0"), DEPRECATED), + + # The regression this file was added for: a present-but-empty key. + ('new spelling, empty string, no floor elsewhere', + entry(ledmatrix_min_version=""), MISSING), + ("new spelling, null, no floor elsewhere", + entry(ledmatrix_min_version=None), MISSING), + ('deprecated spelling, empty string, no floor elsewhere', + entry(ledmatrix_min=""), MISSING), + ("deprecated spelling, null, no floor elsewhere", + entry(ledmatrix_min=None), MISSING), + + # An empty NEW alongside a real OLD is what the core's `or` falls through + # to, so it is a spelling problem rather than a missing floor. + ("empty new spelling but a valid deprecated one", + entry(ledmatrix_min_version="", ledmatrix_min="2.0.0"), DEPRECATED), + + # The floor may live above versions[]; the core checks there first. + ("no floor in versions[0], top-level declares it", + {**BASE, "min_ledmatrix_version": "2.0.0", "versions": [{"version": "1.0.0"}]}, + CLEAN), + ("no floor in versions[0], requires{} declares it", + {**BASE, "requires": {"min_ledmatrix_version": "2.0.0"}, + "versions": [{"version": "1.0.0"}]}, CLEAN), + ("empty key in versions[0], top-level declares it", + {**BASE, "min_ledmatrix_version": "2.0.0", + "versions": [{"version": "1.0.0", "ledmatrix_min_version": ""}]}, CLEAN), + ("top-level present but empty, nothing else", + {**BASE, "min_ledmatrix_version": "", + "versions": [{"version": "1.0.0"}]}, MISSING), + + ("no floor anywhere", entry(), MISSING), +] + + +def main() -> int: + failures = [] + for label, manifest, expected in CASES: + got = classify(manifest) + ok = got == expected + print(f" [{'pass' if ok else 'FAIL'}] {label:52} -> {got}") + if not ok: + failures.append(f"{label}: expected {expected}, got {got}") + + print() + if failures: + print(f"{len(failures)} failure(s):", file=sys.stderr) + for f in failures: + print(f" - {f}", file=sys.stderr) + return 1 + print(f"All {len(CASES)} cases passed.") + return 0 + + +if __name__ == "__main__": + sys.exit(main())