feat(store): refuse to install a plugin that needs a newer core (re-target of #429)

.github/workflows/release-version-check.yml

@@ -0,0 +1,40 @@
+name: Release version check
+
+# A release tag, the CHANGELOG, and src.__version__ must agree. They have not
+# always: v3.1.0 was tagged while src/__init__.py still said "1.0.0", which
+# silently exempted every device installed from that release from plugin
+# compatibility warnings. See docs/SPORTS_UNIFICATION.md (phase B4).
+on:
+  push:
+    tags: ["v*"]
+  release:
+    types: [published]
+  # Pre-flight: run this against the tag you are about to create.
+  workflow_dispatch:
+    inputs:
+      tag:
+        description: "Tag to check (e.g. v3.2.0)"
+        required: true
+        type: string
+
+permissions:
+  contents: read
+
+jobs:
+  version-matches-tag:
+    name: Tag matches src.__version__
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
+        with:
+          persist-credentials: false
+
+      - uses: actions/setup-python@0b93645e9fea7318ecaed2b359559ac225c90a2b # v5.3.0
+        with:
+          python-version: "3.12"
+
+      # No dependencies: the script reads src/__init__.py and CHANGELOG.md only.
+      - name: Assert the tag, CHANGELOG and src.__version__ agree
+        run: python scripts/check_release_version.py "${TAG}"
+        env:
+          TAG: ${{ inputs.tag || github.ref_name }}

.github/workflows/test.yml

@@ -76,4 +76,7 @@ jobs:
             test/test_sports_core_promotions.py \
             test/test_sports_modes_promotions.py \
             test/test_sports_capabilities.py \
-            test/test_sports_scroll.py
+            test/test_sports_scroll.py \
+            test/test_version_consistency.py \
+            test/test_plugin_compatibility_gate.py \
+            test/test_install_preserves_existing.py

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

src/plugin_system/plugin_loader.py

@@ -702,34 +702,25 @@ def _warn_if_incompatible(self, plugin_id: str, manifest: Dict[str, Any]) -> Non
         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:
-            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)
+        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
-        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,

src/plugin_system/store_manager.py

