rick/LEDMatrix
1
0
Fork 0
mirror of https://github.com/ChuckBuilds/LEDMatrix.git synced 2026-08-04 10:18:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
* 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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

* fix(store): a failed install must not destroy the plugin it replaced

Found while validating the compatibility gate. `_install_plugin_impl` deletes
the existing plugin directory *before* downloading, so any failure after that
point leaves the user with nothing. `_reinstall_with_rollback` protects the
update path exactly this way; a direct `install_plugin` had no equivalent.

The gate made this reachable in a new way: a plugin whose declared floor
exceeds the running core is now refused *after* the old copy is already gone.
Floors are hand-written and can be over-declared, so the refusal could remove
a plugin that had been working fine on that core.

install_plugin is now a thin wrapper that renames any existing install aside,
delegates to _install_plugin_impl, and restores it on failure -- including
when the implementation raises, which is re-raised after the restore. It is a
pass-through when nothing is installed and when called from
_reinstall_with_rollback, which has already moved the old copy aside; a test
pins that so the two mechanisms cannot start nesting.

The aside name embeds '.standalone-backup-' because
plugin_manager._scan_directory_for_plugins keys on exactly that substring to
skip backups. A different name would have made the backup discoverable as a
duplicate plugin; a test pins that too.

789 core unit tests pass, including 7 new ones.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

* fix(store): serialize concurrent installs, and make the lock reentrant

Second bug found while validating the previous commit on hardware.

install_plugin's new set-aside/restore had no lock. The web UI runs Flask
threaded, so a double-clicked Install button gives two threads the same
plugin_id; interleaved, one thread's restore deletes the other's freshly
installed copy. _reinstall_with_rollback already guards exactly this with a
per-plugin lock, and install_plugin needs the same one.

Taking that lock naively deadlocks. _reinstall_with_rollback holds it across
its call to install_plugin, and threading.Lock is not reentrant -- so the
request thread hangs forever on the standard monorepo update path
(update_plugin -> _reinstall_with_rollback -> install_plugin), which is to say
on every plugin update. Verified by reverting to a plain Lock: the regression
test times out after 10s instead of passing.

The per-plugin locks are now RLocks, and install_plugin holds one for its
whole set-aside/install/restore sequence.

Verified on devpi (Pi, Python 3.13.5, real registry and network):
- update_plugin on an up-to-date plugin: True in 5.4s
- update_plugin forced through the full reinstall-with-rollback path:
  True in 13.1s, correct version restored, old copy replaced, no backup
  directories left behind
- install -> reinstall-over-existing -> failed-reinstall-restores: all pass
  against real downloads
- 22 plugins load, no tracebacks, web API and UI 200, steady-state journal
  50 lines/min

