From 749a6a902887eabbb652d8fc498b1eb9b642981e Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Sun, 2 Aug 2026 18:08:24 -0400 Subject: [PATCH] feat(store): refuse to install a plugin that needs a newer core `ledmatrix_min_version` was decoration. The loader logged an advisory warning and continued; the store never compared the core version at all, so a routine "update" delivered a plugin that could not run. That is the gap phase B6 (the sports-unification sunset) cannot be done over: deleting a plugin's bundled fallback while nothing enforces the floor hands un-updated users a scoreboard that raises ModuleNotFoundError at load and is reported only as one line in the journal. The gate lives in install_plugin, after the manifest is on disk and before dependencies are installed. That is the earliest knowable point -- the registry carries no compatibility field, so the floor is not visible until the files are down -- and it is also the chokepoint: _reinstall_with_rollback calls install_plugin, so a refused *update* restores the version the user already had, for free. Floor resolution and the comparison move to src/plugin_system/compatibility.py, shared with the loader so the two cannot drift. Both read all four spellings published manifests use, including the deprecated `ledmatrix_min`. Refusal requires evidence. An undeclared floor, an unparseable version on either side, or a core below TRUSTWORTHY_FLOOR (2.0.0) all allow the install. That last one is deliberate and load-bearing: the v3.1.0 release reports __version__ = "1.0.0" while nearly every published manifest floors at 2.0.0, so a strict gate would lock those users out of the plugin store entirely -- much worse than the problem being solved. They stay unprotected until they update the core, which is also what fixes their version string. Verified: 782 core unit tests pass, including 25 new ones and the existing loader-warning suite unchanged (the refactor is behavior-preserving). The install tests drive the real install_plugin path with the download stubbed -- the allow and refuse cases differ only in the declared floor, so the refusal is demonstrably the gate and not an earlier bail-out. Follow-ups, deliberately not in this PR: surfacing the reason in the store UI rather than only the log, and publishing the floor in plugins.json so the store can refuse before downloading. Phase B4 in docs/SPORTS_UNIFICATION.md. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 --- src/plugin_system/compatibility.py | 102 +++++++++++ src/plugin_system/plugin_loader.py | 43 ++--- src/plugin_system/store_manager.py | 20 +++ test/test_plugin_compatibility_gate.py | 232 +++++++++++++++++++++++++ 4 files changed, 371 insertions(+), 26 deletions(-) create mode 100644 src/plugin_system/compatibility.py create mode 100644 test/test_plugin_compatibility_gate.py diff --git a/src/plugin_system/compatibility.py b/src/plugin_system/compatibility.py new file mode 100644 index 00000000..dd3925e7 --- /dev/null +++ b/src/plugin_system/compatibility.py @@ -0,0 +1,102 @@ +"""One place that answers "can this plugin run on this core?". + +Two callers ask that question and they must not drift apart: + +- `PluginLoader._warn_if_incompatible` — at load time, **advisory**. A plugin + already on disk keeps loading regardless, because the guarded-import pattern + means most incompatibilities degrade rather than break. +- `PluginStoreManager.install_plugin` — at install/update time, **blocking**. + This is the point where refusing costs the user nothing (they keep the + version they already had) and allowing can cost them a plugin that fails to + load with only a log line to explain it. + +## The trustworthiness problem + +The core's own `__version__` has not always been right. `v3.1.0` was tagged +2026-05-31 while `src/__init__.py` still said `"1.0.0"`; the bump landed +2026-07-12. Devices installed from that release report `1.0.0` — below the +floor that essentially every published plugin declares. + +So a core reporting a version below `TRUSTWORTHY_FLOOR` is treated as +**unknown, not old**: it neither warns nor blocks. Blocking on it would be far +worse than the problem being solved — nearly every manifest in the ecosystem +floors at `2.0.0`, so a strict gate would stop those users installing *any* +plugin. They are unprotected until they update the core, which is also what +fixes their version string. See `docs/SPORTS_UNIFICATION.md`, phase B4. +""" + +from __future__ import annotations + +from typing import Any, Dict, Optional, Tuple + +# Below this, the core's self-reported version is not evidence of anything. +# See the module docstring. +TRUSTWORTHY_FLOOR: Tuple[int, int, int] = (2, 0, 0) + + +def parse_semver(value: Any) -> Optional[Tuple[int, int, int]]: + """Parse ``X.Y.Z`` (extra parts and suffixes ignored) into a comparable + 3-tuple, or ``None`` when unparseable. A leading ``v`` is tolerated.""" + if not isinstance(value, str): + return None + parts = value.strip().lstrip('v').split('.') + try: + nums = [int(''.join(ch for ch in p if ch.isdigit()) or 0) for p in parts[:3]] + except ValueError: + return None + while len(nums) < 3: + nums.append(0) + return tuple(nums) # type: ignore[return-value] + + +def declared_min_version(manifest: Dict[str, Any]) -> Optional[str]: + """The core version this plugin says it needs, or ``None`` if it doesn't say. + + Checked in order of specificity. `ledmatrix_min` is the deprecated spelling + of `ledmatrix_min_version` (`store_manager._validate_manifest_fields` flags + it); both are read because a large share of published manifests still carry + the old one. + """ + declared = ( + manifest.get('min_ledmatrix_version') + or (manifest.get('requires') or {}).get('min_ledmatrix_version') + ) + if declared: + return declared + + versions = manifest.get('versions') or [] + if versions and isinstance(versions[0], dict): + return (versions[0].get('ledmatrix_min_version') + or versions[0].get('ledmatrix_min')) + return None + + +def check(manifest: Dict[str, Any], core_version: str) -> Tuple[bool, Optional[str]]: + """Return ``(compatible, reason)``. + + ``compatible`` is False **only** when the plugin declares a parseable floor, + the core reports a parseable and trustworthy version, and the floor is + genuinely above it. Every uncertain case resolves to compatible: an + undeclared floor, an unparseable version on either side, or a core whose + version is below `TRUSTWORTHY_FLOOR`. Refusing on a guess would break + working installs, which is the more expensive mistake here. + + ``reason`` is user-facing text, present only when incompatible. + """ + declared = declared_min_version(manifest) + needed = parse_semver(declared) + if needed is None: + return True, None + + current = parse_semver(core_version) + if current is None or current < TRUSTWORTHY_FLOOR: + return True, None + + if needed > current: + name = manifest.get('name') or manifest.get('id') or 'This plugin' + return False, ( + f"{name} requires LEDMatrix {declared} or newer, but this system is " + f"running {core_version}. Update LEDMatrix first, then install it." + ) + + return True, None diff --git a/src/plugin_system/plugin_loader.py b/src/plugin_system/plugin_loader.py index 733bfde1..bd748ef7 100644 --- a/src/plugin_system/plugin_loader.py +++ b/src/plugin_system/plugin_loader.py @@ -702,34 +702,25 @@ class PluginLoader: newer than the running core. Advisory only — never raises — so a plugin that guards optional features with try/except keeps working. """ - declared = ( - manifest.get('min_ledmatrix_version') - or manifest.get('requires', {}).get('min_ledmatrix_version') - ) - if not declared: - versions = manifest.get('versions') or [] - if versions and isinstance(versions[0], dict): - declared = (versions[0].get('ledmatrix_min_version') - or versions[0].get('ledmatrix_min')) - needed = self._parse_semver(declared) - if needed is None: + from src import __version__ as core_version + from src.plugin_system import compatibility + + compatible, _reason = compatibility.check(manifest, core_version) + if compatible: + # Distinguish "fine" from "couldn't tell" for anyone reading logs: + # a core below the trustworthy floor is skipped, not cleared. + current = compatibility.parse_semver(core_version) + if current is None or current < compatibility.TRUSTWORTHY_FLOOR: + self.logger.debug( + "Skipping version compatibility check for %s: core __version__ " + "(%s) is below the ecosystem floor", plugin_id, core_version) return - from src import __version__ as core_version - current = self._parse_semver(core_version) - # Anti-spam guard: if the core's own version number is stale (below - # the ecosystem floor every shipped plugin declares), comparing would - # warn on nearly everything — skip with a debug note instead. - if current is None or current < (2, 0, 0): - self.logger.debug( - "Skipping version compatibility check for %s: core __version__ " - "(%s) is below the ecosystem floor", plugin_id, core_version) - return - if needed > current: - self.logger.warning( - "Plugin %s declares min LEDMatrix version %s but this core is %s — " - "features it relies on may be missing; update the core or expect " - "degraded fallbacks", plugin_id, declared, core_version) + declared = compatibility.declared_min_version(manifest) + self.logger.warning( + "Plugin %s declares min LEDMatrix version %s but this core is %s — " + "features it relies on may be missing; update the core or expect " + "degraded fallbacks", plugin_id, declared, core_version) def load_plugin( self, diff --git a/src/plugin_system/store_manager.py b/src/plugin_system/store_manager.py index bcd93875..3d5ade36 100644 --- a/src/plugin_system/store_manager.py +++ b/src/plugin_system/store_manager.py @@ -1333,6 +1333,26 @@ class PluginStoreManager: self._safe_remove_directory(plugin_path) return False + # Refuse a plugin that needs a newer core than this one. The + # registry carries no compatibility field, so the floor is only + # knowable once the files are down — checking here, before + # dependency installation, is the earliest possible point. + # + # Refusing costs the user nothing: on an update this returns + # False and _reinstall_with_rollback restores the version they + # already had. Allowing it costs them a plugin that raises + # ModuleNotFoundError at load and is reported only as one line + # in the journal. See docs/SPORTS_UNIFICATION.md (phase B4/B6). + from src import __version__ as core_version + from src.plugin_system import compatibility + + compatible, reason = compatibility.check(manifest, core_version) + if not compatible: + self.logger.error( + "Refusing to install %s: %s", plugin_id, reason) + self._safe_remove_directory(plugin_path) + return False + if 'entry_point' not in manifest: manifest['entry_point'] = 'manager.py' manifest_modified = True diff --git a/test/test_plugin_compatibility_gate.py b/test/test_plugin_compatibility_gate.py new file mode 100644 index 00000000..f0f252f1 --- /dev/null +++ b/test/test_plugin_compatibility_gate.py @@ -0,0 +1,232 @@ +"""The install/update gate, and the shared compatibility rules behind it. + +Before this existed, `ledmatrix_min_version` was decoration: the loader logged +an advisory warning and the store never looked at the core version at all, so a +routine store update happily delivered a plugin that could not run. Deleting a +plugin's bundled fallback under those conditions would have handed un-updated +users a scoreboard that fails to load with one line in the journal. + +The rules being pinned here, in priority order: + +1. Refuse only on **evidence**. Undeclared floor, unparseable version on either + side, or a core whose self-reported version is untrustworthy → allow. A + wrong refusal breaks a working install; a wrong allowance degrades to the + behavior we already had. +2. A core below `TRUSTWORTHY_FLOOR` is *unknown*, not old. The v3.1.0 release + reports `1.0.0` while nearly every manifest floors at `2.0.0`; blocking on + that number would stop those users installing anything at all. +3. The loader and the store must agree, because they read the same manifests. +""" + +import json +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from src.plugin_system import compatibility + + +# -------------------------------------------------------------------------- +# Floor resolution — every spelling published plugins actually use +# -------------------------------------------------------------------------- + +class TestDeclaredMinVersion: + def test_top_level_min_ledmatrix_version(self): + assert compatibility.declared_min_version( + {"min_ledmatrix_version": "3.2.0"}) == "3.2.0" + + def test_requires_block(self): + assert compatibility.declared_min_version( + {"requires": {"min_ledmatrix_version": "3.1.0"}}) == "3.1.0" + + def test_versions_array_new_spelling(self): + assert compatibility.declared_min_version( + {"versions": [{"ledmatrix_min_version": "3.2.0"}]}) == "3.2.0" + + def test_versions_array_deprecated_spelling(self): + """Most published manifests still say `ledmatrix_min`; ignoring it + would silently exempt them from the gate.""" + assert compatibility.declared_min_version( + {"versions": [{"ledmatrix_min": "2.0.0"}]}) == "2.0.0" + + def test_absent(self): + assert compatibility.declared_min_version({"id": "x"}) is None + + def test_requires_present_but_null(self): + assert compatibility.declared_min_version({"requires": None}) is None + + +# -------------------------------------------------------------------------- +# The decision itself +# -------------------------------------------------------------------------- + +class TestCheck: + def test_blocks_when_plugin_needs_a_newer_core(self): + ok, reason = compatibility.check( + {"name": "Hockey Scoreboard", "min_ledmatrix_version": "3.2.0"}, "3.1.0") + assert ok is False + assert "3.2.0" in reason and "3.1.0" in reason + assert "Hockey Scoreboard" in reason + + def test_allows_equal_version(self): + ok, _ = compatibility.check({"min_ledmatrix_version": "3.2.0"}, "3.2.0") + assert ok is True + + def test_allows_newer_core(self): + ok, _ = compatibility.check({"min_ledmatrix_version": "3.2.0"}, "4.0.0") + assert ok is True + + def test_allows_when_no_floor_declared(self): + ok, reason = compatibility.check({"id": "x"}, "3.2.0") + assert ok is True and reason is None + + def test_untrustworthy_core_version_allows_everything(self): + """The v3.1.0 release reports 1.0.0. Nearly every manifest floors at + 2.0.0, so blocking here would stop those users installing any plugin + at all — strictly worse than the problem being solved.""" + ok, reason = compatibility.check( + {"min_ledmatrix_version": "3.2.0"}, "1.0.0") + assert ok is True and reason is None + + def test_unparseable_core_version_allows(self): + ok, _ = compatibility.check({"min_ledmatrix_version": "3.2.0"}, "not-a-version") + assert ok is True + + def test_unparseable_floor_allows(self): + ok, _ = compatibility.check({"min_ledmatrix_version": {"nope": 1}}, "3.2.0") + assert ok is True + + def test_v_prefix_tolerated_on_both_sides(self): + ok, _ = compatibility.check({"min_ledmatrix_version": "v3.3.0"}, "v3.2.0") + assert ok is False + + @pytest.mark.parametrize("floor,core,expected_ok", [ + ("3.2.0", "3.2.1", True), + ("3.2.1", "3.2.0", False), + ("3.10.0", "3.9.0", False), # numeric compare, not lexical + ("3.9.0", "3.10.0", True), + ]) + def test_ordering(self, floor, core, expected_ok): + ok, _ = compatibility.check({"min_ledmatrix_version": floor}, core) + assert ok is expected_ok + + +# -------------------------------------------------------------------------- +# The gate in install_plugin +# -------------------------------------------------------------------------- + +def _write_plugin(plugins_dir: Path, plugin_id: str, manifest: dict) -> Path: + path = plugins_dir / plugin_id + path.mkdir(parents=True) + (path / "manifest.json").write_text(json.dumps(manifest), encoding="utf-8") + (path / "manager.py").write_text("class P: pass\n", encoding="utf-8") + return path + + +@pytest.fixture +def store(tmp_path, monkeypatch): + """A PluginStoreManager whose download step is stubbed to drop a plugin + directory in place, so the test exercises the post-download validation + path without touching the network.""" + from src.plugin_system.store_manager import PluginStoreManager + + plugins_dir = tmp_path / "plugin-repos" + plugins_dir.mkdir() + mgr = PluginStoreManager(plugins_dir=str(plugins_dir)) + mgr.logger = MagicMock() + return mgr, plugins_dir + + +class TestInstallGate: + """`install_plugin` is the chokepoint: `_reinstall_with_rollback` calls it, + so gating there covers updates too, and a refused update restores the + version the user already had.""" + + def _install_with_manifest(self, store, manifest, core_version, monkeypatch): + mgr, plugins_dir = store + plugin_id = manifest["id"] + + monkeypatch.setattr( + mgr, "get_plugin_info", + lambda *a, **k: {"repo": "https://example.invalid/r", + "plugin_path": f"plugins/{plugin_id}", + "branch": "main"}) + # Stand in for the download: put the files where install_plugin expects. + monkeypatch.setattr( + mgr, "_install_from_monorepo", + lambda *a, **k: bool(_write_plugin(plugins_dir, plugin_id, manifest))) + monkeypatch.setattr(mgr, "_install_from_monorepo_api", lambda *a, **k: False) + monkeypatch.setattr(mgr, "_install_dependencies", lambda *a, **k: True) + + import src + monkeypatch.setattr(src, "__version__", core_version) + return mgr.install_plugin(plugin_id), plugins_dir / plugin_id + + def test_refuses_and_leaves_nothing_behind(self, store, monkeypatch): + manifest = { + "id": "needs-newer", "name": "Needs Newer", "class_name": "P", + "display_modes": ["a"], "min_ledmatrix_version": "9.9.9", + } + ok, path = self._install_with_manifest(store, manifest, "3.2.0", monkeypatch) + + assert ok is False, "install must refuse a plugin that needs a newer core" + assert not path.exists(), ( + "a refused install must not leave a half-installed directory — " + "plugin discovery would pick it up and fail to load it") + + def test_allows_a_compatible_plugin(self, store, monkeypatch): + manifest = { + "id": "fine", "name": "Fine", "class_name": "P", + "display_modes": ["a"], "min_ledmatrix_version": "3.0.0", + } + ok, path = self._install_with_manifest(store, manifest, "3.2.0", monkeypatch) + + assert ok is True + assert (path / "manifest.json").exists() + + def test_untrustworthy_core_does_not_block_installs(self, store, monkeypatch): + """Regression guard for the worst possible outcome of this feature: + users on the v3.1.0 release (which reports 1.0.0) must not be locked + out of the plugin store entirely.""" + manifest = { + "id": "floored", "name": "Floored", "class_name": "P", + "display_modes": ["a"], "versions": [{"ledmatrix_min": "2.0.0"}], + } + ok, path = self._install_with_manifest(store, manifest, "1.0.0", monkeypatch) + + assert ok is True, ( + "a core below the trustworthy floor must not block installs — " + "nearly every published manifest floors at 2.0.0") + assert (path / "manifest.json").exists() + + +class TestLoaderAndStoreAgree: + """Both read the same manifests; a disagreement means one of them is + lying to the user.""" + + @pytest.mark.parametrize("manifest,core,expected", [ + ({"min_ledmatrix_version": "3.2.0"}, "3.1.0", False), + ({"versions": [{"ledmatrix_min": "2.0.0"}]}, "3.2.0", True), + ({"versions": [{"ledmatrix_min_version": "9.0.0"}]}, "3.2.0", False), + ({}, "3.2.0", True), + ]) + def test_same_verdict(self, manifest, core, expected): + from src.plugin_system.plugin_loader import PluginLoader + + store_ok, _ = compatibility.check(manifest, core) + assert store_ok is expected + + # The loader resolves the floor through the same helper, so a + # divergence in spelling handling would show up here. + loader_needed = compatibility.parse_semver( + compatibility.declared_min_version(manifest)) + current = compatibility.parse_semver(core) + loader_would_warn = ( + loader_needed is not None + and current is not None + and current >= compatibility.TRUSTWORTHY_FLOOR + and loader_needed > current + ) + assert loader_would_warn is (not expected) + assert hasattr(PluginLoader, "_warn_if_incompatible")