mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-04 18:28:06 +00:00
d305be60897d70112ddba7c956cfb5bec7c4cb32
2
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
53af53b4a1 |
feat(store): evaluate compatible_versions, not just the floor (#433)
* feat(store): evaluate compatible_versions, not just the floor Closes the gap CodeRabbit surfaced on #427. `compatible_versions` is the canonical compatibility contract -- schema/manifest_schema.json marks it required, all 42 published manifests carry it -- and it is the only field that can express an *upper* bound. `ledmatrix_min_version` is a floor and cannot say "not compatible with 4.x". The gate read only the floor, so a plugin declaring ["2.0.0 - 2.9.9"] would be installed on 3.2.0 regardless of having said it stops at 2.x. check() now evaluates both and the more restrictive wins. The array is a set of alternatives (satisfying any one entry suffices), supporting every form the schema permits: >=, <=, >, <, ~, ^, a bare exact version, and an inclusive "A - B" range, with prerelease/build suffixes tolerated. Refusal still requires evidence. Anything unparseable, absent, or below TRUSTWORTHY_FLOOR resolves to compatible. That last point needed a new strict parser. parse_semver is deliberately lenient -- it strips non-digits and yields (0, 0, 0) for a string with no numbers at all. Harmless for a floor (0.0.0 never blocks) but wrong for a range, where the same leniency turned an unreadable spec into a *refusal*: a manifest whose only entry was garbage got compared against 0.0.0 and refused. Range specs are now shape-checked first, so garbage reads as "no evidence". parse_semver itself is unchanged, since the loader depends on its behaviour. Verified: 815 core unit tests pass, 18 of them new. Swept the real registry -- all 42 published manifests, at cores 1.0.0 / 2.0.0 / 3.1.0 / 3.2.0 / 4.0.0 -- and nothing is refused at any of them. The gate stays inert for shipped plugins, which is the property that makes it safe to land ahead of B5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * feat(store): protect the one population the sunset would break The B6 sunset deletes each plugin's guarded-import fallback, so a plugin that floors at 3.2.0 must never reach a core that lacks the 3.2.0 modules. The gate could not stop that for the population most at risk. A device installed from the v3.1.0 release reports __version__ = "1.0.0". The gate treated anything below TRUSTWORTHY_FLOOR as "unknown, do not block" -- correct while every manifest floors at 2.0.0, because blocking would have emptied the plugin store for those users. But after the sunset it hands them a 3.2.0-floored plugin with no fallback, which fails to load with one log line. Nothing else in the system protects them: they cannot be told apart from a genuine 1.0.0 install. On an untrustworthy core the gate now refuses a floor ABOVE 2.0.0 and still allows anything at or below it. A floor above the ecosystem baseline says the plugin needs modules that arrived after 2.0.0, and a core reporting below that -- whether it is the v3.1.0 release or something genuinely ancient -- will not have them. Refusing leaves the user on the version they already run instead of one that cannot load. Measured against all 42 published manifests: today (every manifest floors at 2.0.0) core 1.0.0 / 2.0.0 / 3.1.0 / 3.2.0 / unparseable -> 0 of 42 refused after B6 (same manifests floored at 3.2.0) core 1.0.0 -> 38 refused, core 3.1.0 -> 38 refused, core 3.2.0 -> 0 So nobody loses the store today, and the sunset cannot reach a core that cannot run it. Two older tests asserted the previous "allow everything" behaviour; they now express the new rule with a 2.0.0 floor, which is what their no-lockout intent was actually about. The 38-of-42 in that measurement surfaced a separate B6 trap, recorded here because it will bite whoever raises the floors: four plugins (flights, leaderboard, music, stocks) declare the floor as a TOP-LEVEL `min_ledmatrix_version`, a third spelling, which declared_min_version checks before the versions[] array. For those, editing versions[0] is a silent no-op and the floor stays at 2.0.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(store): address review — malformed manifests, and suffixed versions Both CodeRabbit findings on #433 verified against the code and fixed. 1. Malformed manifest sections raised instead of degrading. `requires` as a list hit AttributeError ('list' object has no attribute 'get') and `versions` as a mapping hit KeyError: 0. Both reproduced. This got worse with the sunset rule in the previous commit: that branch resolves the floor for *every* manifest on an untrustworthy core, where the old code returned early. One hand-edited or third-party file with the wrong shape would have taken down the whole install path rather than just itself. Container types are now validated and an unrecognised shape reads as "no declared floor". 2. Prerelease and build metadata leaked into the version numbers. The digit scrape parsed "3.2.0+build42" as (3, 2, 42) and "3.2.0-rc1" as (3, 2, 1) -- a release candidate ranking above its own release. Both fed reject decisions, and the consequence was demonstrable: a plugin pinned to exactly "3.2.0" refused a core running 3.2.0+build42, which is that same version. The suggested remedy -- use the strict token parser -- would not have fixed it. _parse_strict validates the shape but delegates the numbers to parse_semver, so it returned the same (3, 2, 42). The bug is in the scrape, so suffixes are now dropped before it. Prereleases compare equal to their release rather than below it; full prerelease ordering is more than any caller needs and equal is far closer to right than what it did before. parse_semver is shared with PluginLoader, so its suite was re-run: unchanged, and it only ever gets more correct here. Verified: 839 core unit tests pass, 21 of them new -- six malformed shapes, five suffixed forms, and the two demonstrated regressions. The real-registry sweep is unchanged at 0 of 42 refused across cores 1.0.0, 3.1.0, 3.2.0, 3.2.0+build42 and an unparseable string. 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> |
||
|
|
970ca2d04f |
feat(store): refuse to install a plugin that needs a newer core (re-target of #429) (#431)
* 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 * ci: run the new suites, and check tag/version agreement at release time These were split out of #428/#429 because the token pushing them lacked the `workflow` scope. Folding them in here rather than opening a stacked PR -- #429 was merged into its stacked base after that base had already been squash-merged, so its content never reached main, and one such near-miss is enough. All three enrolled suites exist on this branch: test_version_consistency.py came with #428 and is on main; the other two arrive with the commits above. Enrolling them in a separate PR would have either raced with this one on test.yml or briefly pointed CI at files main did not have. - test.yml: enroll test_version_consistency, test_plugin_compatibility_gate and test_install_preserves_existing in the core unit job. Until now these 32 tests existed but nothing ran them automatically. - release-version-check.yml: run scripts/check_release_version.py on pushed v* tags and published releases, plus workflow_dispatch so a tag can be checked *before* it is created. No dependencies -- it reads src/__init__.py and CHANGELOG.md only. Verified: both workflow files parse, and the release check still passes for v3.2.0 against this tree. 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> |