791 core unit tests pass, including 2 new concurrency tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-08-03 15:21:57 -04:00
committed by GitHub
co-authored by Claude Opus 5
parent e26ed29385
commit c0eb160a4f
5 changed files with 690 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/plugin_system/compatibility.py
+102
View File
@@ -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
+17 -26
View File
@@ -702,34 +702,25 @@ class PluginLoader:
newer than the running core. Advisory only — never raises — so a newer than the running core. Advisory only — never raises — so a
plugin that guards optional features with try/except keeps working. plugin that guards optional features with try/except keeps working.
""" """
declared = ( from src import __version__ as core_version
manifest.get('min_ledmatrix_version') from src.plugin_system import compatibility
or manifest.get('requires', {}).get('min_ledmatrix_version')
) compatible, _reason = compatibility.check(manifest, core_version)
if not declared: if compatible:
versions = manifest.get('versions') or [] # Distinguish "fine" from "couldn't tell" for anyone reading logs:
if versions and isinstance(versions[0], dict): # a core below the trustworthy floor is skipped, not cleared.
declared = (versions[0].get('ledmatrix_min_version') current = compatibility.parse_semver(core_version)
or versions[0].get('ledmatrix_min')) if current is None or current < compatibility.TRUSTWORTHY_FLOOR:
needed = self._parse_semver(declared) self.logger.debug(
if needed is None: "Skipping version compatibility check for %s: core __version__ "
"(%s) is below the ecosystem floor", plugin_id, core_version)
return return
from src import __version__ as core_version declared = compatibility.declared_min_version(manifest)
current = self._parse_semver(core_version) self.logger.warning(
# Anti-spam guard: if the core's own version number is stale (below "Plugin %s declares min LEDMatrix version %s but this core is %s — "
# the ecosystem floor every shipped plugin declares), comparing would "features it relies on may be missing; update the core or expect "
# warn on nearly everything — skip with a debug note instead. "degraded fallbacks", plugin_id, declared, core_version)
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)
def load_plugin( def load_plugin(
self, self,
src/plugin_system/store_manager.py
+117 -4
View File
@@ -149,18 +149,27 @@ class PluginStoreManager:
# loser can end up renaming the winner's in-progress install aside # loser can end up renaming the winner's in-progress install aside
# mid-download, stealing its own rollback safety net. Keyed by # mid-download, stealing its own rollback safety net. Keyed by
# plugin_id so unrelated plugins still update concurrently. # 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() self._reinstall_locks_guard = threading.Lock()
# Ensure plugins directory exists # Ensure plugins directory exists
self.plugins_dir.mkdir(exist_ok=True) self.plugins_dir.mkdir(exist_ok=True)
def _get_reinstall_lock(self, plugin_id: str) -> threading.Lock: def _get_reinstall_lock(self, plugin_id: str):
"""Lazily create (or fetch) the per-plugin reinstall lock.""" """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: with self._reinstall_locks_guard:
lock = self._reinstall_locks.get(plugin_id) lock = self._reinstall_locks.get(plugin_id)
if lock is None: if lock is None:
lock = threading.Lock() lock = threading.RLock()
self._reinstall_locks[plugin_id] = lock self._reinstall_locks[plugin_id] = lock
return lock return lock
@@ -1192,6 +1201,90 @@ class PluginStoreManager:
return next((p for p in plugins if p.get('id') == plugin_id), None) 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: 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 Install a plugin from the official registry. Always installs the latest commit
from the repository's default branch (or specified branch). from the repository's default branch (or specified branch).
@@ -1333,6 +1426,26 @@ class PluginStoreManager:
self._safe_remove_directory(plugin_path) self._safe_remove_directory(plugin_path)
return False 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: if 'entry_point' not in manifest:
manifest['entry_point'] = 'manager.py' manifest['entry_point'] = 'manager.py'
manifest_modified = True manifest_modified = True
test/test_install_preserves_existing.py
+222
View File
@@ -0,0 +1,222 @@
"""A failed (re)install must not destroy the working plugin it replaced.
`_install_plugin_impl` deletes the existing plugin directory *before* it
downloads anything, so any failure after that point used to leave the user with
nothing. The update path was protected — `_reinstall_with_rollback` renames the
old copy aside first — but a direct `install_plugin` was not, and the
compatibility gate added a new way to fail late: a plugin whose declared floor
exceeds the running core is now refused *after* the old copy is already gone.
Concretely, without the wrapper: a user on core 3.1.0 with a working
hockey-scoreboard clicks Install; the new manifest floors at 3.2.0; the gate
refuses; the plugin they had is deleted. Floors are hand-written and can be
over-declared, so this could remove a plugin that was working fine.
"""
import json
from pathlib import Path
from unittest.mock import MagicMock
import pytest
from src.plugin_system.store_manager import PluginStoreManager
@pytest.fixture
def store(tmp_path):
plugins_dir = tmp_path / "plugin-repos"
plugins_dir.mkdir()
mgr = PluginStoreManager(plugins_dir=str(plugins_dir))
mgr.logger = MagicMock()
return mgr, plugins_dir
def _existing_install(plugins_dir: Path, plugin_id: str, marker: str) -> Path:
path = plugins_dir / plugin_id
path.mkdir(parents=True)
(path / "manifest.json").write_text(
json.dumps({"id": plugin_id, "name": plugin_id, "class_name": "P",
"display_modes": ["a"], "version": "1.0.0"}),
encoding="utf-8")
(path / "marker.txt").write_text(marker, encoding="utf-8")
return path
class TestFailedInstallPreservesPrevious:
def test_failed_install_restores_the_old_copy(self, store, monkeypatch):
mgr, plugins_dir = store
path = _existing_install(plugins_dir, "hockey-scoreboard", "the-original")
monkeypatch.setattr(mgr, "_install_plugin_impl", lambda *a, **k: False)
assert mgr.install_plugin("hockey-scoreboard") is False
assert path.exists(), "the previous install must be restored"
assert (path / "marker.txt").read_text() == "the-original"
def test_raising_install_restores_and_reraises(self, store, monkeypatch):
mgr, plugins_dir = store
path = _existing_install(plugins_dir, "hockey-scoreboard", "the-original")
def boom(*a, **k):
raise RuntimeError("network died mid-install")
monkeypatch.setattr(mgr, "_install_plugin_impl", boom)
with pytest.raises(RuntimeError):
mgr.install_plugin("hockey-scoreboard")
assert path.exists()
assert (path / "marker.txt").read_text() == "the-original"
def test_successful_install_clears_the_backup(self, store, monkeypatch):
mgr, plugins_dir = store
_existing_install(plugins_dir, "hockey-scoreboard", "the-original")
def succeed(plugin_id, branch=None):
_existing_install(plugins_dir, plugin_id, "the-new-one")
return True
monkeypatch.setattr(mgr, "_install_plugin_impl", succeed)
assert mgr.install_plugin("hockey-scoreboard") is True
assert (plugins_dir / "hockey-scoreboard" / "marker.txt").read_text() == "the-new-one"
leftovers = [p.name for p in plugins_dir.iterdir() if "backup" in p.name]
assert not leftovers, f"backup left behind: {leftovers}"
def test_backup_name_is_invisible_to_plugin_discovery(self, store, monkeypatch):
"""A backup that discovery can see becomes a duplicate plugin entry;
the marker '.standalone-backup-' is what makes it skip."""
mgr, plugins_dir = store
_existing_install(plugins_dir, "hockey-scoreboard", "the-original")
seen = {}
def capture(plugin_id, branch=None):
seen["dirs"] = sorted(p.name for p in plugins_dir.iterdir())
return False
monkeypatch.setattr(mgr, "_install_plugin_impl", capture)
mgr.install_plugin("hockey-scoreboard")
backups = [d for d in seen["dirs"] if d != "hockey-scoreboard"]
assert backups, "expected the old copy to be set aside during install"
for name in backups:
assert ".standalone-backup-" in name, (
f"{name} would be picked up by "
"plugin_manager._scan_directory_for_plugins as a real plugin")
def test_fresh_install_is_a_pass_through(self, store, monkeypatch):
"""Nothing installed means nothing to protect; don't create stray dirs."""
mgr, plugins_dir = store
calls = []
monkeypatch.setattr(
mgr, "_install_plugin_impl",
lambda *a, **k: calls.append(a) or True)
assert mgr.install_plugin("brand-new") is True
assert calls, "the implementation must still be called"
assert list(plugins_dir.iterdir()) == []
def test_stale_backup_from_a_crash_does_not_block(self, store, monkeypatch):
mgr, plugins_dir = store
_existing_install(plugins_dir, "hockey-scoreboard", "the-original")
stale = plugins_dir / "hockey-scoreboard.standalone-backup-preinstall"
stale.mkdir()
(stale / "junk.txt").write_text("from a previous crash", encoding="utf-8")
monkeypatch.setattr(mgr, "_install_plugin_impl", lambda *a, **k: False)
assert mgr.install_plugin("hockey-scoreboard") is False
assert (plugins_dir / "hockey-scoreboard" / "marker.txt").read_text() == "the-original"
class TestUpdatePathStillWorks:
def test_reinstall_with_rollback_is_not_double_wrapped(self, store, monkeypatch):
"""_reinstall_with_rollback moves the plugin aside itself, so by the
time install_plugin runs there is nothing at the original path and the
wrapper must be a pass-through rather than staging a second backup."""
mgr, plugins_dir = store
path = _existing_install(plugins_dir, "hockey-scoreboard", "the-original")
observed = {}
def impl(plugin_id, branch=None):
observed["dirs"] = sorted(p.name for p in plugins_dir.iterdir())
return False
monkeypatch.setattr(mgr, "_install_plugin_impl", impl)
assert mgr._reinstall_with_rollback("hockey-scoreboard", path) is False
# Exactly one aside directory existed during the attempt — rollback's.
assert observed["dirs"] == ["hockey-scoreboard.standalone-backup-migrating"]
# And the user still has their plugin.
assert (plugins_dir / "hockey-scoreboard" / "marker.txt").read_text() == "the-original"
class TestConcurrency:
"""The web UI runs Flask threaded, so a double-clicked Install button puts
two threads on the same plugin_id. `_reinstall_with_rollback` already
guarded against this; the install wrapper has to as well, or one thread's
restore deletes the other's freshly installed copy."""
def test_rollback_calling_install_does_not_deadlock(self, store, monkeypatch):
"""The rollback path holds the per-plugin lock across its call to
install_plugin. A non-reentrant lock would hang the request thread
forever — this test would time out rather than fail."""
import threading
mgr, plugins_dir = store
path = _existing_install(plugins_dir, "hockey-scoreboard", "the-original")
monkeypatch.setattr(
mgr, "_install_plugin_impl",
lambda pid, branch=None: bool(_existing_install(plugins_dir, pid, "new")))
done = threading.Event()
result = {}
def run():
result["ok"] = mgr._reinstall_with_rollback("hockey-scoreboard", path)
done.set()
t = threading.Thread(target=run, daemon=True)
t.start()
assert done.wait(timeout=10), (
"install_plugin deadlocked when called from _reinstall_with_rollback "
"— the per-plugin lock must be reentrant"
)
assert result["ok"] is True
def test_concurrent_installs_serialize(self, store, monkeypatch):
"""Two threads installing the same plugin must not interleave their
set-aside/restore, and the survivor must be a complete install."""
import threading
mgr, plugins_dir = store
_existing_install(plugins_dir, "hockey-scoreboard", "the-original")
in_flight = []
overlap = []
def slow_impl(plugin_id, branch=None):
in_flight.append(1)
if len(in_flight) > 1:
overlap.append(1)
threading.Event().wait(0.05)
_existing_install(plugins_dir, plugin_id, "installed")
in_flight.pop()
return True
monkeypatch.setattr(mgr, "_install_plugin_impl", slow_impl)
threads = [threading.Thread(target=mgr.install_plugin,
args=("hockey-scoreboard",), daemon=True)
for _ in range(2)]
for t in threads:
t.start()
for t in threads:
t.join(timeout=10)
assert not t.is_alive(), "concurrent install hung"
assert not overlap, "two installs of the same plugin ran concurrently"
assert (plugins_dir / "hockey-scoreboard" / "marker.txt").exists()
leftovers = [p.name for p in plugins_dir.iterdir() if "backup" in p.name]
assert not leftovers, f"backup left behind: {leftovers}"
test/test_plugin_compatibility_gate.py
+232
View File
@@ -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")