diff --git a/src/plugin_system/compatibility.py b/src/plugin_system/compatibility.py index dd3925e7..784a6b6a 100644 --- a/src/plugin_system/compatibility.py +++ b/src/plugin_system/compatibility.py @@ -27,6 +27,7 @@ fixes their version string. See `docs/SPORTS_UNIFICATION.md`, phase B4. from __future__ import annotations +import re from typing import Any, Dict, Optional, Tuple # Below this, the core's self-reported version is not evidence of anything. @@ -39,7 +40,20 @@ def parse_semver(value: Any) -> Optional[Tuple[int, int, int]]: 3-tuple, or ``None`` when unparseable. A leading ``v`` is tolerated.""" if not isinstance(value, str): return None - parts = value.strip().lstrip('v').split('.') + text = value.strip().lstrip('v') + # Drop the prerelease/build suffix before scraping digits. Without this the + # scrape pulls them into the numbers: "3.2.0+build42" parsed as (3, 2, 42) + # and "3.2.0-rc1" as (3, 2, 1) -- a release candidate ranking *above* its + # own release, and a build of 3.2.0 failing an exact "3.2.0" match. + # + # Prereleases compare equal to their release here rather than below it. + # Full prerelease ordering is more than any caller needs, and equal is far + # closer to right than the old behaviour. + for sep in ('+', '-'): + head, found, _tail = text.partition(sep) + if found: + text = head + parts = text.split('.') try: nums = [int(''.join(ch for ch in p if ch.isdigit()) or 0) for p in parts[:3]] except ValueError: @@ -49,6 +63,92 @@ def parse_semver(value: Any) -> Optional[Tuple[int, int, int]]: return tuple(nums) # type: ignore[return-value] +# `parse_semver` is deliberately lenient — it strips non-digits and yields +# (0, 0, 0) for a string with no numbers at all, which is fine for a floor +# (a floor of 0.0.0 never blocks anything) but wrong for a range, where the +# same leniency would turn an unreadable spec into a *refusal*. Range specs +# are therefore validated against this first, so garbage reads as "no +# evidence" rather than "incompatible". +_VERSION_TOKEN = re.compile(r"^v?\d+(\.\d+){0,2}(-[\w.-]+)?(\+[\w.-]+)?$") + + +def _parse_strict(value: str) -> Optional[Tuple[int, int, int]]: + """`parse_semver`, but ``None`` unless the string really looks like one.""" + if not isinstance(value, str) or not _VERSION_TOKEN.match(value.strip()): + return None + return parse_semver(value) + + +def _satisfies_range(core: Tuple[int, int, int], spec: str) -> Optional[bool]: + """Does ``core`` satisfy one `compatible_versions` entry? + + Returns ``None`` when the spec cannot be parsed — the caller treats that as + "no evidence" rather than as a refusal, so an unrecognised spelling never + costs a user a working install. + + Supports the forms `schema/manifest_schema.json` permits: `>=`, `<=`, `>`, + `<`, `~`, `^`, a bare exact version, and an inclusive `A - B` range. + Prerelease/build suffixes are tolerated and ignored, matching `parse_semver`. + """ + spec = spec.strip() + if not spec: + return None + + if " - " in spec: # inclusive range, e.g. "2.0.0 - 3.1.0" + low_raw, _, high_raw = spec.partition(" - ") + low, high = _parse_strict(low_raw), _parse_strict(high_raw) + if low is None or high is None: + return None + return low <= core <= high + + for op in (">=", "<=", ">", "<", "~", "^"): + if spec.startswith(op): + target = _parse_strict(spec[len(op):]) + if target is None: + return None + if op == ">=": + return core >= target + if op == "<=": + return core <= target + if op == ">": + return core > target + if op == "<": + return core < target + if op == "~": + # Patch-level changes only: >=X.Y.Z, =X.Y.Z, <(X+1).0.0 + return target <= core < (target[0] + 1, 0, 0) + + exact = _parse_strict(spec) + return None if exact is None else core == exact + + +def satisfies_compatible_versions( + manifest: Dict[str, Any], core: Tuple[int, int, int] +) -> Optional[bool]: + """Evaluate the manifest's `compatible_versions` array against ``core``. + + The array is a set of *alternatives*: satisfying any one entry means the + plugin declares itself compatible. Returns ``None`` when the field is + absent or no entry could be parsed, so callers can distinguish "declared + incompatible" from "did not say". + + This is the field `schema/manifest_schema.json` marks **required**, and it + is the only one that can express an upper bound — `ledmatrix_min_version` + is a floor and cannot say "not compatible with 4.x". + """ + specs = manifest.get('compatible_versions') + if not isinstance(specs, list) or not specs: + return None + + verdicts = [_satisfies_range(core, s) for s in specs if isinstance(s, str)] + parsed = [v for v in verdicts if v is not None] + if not parsed: + return None + return any(parsed) + + 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. @@ -56,16 +156,25 @@ def declared_min_version(manifest: Dict[str, Any]) -> Optional[str]: 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. + + Container types are validated rather than assumed. A hand-edited or + third-party manifest can carry `requires` as a list or `versions` as a + mapping, and both used to raise out of here (`AttributeError` and + `KeyError` respectively). That now matters far more than it did: the + untrustworthy-core branch of :func:`check` calls this for *every* manifest, + so one malformed file would take down the install path rather than just + itself. A shape we do not recognise means "no declared floor". """ - declared = ( - manifest.get('min_ledmatrix_version') - or (manifest.get('requires') or {}).get('min_ledmatrix_version') - ) + declared = manifest.get('min_ledmatrix_version') + if not declared: + requires = manifest.get('requires') + if isinstance(requires, dict): + declared = requires.get('min_ledmatrix_version') if declared: return declared - versions = manifest.get('versions') or [] - if versions and isinstance(versions[0], dict): + versions = manifest.get('versions') + if isinstance(versions, list) and versions and isinstance(versions[0], dict): return (versions[0].get('ledmatrix_min_version') or versions[0].get('ledmatrix_min')) return None @@ -74,26 +183,68 @@ def declared_min_version(manifest: Dict[str, Any]) -> Optional[str]: 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. + Two fields can say a plugin is incompatible and **the more restrictive + wins**: + + - `compatible_versions` — the schema-required array of semver ranges, and + the only one that can express an upper bound. + - `ledmatrix_min_version` (or the deprecated `ledmatrix_min`) — the + per-release floor inside `versions[]`. + + They agree across every published manifest today except `7-segment-clock`, + but they *can* disagree, and a plugin that says `["2.0.0 - 2.9.9"]` means + "not compatible with 3.x" no matter what its floor says. + + ``compatible`` is False **only** on evidence: the core reports a parseable, + trustworthy version and a field genuinely excludes it. Every uncertain case + resolves to compatible — nothing declared, an unparseable version on either + side, or a core below `TRUSTWORTHY_FLOOR`. Refusing on a guess breaks a + working install, which is the more expensive mistake here. ``reason`` is user-facing text, present only when incompatible. """ + current = parse_semver(core_version) + name = manifest.get('name') or manifest.get('id') or 'This plugin' + + if current is None or current < TRUSTWORTHY_FLOOR: + # The version is not evidence of what this core HAS. But a floor above + # the ecosystem baseline says the plugin needs modules that arrived + # *after* 2.0.0 — and a core reporting below that either is the v3.1.0 + # release (which ships __version__ = "1.0.0" and has none of the 3.2.0 + # modules) or is genuinely ancient. Either way it will not have them. + # + # This is the only protection available to that population: they cannot + # be told apart from a real 1.0.0 install, so the gate cannot reason + # about them, and the *plugin's* guarded-import fallback disappears at + # the B6 sunset. Refusing the install leaves them on the version they + # already run instead of handing them one that fails to load. + # + # Floors at or below 2.0.0 are still allowed, which is every manifest + # published today — so this does not lock anyone out of the store. + declared = declared_min_version(manifest) + needed = parse_semver(declared) + if needed is not None and needed > TRUSTWORTHY_FLOOR: + return False, ( + f"{name} requires LEDMatrix {declared} or newer. This system " + f"reports {core_version}, which is too old to identify " + f"reliably — update LEDMatrix, then install it." + ) + return True, None + + # Ranges first: they are the canonical field and can rule out a core that + # clears the floor. + if satisfies_compatible_versions(manifest, current) is False: + specs = ", ".join( + s for s in manifest.get('compatible_versions', []) if isinstance(s, str)) + return False, ( + f"{name} supports LEDMatrix {specs}, but this system is running " + f"{core_version}. Install a build in that range, or a plugin " + f"version that supports {core_version}." + ) + 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' + if needed is not None and needed > current: return False, ( f"{name} requires LEDMatrix {declared} or newer, but this system is " f"running {core_version}. Update LEDMatrix first, then install it." diff --git a/test/test_plugin_compatibility_gate.py b/test/test_plugin_compatibility_gate.py index f0f252f1..26834ffa 100644 --- a/test/test_plugin_compatibility_gate.py +++ b/test/test_plugin_compatibility_gate.py @@ -81,17 +81,24 @@ class TestCheck: 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): + def test_untrustworthy_core_allows_todays_ecosystem_floor(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.""" + 2.0.0, so blocking *that* would stop those users installing any plugin + at all — strictly worse than the problem being solved. + + A floor ABOVE 2.0.0 is refused instead; see + TestUntrustworthyCoreAndTheSunset for why that case is different.""" ok, reason = compatibility.check( - {"min_ledmatrix_version": "3.2.0"}, "1.0.0") + {"min_ledmatrix_version": "2.0.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") + def test_unparseable_core_version_allows_the_ecosystem_floor(self): + """An unidentifiable core is treated exactly like an untrustworthy one: + today's 2.0.0 floor is allowed, a post-sunset floor is not.""" + ok, _ = compatibility.check({"min_ledmatrix_version": "2.0.0"}, "not-a-version") assert ok is True + ok, _ = compatibility.check({"min_ledmatrix_version": "3.2.0"}, "not-a-version") + assert ok is False def test_unparseable_floor_allows(self): ok, _ = compatibility.check({"min_ledmatrix_version": {"nope": 1}}, "3.2.0") @@ -230,3 +237,194 @@ class TestLoaderAndStoreAgree: ) assert loader_would_warn is (not expected) assert hasattr(PluginLoader, "_warn_if_incompatible") + + +# -------------------------------------------------------------------------- +# compatible_versions — the schema-required field, and the only one that can +# express an upper bound +# -------------------------------------------------------------------------- + +class TestCompatibleVersions: + @pytest.mark.parametrize("spec,core,expected", [ + (">=2.0.0", "3.2.0", True), + (">=2.0.0", "1.9.9", False), + ("<=3.0.0", "3.2.0", False), + ("<=3.0.0", "2.9.0", True), + (">3.2.0", "3.2.0", False), + ("<4.0.0", "3.2.0", True), + ("3.2.0", "3.2.0", True), # bare == exact match + ("3.2.0", "3.2.1", False), + ("~3.2.0", "3.2.9", True), # patch-level only + ("~3.2.0", "3.3.0", False), + ("^3.2.0", "3.9.9", True), # minor + patch + ("^3.2.0", "4.0.0", False), + ("2.0.0 - 3.2.0", "3.2.0", True), # inclusive both ends + ("2.0.0 - 3.2.0", "2.0.0", True), + ("2.0.0 - 3.2.0", "3.2.1", False), + ("v3.2.0", "3.2.0", True), # leading v tolerated + ("3.2.0-beta.1", "3.2.0", True), # prerelease suffix ignored + ]) + def test_range_forms(self, spec, core, expected): + got = compatibility.satisfies_compatible_versions( + {"compatible_versions": [spec]}, compatibility.parse_semver(core)) + assert got is expected, f"{spec!r} vs {core}" + + def test_array_is_alternatives_not_conjunction(self): + """Satisfying any one entry is enough — otherwise ['<2.0.0','>=3.0.0'] + could never be satisfied by anything.""" + m = {"compatible_versions": ["<2.0.0", ">=3.0.0"]} + assert compatibility.satisfies_compatible_versions( + m, compatibility.parse_semver("3.2.0")) is True + + def test_absent_or_unparseable_is_no_evidence(self): + core = compatibility.parse_semver("3.2.0") + assert compatibility.satisfies_compatible_versions({}, core) is None + assert compatibility.satisfies_compatible_versions( + {"compatible_versions": []}, core) is None + assert compatibility.satisfies_compatible_versions( + {"compatible_versions": ["not a version"]}, core) is None + # One unparseable entry alongside a good one must not poison the result. + assert compatibility.satisfies_compatible_versions( + {"compatible_versions": ["garbage", ">=2.0.0"]}, core) is True + + +class TestMoreRestrictiveWins: + def test_upper_bound_blocks_a_core_that_clears_the_floor(self): + """The gap this closes: the floor says 2.0.0 and the core is 3.2.0, so + the floor alone would allow it — but the plugin said it stops at 2.x.""" + m = {"name": "Legacy Plugin", + "compatible_versions": ["2.0.0 - 2.9.9"], + "versions": [{"ledmatrix_min_version": "2.0.0"}]} + ok, reason = compatibility.check(m, "3.2.0") + assert ok is False + assert "2.0.0 - 2.9.9" in reason and "3.2.0" in reason + + def test_floor_blocks_when_ranges_would_allow(self): + m = {"name": "Needs Newer", + "compatible_versions": [">=1.0.0"], + "versions": [{"ledmatrix_min_version": "9.9.9"}]} + ok, reason = compatibility.check(m, "3.2.0") + assert ok is False + assert "9.9.9" in reason + + def test_both_satisfied_allows(self): + m = {"compatible_versions": [">=2.0.0"], + "versions": [{"ledmatrix_min_version": "2.0.0"}]} + assert compatibility.check(m, "3.2.0") == (True, None) + + def test_untrustworthy_core_still_bypasses_both_checks(self): + """A core reporting 1.0.0 fails `>=2.0.0`, which 41 of 42 published + manifests declare. Blocking there would empty the plugin store for + exactly the users who cannot be helped by it.""" + m = {"compatible_versions": [">=2.0.0"], + "versions": [{"ledmatrix_min_version": "2.0.0"}]} + assert compatibility.check(m, "1.0.0") == (True, None) + + +class TestUntrustworthyCoreAndTheSunset: + """The population B6 would otherwise break. + + A device installed from the v3.1.0 release reports `__version__ = "1.0.0"`. + The gate cannot tell it apart from a genuine 1.0.0 install, so it cannot + reason about what that core actually has — and at the B6 sunset the + plugin's guarded-import fallback is gone. Without this rule the store hands + those users a 3.2.0-floored plugin that fails to load, and nothing else in + the system protects them. + + The rule: on an untrustworthy core, refuse a floor *above* the ecosystem + baseline, allow anything at or below it. Every manifest published today + floors at exactly 2.0.0, so nobody is locked out of the store. + """ + + UNTRUSTWORTHY = ["1.0.0", "0.9.0", "1.9.9"] + + @pytest.mark.parametrize("core", UNTRUSTWORTHY) + def test_refuses_a_post_sunset_floor(self, core): + m = {"name": "Hockey Scoreboard", + "versions": [{"ledmatrix_min_version": "3.2.0"}]} + ok, reason = compatibility.check(m, core) + assert ok is False, ( + f"core {core} must not receive a 3.2.0-floored plugin: after the " + "sunset there is no fallback and it will fail to load" + ) + assert "3.2.0" in reason and core in reason + + @pytest.mark.parametrize("core", UNTRUSTWORTHY) + def test_still_allows_todays_ecosystem_floor(self, core): + """Regression guard: every published manifest floors at 2.0.0. If this + starts refusing, those users lose the plugin store entirely.""" + m = {"versions": [{"ledmatrix_min": "2.0.0"}]} + assert compatibility.check(m, core) == (True, None) + + @pytest.mark.parametrize("core", UNTRUSTWORTHY) + def test_still_allows_an_undeclared_floor(self, core): + assert compatibility.check({"id": "x"}, core) == (True, None) + + def test_trustworthy_core_below_the_floor_is_unaffected(self): + """A core that reports 3.1.0 is believable and already handled by the + ordinary comparison — not by this rule.""" + m = {"name": "P", "versions": [{"ledmatrix_min_version": "3.2.0"}]} + ok, reason = compatibility.check(m, "3.1.0") + assert ok is False + assert "too old to identify" not in reason, ( + "a believable version should get the ordinary message" + ) + + +class TestMalformedManifests: + """A manifest we cannot parse must read as "no declared floor", not raise. + + This matters more since the untrustworthy-core branch of check() began + resolving the floor for *every* manifest: one hand-edited or third-party + file with the wrong shape would take down the whole install path rather + than just itself. + """ + + @pytest.mark.parametrize("manifest", [ + {"requires": ["python>=3.9"]}, # a list, not a mapping + {"requires": "python>=3.9"}, # a bare string + {"versions": {"a": 1}}, # a mapping, not a list + {"versions": "1.0.0"}, # a bare string + {"versions": [None]}, # a list of the wrong thing + {"versions": []}, + ]) + def test_shape_errors_read_as_no_floor(self, manifest): + assert compatibility.declared_min_version(manifest) is None + assert compatibility.check(manifest, "1.0.0") == (True, None) + assert compatibility.check(manifest, "3.2.0") == (True, None) + + def test_a_valid_requires_block_still_works(self): + assert compatibility.declared_min_version( + {"requires": {"min_ledmatrix_version": "3.2.0"}}) == "3.2.0" + + +class TestSuffixedVersions: + """Prerelease and build metadata must not leak into the numbers. + + The digit scrape used to pull them in: "3.2.0+build42" became (3, 2, 42) + and "3.2.0-rc1" became (3, 2, 1) — a release candidate ranking above its + own release. Both fed reject decisions. + """ + + @pytest.mark.parametrize("text,expected", [ + ("3.2.0", (3, 2, 0)), + ("3.2.0+build42", (3, 2, 0)), + ("3.2.0-rc1", (3, 2, 0)), + ("3.2.0-rc.1+build.9", (3, 2, 0)), + ("v3.2.0+build42", (3, 2, 0)), + ]) + def test_suffixes_are_dropped(self, text, expected): + assert compatibility.parse_semver(text) == expected + + def test_a_build_of_the_pinned_version_is_not_refused(self): + """The regression: an exact "3.2.0" pin refused a core running + 3.2.0+build42, which is that same version.""" + ok, reason = compatibility.check( + {"compatible_versions": ["3.2.0"]}, "3.2.0+build42") + assert ok is True, f"refused a build of the pinned version: {reason}" + + def test_a_release_candidate_does_not_outrank_its_release(self): + m = {"versions": [{"ledmatrix_min_version": "3.2.0"}]} + assert compatibility.check(m, "3.2.0-rc1")[0] is True + # ...and still refuses something genuinely older. + assert compatibility.check(m, "3.1.0-rc1")[0] is False