@@ -149,18 +149,27 @@ def __init__(self, plugins_dir: str = "plugins",
         # loser can end up renaming the winner's in-progress install aside
         # mid-download, stealing its own rollback safety net. Keyed by
         # plugin_id so unrelated plugins still update concurrently.
-        self._reinstall_locks: Dict[str, threading.Lock] = {}
+        # Reentrant: install_plugin takes this lock, and _reinstall_with_rollback
+        # holds it across its call to install_plugin. A plain Lock would
+        # self-deadlock on that nesting.
+        self._reinstall_locks: Dict[str, "threading.RLock"] = {}
         self._reinstall_locks_guard = threading.Lock()
 
         # Ensure plugins directory exists
         self.plugins_dir.mkdir(exist_ok=True)
 
-    def _get_reinstall_lock(self, plugin_id: str) -> threading.Lock:
-        """Lazily create (or fetch) the per-plugin reinstall lock."""
+    def _get_reinstall_lock(self, plugin_id: str):
+        """Lazily create (or fetch) the per-plugin reinstall lock.
+
+        Reentrant by necessity: `install_plugin` acquires it to protect its
+        set-aside/restore, and `_reinstall_with_rollback` holds it across its
+        own call to `install_plugin`. With a plain `Lock` that nesting
+        deadlocks the request thread.
+        """
         with self._reinstall_locks_guard:
             lock = self._reinstall_locks.get(plugin_id)
             if lock is None:
-                lock = threading.Lock()
+                lock = threading.RLock()
                 self._reinstall_locks[plugin_id] = lock
             return lock
 
@@ -1192,6 +1201,90 @@ def get_registry_info(self, plugin_id: str) -> Optional[Dict]:
         return next((p for p in plugins if p.get('id') == plugin_id), None)
 
     def install_plugin(self, plugin_id: str, branch: Optional[str] = None) -> bool:
+        """Install a plugin, keeping any existing install until the new one is
+        known good.
+
+        `_install_plugin_impl` deletes the existing directory *before*
+        downloading, so every failure after that point — a dropped connection, a
+        malformed manifest, or the compatibility gate refusing the new version —
+        left the user with no plugin at all. `_reinstall_with_rollback` gives the
+        *update* path exactly this protection; a direct install had none, and the
+        compatibility gate added a new way to reach it.
+
+        Pass-through when nothing is installed, and when called from
+        `_reinstall_with_rollback`, which has already moved the old copy aside.
+
+        The aside name embeds '.standalone-backup-' so plugin discovery
+        (`plugin_manager._scan_directory_for_plugins`) skips it even though it
+        still holds a manifest.json.
+
+        Held under the per-plugin reinstall lock for the same reason
+        `_reinstall_with_rollback` is: the web UI runs Flask with
+        threaded=True, so a double-clicked Install button gives two threads the
+        same plugin_id. Interleaved, one thread's restore would delete the
+        other's freshly installed copy. The lock is reentrant because the
+        rollback path already holds it when it calls in here.
+        """
+        with self._get_reinstall_lock(plugin_id):
+            plugin_path = self.plugins_dir / plugin_id
+            if not plugin_path.exists():
+                return self._install_plugin_impl(plugin_id, branch)
+
+            backup_path = plugin_path.with_name(
+                f"{plugin_path.name}.standalone-backup-preinstall")
+            if backup_path.exists() and not self._safe_remove_directory(backup_path):
+                # Can't stage a safety net. Better to attempt the install than
+                # to refuse outright, which is what callers got before this
+                # existed.
+                self.logger.warning(
+                    "Could not clear stale pre-install backup for %s at %s; "
+                    "installing without a rollback net", plugin_id, backup_path)
+                return self._install_plugin_impl(plugin_id, branch)
+
+            try:
+                plugin_path.rename(backup_path)
+            except OSError as e:
+                self.logger.warning(
+                    "Could not set aside existing install of %s (%s); "
+                    "installing without a rollback net", plugin_id, e)
+                return self._install_plugin_impl(plugin_id, branch)
+
+            try:
+                installed = self._install_plugin_impl(plugin_id, branch)
+            except Exception:
+                self._restore_preinstall_backup(plugin_id, plugin_path, backup_path)
+                raise
+
+            if installed:
+                if not self._safe_remove_directory(backup_path):
+                    self.logger.warning(
+                        "Install of %s succeeded but the previous copy at %s "
+                        "could not be removed; it will be cleared on the next "
+                        "install", plugin_id, backup_path)
+                return True
+
+            self._restore_preinstall_backup(plugin_id, plugin_path, backup_path)
+            return False
+
+    def _restore_preinstall_backup(
+        self, plugin_id: str, plugin_path: Path, backup_path: Path
+    ) -> None:
+        """Put the previous install back after a failed (re)install."""
+        self.logger.error(
+            "Install of %s failed; restoring the previous version", plugin_id)
+        try:
+            if plugin_path.exists():
+                # Partial download debris from the failed install.
+                self._safe_remove_directory(plugin_path)
+            backup_path.rename(plugin_path)
+            self.logger.info("Restored previous install of %s", plugin_id)
+        except OSError as e:
+            self.logger.error(
+                "CRITICAL: could not restore %s from %s: %s. The previous "
+                "install is preserved there — rename it back manually.",
+                plugin_id, backup_path, e)
+
+    def _install_plugin_impl(self, plugin_id: str, branch: Optional[str] = None) -> bool:
         """
         Install a plugin from the official registry. Always installs the latest commit
         from the repository's default branch (or specified branch).
@@ -1333,6 +1426,26 @@ def install_plugin(self, plugin_id: str, branch: Optional[str] = None) -> bool:
                     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