diff --git a/src/plugin_system/compatibility.py b/src/plugin_system/compatibility.py index aeb8d351..784a6b6a 100644 --- a/src/plugin_system/compatibility.py +++ b/src/plugin_system/compatibility.py @@ -40,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: @@ -143,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 diff --git a/test/test_plugin_compatibility_gate.py b/test/test_plugin_compatibility_gate.py index 0d9c356b..26834ffa 100644 --- a/test/test_plugin_compatibility_gate.py +++ b/test/test_plugin_compatibility_gate.py @@ -369,3 +369,62 @@ class TestUntrustworthyCoreAndTheSunset: 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