From 3d57609f25f773a9dbfbabcbd4303c18f962ce63 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 18:10:37 -0400 Subject: [PATCH 1/3] ci: require the newest version entry to use ledmatrix_min_version Phase B4's manifest migration, scoped to what it is actually worth. 39 of 42 plugins still spell the floor `ledmatrix_min` in versions[0]. The tempting move is a sweep, but it would change no behaviour and cost 39 version bumps -- 39 store updates pushed to every user for a field rename: - compatibility.declared_min_version reads versions[0] and accepts either spelling, so the install gate already behaves identically for both. - store_manager's own deprecation check is warnings-only, and reachable only from the sideload path (install_from_url). So instead of rewriting 42 manifests, CI now 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, at zero cost to users. The nine sports plugins get it free during B5 adoption, since they add a fresh versions[0] entry flooring at 3.2.0 anyway. Historical entries (229 of them) are deliberately left alone: nothing reads them, and rewriting shipped release records to satisfy a linter is worse than the inconsistency. The check also requires a non-empty `compatible_versions` -- the core's schema marks it required, and it is the only field that can express an upper bound, which the gate now evaluates (LEDMatrix#433). scripts/check_manifest_version_fields.py runs per changed plugin in CI (failing) and has an `--all` audit mode (reporting, non-failing) for tracking what is left. Verified: exit 1 on an unmigrated changed plugin, 0 on a migrated one, 0 for `--all` and for no arguments. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 --- .github/workflows/test-plugins.yml | 17 ++++ scripts/check_manifest_version_fields.py | 120 +++++++++++++++++++++++ 2 files changed, 137 insertions(+) create mode 100644 scripts/check_manifest_version_fields.py diff --git a/.github/workflows/test-plugins.yml b/.github/workflows/test-plugins.yml index d9e4e8d3..ea7515c0 100644 --- a/.github/workflows/test-plugins.yml +++ b/.github/workflows/test-plugins.yml @@ -97,6 +97,23 @@ 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 + - name: Validate manifests against schema if: steps.changed.outputs.ids != '' env: diff --git a/scripts/check_manifest_version_fields.py b/scripts/check_manifest_version_fields.py new file mode 100644 index 00000000..9141bfd9 --- /dev/null +++ b/scripts/check_manifest_version_fields.py @@ -0,0 +1,120 @@ +#!/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 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] = [] + + if OLD in head and NEW not in head: + 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." + ) + + # 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()) From 783815124984e3765edca54986ac82bd31e397cf Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 13:52:40 -0400 Subject: [PATCH 2/3] =?UTF-8?q?ci:=20address=20review=20=E2=80=94=20reject?= =?UTF-8?q?=20a=20missing=20floor,=20and=20migrate=20the=20two=20entries?= =?UTF-8?q?=20I=20wrote?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both CodeRabbit findings on #244, verified against the real manifests. 1. baseball 1.21.1 and football 2.10.1 use the deprecated `ledmatrix_min` in versions[0], so they would fail this very gate on their next change. I wrote both entries earlier in this same session and matched the neighbouring (deprecated) spelling rather than the new one. Migrated. 2. An entry with NEITHER spelling passed silently. That is a real gap: 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 failure the B6 sunset turns fatal. Now rejected. The suggested fix for (2) -- `if NEW not in head: fail` -- would have broken four manifests that are already correct. flights, leaderboard, music and stocks declare the floor as a TOP-LEVEL `min_ledmatrix_version`, which `declared_min_version` checks BEFORE the versions[] array. Requiring the key in versions[0] unconditionally would have reported them as missing a floor they do declare, and pushed them into declaring it twice. So the missing-floor rule resolves the floor the way the core does -- top-level, then requires{}, then versions[0] -- and only fires when none of them has one. The deprecated-spelling rule is unchanged and still applies to those four, which is correct: their versions[0] key really is deprecated. Verified across all five shapes: versions[0] ledmatrix_min_version -> clean versions[0] ledmatrix_min (deprecated) -> deprecated-spelling no floor in versions[0], top-level has it -> clean no floor in versions[0], requires{} has it -> clean no floor anywhere -> missing-floor And across the real fleet: 0 of 42 plugins fail the new missing-floor rule, so it adds no breakage today. The audit is down to 37 still to migrate, from 39. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 --- plugins/baseball-scoreboard/manifest.json | 2 +- plugins/football-scoreboard/manifest.json | 2 +- scripts/check_manifest_version_fields.py | 27 +++++++++++++++++++++++ 3 files changed, 29 insertions(+), 2 deletions(-) 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 index 9141bfd9..1222a631 100644 --- a/scripts/check_manifest_version_fields.py +++ b/scripts/check_manifest_version_fields.py @@ -39,6 +39,23 @@ 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" @@ -64,6 +81,16 @@ def check_plugin(plugin_id: str) -> list[str]: 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 NEW not in head 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 From 570220ff85a2af41712948a9e1f22b5069e9efb8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 14:08:02 -0400 Subject: [PATCH 3/3] ci: check the floor's value, not just that the key is there Valid finding. The gate tested key presence, so a manifest carrying `"ledmatrix_min_version": ""` or `null` passed while the core resolved no floor at all -- confirmed against the merged compatibility module, which resolves both to None. That is precisely the manifest this gate exists to catch: one that looks migrated and leaves the install gate nothing to enforce. It was also inconsistent within this file. _declares_floor_elsewhere already used truthiness; only the versions[0] check did not. Both now use values, matching how the core resolves the floor: `head.get(NEW) or head.get(OLD)`. That `or` also decides the classification. An empty NEW alongside a real OLD is what the core falls through to, so it is a spelling problem rather than a missing floor, and is reported as such. Adds scripts/test_check_manifest_version_fields.py -- 12 cases pinning both things that are easy to get wrong here: values rather than key presence (including the empty-string and null cases this finding names, with and without a floor above versions[]), and that a floor declared at the top level or in requires{} is legitimate, since the core checks there first and four published plugins rely on it. Verified the suite bites: reverted to the presence check and 5 of the 12 cases fail, exit 1; restored, exit 0. Wired into the same CI step as the gate, whose failure mode is silent enough to deserve it. Real fleet unchanged: 0 of 42 plugins fail the missing-floor rule, audit still reports 37 to migrate. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 --- .github/workflows/test-plugins.yml | 4 + scripts/check_manifest_version_fields.py | 13 +- scripts/test_check_manifest_version_fields.py | 124 ++++++++++++++++++ 3 files changed, 139 insertions(+), 2 deletions(-) create mode 100644 scripts/test_check_manifest_version_fields.py diff --git a/.github/workflows/test-plugins.yml b/.github/workflows/test-plugins.yml index ea7515c0..4c7cbc9e 100644 --- a/.github/workflows/test-plugins.yml +++ b/.github/workflows/test-plugins.yml @@ -113,6 +113,10 @@ jobs: 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 != '' diff --git a/scripts/check_manifest_version_fields.py b/scripts/check_manifest_version_fields.py index 1222a631..bb758cb8 100644 --- a/scripts/check_manifest_version_fields.py +++ b/scripts/check_manifest_version_fields.py @@ -75,13 +75,22 @@ def check_plugin(plugin_id: str) -> list[str]: version_label = head.get("version", "?") problems: list[str] = [] - if OLD in head and NEW not in head: + # 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 NEW not in head and not _declares_floor_elsewhere(manifest): + 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 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())