From 5ea0d511dc782fbe20cbe65f356c790db1a5238d Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Wed, 30 Sep 2026 09:31:16 -0400 Subject: [PATCH] feat(store): read ledmatrix_min_version, aliases and commit from the registry (#686) The store reads three optional registry fields: ledmatrix_min_version (an incompatible install/update is refused before any download, with a "Needs LEDMatrix X+" card badge), aliases (update/uninstall/reinstall by registry id find a plugin installed under its manifest id, with registry proof only), and commit (shown and linked on the store card). An older plugins.json behaves as before. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 32 ++ CLAUDE.md | 1 + docs/REST_API_REFERENCE.md | 20 +- docs/SPORTS_UNIFICATION.md | 8 +- src/plugin_system/store_install.py | 48 ++- src/plugin_system/store_manager.py | 75 +++- src/plugin_system/store_registry.py | 151 ++++++- src/plugin_system/store_update.py | 36 +- test/js/README.md | 1 + test/js/run_all.js | 3 +- test/js/unit/test_store_registry_fields.js | 105 +++++ test/test_plugin_dirs.py | 7 +- test/test_store_registry_fields.py | 388 ++++++++++++++++++ .../blueprints/api_v3/plugin_store.py | 68 ++- web_interface/static/v3/plugins_manager.js | 18 +- 15 files changed, 922 insertions(+), 39 deletions(-) create mode 100644 test/js/unit/test_store_registry_fields.js create mode 100644 test/test_store_registry_fields.py diff --git a/CHANGELOG.md b/CHANGELOG.md index d25c432b..905bcb91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -118,8 +118,40 @@ read any of them: `"fixed"`): both always scrolled. Documented only; no warning, because official plugins' schemas still offer `"fixed"`. +### Plugin store + +- The store reads three optional registry fields that ledmatrix-plugins' + `update_registry.py` now publishes (ChuckBuilds/ledmatrix-plugins#579). An + older `plugins.json` without them behaves as before. + - `ledmatrix_min_version`: an install or update this core cannot run is + refused before anything is downloaded, pulled or moved aside, and the web + UI says why ("requires LEDMatrix X or newer…", HTTP 409) instead of "check + logs for details". The store card shows a "Needs LEDMatrix X+" badge. The + check on the downloaded manifest stays as the fallback (older registries, + an explicitly requested other branch, `compatible_versions`). + - `aliases`: the entry's other ids. Update, uninstall and reinstall by the + registry id now find a plugin installed under its manifest id + (`weather` → `ledmatrix-weather/`; likewise leaderboard, music, stocks). + Only registry proof counts: the entry's `aliases` or its `plugin_path` + name, or a folder whose manifest declares one of those ids. A + `ledmatrix-/` folder with no such proof is never replaced or removed; + uninstall and update report "not installed" and log the folder's path. + Install and update fetch the registry first when such a folder exists + and none is loaded; uninstall stays offline. + - `commit`: the monorepo commit that introduced the listed version, shown + on the store card and linked to the plugin's source at that commit. + Informational only; installs still come from the branch head. + ### Fixes +- Reinstalling a plugin by its registry id when it is installed under its + manifest id (`weather` in `ledmatrix-weather/`) no longer deletes it when + the install then fails. The safety copy was taken of `weather/`, which did + not exist, and the real install was removed to make room for the download, + so a refusal by the compatibility gate left no plugin at all. Uninstalling + by the registry id reported success and removed nothing; updating by it + said "not installed". All three now find the install. + - On-demand no longer restarts a running display. `POST /display/on-demand/start` treated `start_service` (on by default, and what "Preview on display", the on-demand dialog and the MQTT bridge all send) as diff --git a/CLAUDE.md b/CLAUDE.md index c0805ca1..a64c558e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -46,6 +46,7 @@ - Store manager (`PluginStoreManager` in `src/plugin_system/store_manager.py`) handles install/update/uninstall - Monorepo plugins are installed without a `.git` directory: GitHub Trees API + raw downloads, falling back to ZIP extraction - Update detection for monorepo plugins uses version comparison (manifest version vs registry latest_version) +- Optional registry entry fields (`store_registry.py`): `ledmatrix_min_version` refuses an incompatible install/update before the download (the post-download manifest gate stays as the fallback); `aliases` are the entry's other ids (manifest id `ledmatrix-weather` for `weather`), used with the `plugin_path` name by update/uninstall/reinstall to find the install — only this registry proof counts, never a bare `ledmatrix-` folder (owner decision, #686; such a folder is only logged); `commit` is informational. An older plugins.json has none of them - Plugin configs stored in `config/config.json`, NOT in plugin directories — safe across reinstalls - Third-party plugins can use their own repo URL with empty `plugin_path` diff --git a/docs/REST_API_REFERENCE.md b/docs/REST_API_REFERENCE.md index 4737ef5d..243d4d88 100644 --- a/docs/REST_API_REFERENCE.md +++ b/docs/REST_API_REFERENCE.md @@ -687,6 +687,12 @@ Install a plugin from the plugin store. When the operation queue is unavailable the install runs synchronously and the response has only a `message`. +A plugin whose registry entry (or downloaded manifest) needs a newer +LEDMatrix is refused: the synchronous install answers `409` with a message +such as `Failed to install plugin x: X requires LEDMatrix 3.8.0 or newer…`, +and a queued one fails with that message. Nothing already installed is +changed. + ### Uninstall Plugin **POST** `/api/v3/plugins/uninstall` @@ -735,6 +741,9 @@ Update a plugin to the latest version. Runs synchronously. } ``` +An update this core cannot run answers `409` with `Plugin update refused:` +and the reason; the installed version is left as it was. + ### Install Plugin from URL **POST** `/api/v3/plugins/install-from-url` @@ -1235,13 +1244,22 @@ searches. "version": "1.2.3", "branch": "main", "default_branch": "main", - "plugin_path": "plugins/football-scoreboard" + "plugin_path": "plugins/football-scoreboard", + "commit": "843588025a81197056f8d96779ccb2be19337ab8", + "ledmatrix_min_version": "3.7.0", + "aliases": [], + "incompatible_reason": null } ] } } ``` +`commit` (the monorepo commit that introduced `version`), +`ledmatrix_min_version` and `aliases` come from the registry entry and are +`null` / `[]` when an older registry lacks them. `incompatible_reason` is the +message an install would be refused with on this core, or `null`. + ### Get GitHub Status **GET** `/api/v3/plugins/store/github-status` diff --git a/docs/SPORTS_UNIFICATION.md b/docs/SPORTS_UNIFICATION.md index 3c81317b..03dbd53a 100644 --- a/docs/SPORTS_UNIFICATION.md +++ b/docs/SPORTS_UNIFICATION.md @@ -514,8 +514,12 @@ a floor can be trusted against, and today it is not: the update path that re-downloads. `update_plugin`'s git branch pulls in place and re-downloads nothing, so it stayed ungated until `_gate_pulled_commit` closed it — checked after the pull - (the registry carries no floor field, so the incoming floor is unknowable - before it) and undone with `git reset --hard` to the pre-pull commit. That + (the registry then carried no floor field, so the incoming floor was + unknowable before it) and undone with `git reset --hard` to the pre-pull + commit. The registry now publishes `ledmatrix_min_version`, and install and + update refuse on it before downloading or pulling; both post-download gates + remain as the fallback for older registries, other branches and + `compatible_versions`. That route is rare in practice, since monorepo plugins install as archives; it was closed because the sunset rule in the plugins repo's `08-shared-sports-code.md` states as **condition 3** that the core enforces diff --git a/src/plugin_system/store_install.py b/src/plugin_system/store_install.py index e858fc59..1665f7bc 100644 --- a/src/plugin_system/store_install.py +++ b/src/plugin_system/store_install.py @@ -63,7 +63,14 @@ class _InstallMixin: return False with self._get_reinstall_lock(plugin_id): - plugin_path = self.plugins_dir / plugin_id + # The copy to protect is wherever this plugin is installed, not + # necessarily plugins_dir/: asked for the registry id + # `weather`, the install lives in `ledmatrix-weather/`, the + # manifest's id. Backing up only `weather/` protected nothing, + # and _install_plugin_impl then deleted `ledmatrix-weather/` to + # make room for the download -- so a refusal after that point + # (the post-download compatibility gate) left no plugin at all. + plugin_path = self._existing_install(plugin_id) or self.plugins_dir / plugin_id if not plugin_path.exists(): return self._install_plugin_impl(plugin_id, branch) @@ -91,6 +98,26 @@ class _InstallMixin: self._restore_backup(plugin_id, plugin_path, backup_path, "Install") return False + def _existing_install(self, plugin_id: str) -> Optional[Path]: + """The installed copy of ``plugin_id`` in plugins_dir, by the id or an + alias the registry proves (`_installed_id_candidates`). + + When nothing matches but a ``ledmatrix-`` folder exists and no + registry is loaded yet, the registry is fetched first -- the install + fetches it anyway -- because without it that folder can be neither + protected nor trusted: the download may be renamed onto it. + """ + dirs = [self.plugins_dir] + found = self._resolve_installed(plugin_id, dirs) + if (found is None and not getattr(self, 'registry_cache', None) + and self._unproven_prefix_folder(plugin_id, dirs) is not None): + try: + self.fetch_registry() + except Exception as e: # noqa: BLE001 - proceed as before without proof + self.logger.debug("Registry fetch before installing %s failed: %s", plugin_id, e) + found = self._resolve_installed(plugin_id, dirs) + return found + def _set_aside(self, plugin_path: Path, backup_path: Path) -> Optional[str]: """Rename an installed plugin to ``backup_path`` so a failed (re)install can put it back. @@ -164,6 +191,16 @@ class _InstallMixin: self.logger.error(f"Plugin {plugin_id} missing repository URL") return False + # The registry's floor describes the release on the entry's branch. + # Checked here, before anything is removed or downloaded; the gate on + # the downloaded manifest below stays as the fallback (older + # registries, compatible_versions ranges). A different branch asked + # for by name is a different release, so only the fallback applies. + registry_branch = plugin_info.get('branch') or plugin_info.get('default_branch') + if (not branch or not registry_branch or branch == registry_branch) and \ + self._refuse_if_registry_incompatible(plugin_id, plugin_info, "install"): + return False + plugin_subpath = plugin_info.get('plugin_path') # If branch is provided, prioritize it; otherwise use default logic branch_candidates = self._distinct_sequence([ @@ -280,9 +317,11 @@ class _InstallMixin: 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. + # registry's `ledmatrix_min_version` already refused the + # common case before the download (above); this is the + # fallback for a registry without it, a branch other than the + # registry's, and `compatible_versions`, which only the + # manifest carries. Before dependency installation, still. # # Refusing costs the user nothing: on an update this returns # False and _reinstall_with_rollback restores the version they @@ -299,6 +338,7 @@ class _InstallMixin: if not compatible: self.logger.error( "Refusing to install %s: %s", plugin_id, reason) + self._note_refusal(requested_id, reason) self._safe_remove_directory(plugin_path) return False diff --git a/src/plugin_system/store_manager.py b/src/plugin_system/store_manager.py index 4cde3fc2..d7b32e68 100644 --- a/src/plugin_system/store_manager.py +++ b/src/plugin_system/store_manager.py @@ -21,7 +21,7 @@ from src.plugin_system.plugin_dirs import ( PluginDirectoryIndex, resolve_plugin_dir, store_search_dirs, ) from src.plugin_system.store_install import _InstallMixin -from src.plugin_system.store_registry import _RegistryMixin +from src.plugin_system.store_registry import _RegistryMixin, prefix_hint from src.plugin_system.store_update import _UpdateMixin @@ -407,13 +407,20 @@ class PluginStoreManager(_RegistryMixin, _InstallMixin, _UpdateMixin): alone reported such a plugin as not installed, so update_plugin() silently did nothing. - No ``ledmatrix-`` prefix and no case folding here, unlike the loader: - a store operation may delete what this returns, so it only accepts a - directory that names the id exactly or declares it. So a registry id - such as `stocks` does not resolve to an installed `ledmatrix-stocks/` - declaring `ledmatrix-stocks` (the monorepo's leaderboard, music, - stocks and weather); callers pass the installed id, and - update_plugin() maps it back to the registry id itself. + When nothing answers to the id itself, the ids the registry proves + are the same plugin are tried the same way + (`_installed_id_candidates`): the entry's own id, its ``aliases`` and + its ``plugin_path`` name. So the registry id `stocks` finds an + installed `ledmatrix-stocks/` declaring `ledmatrix-stocks` (the + monorepo's leaderboard, music, stocks and weather), and uninstalling + by the registry id no longer reports success while leaving the + plugin on disk. + + Never ``ledmatrix-`` without that proof -- no registry loaded, or + an entry that doesn't name it: a store operation may delete or + replace what this returns, and an unrelated plugin can own that + folder. Such a folder is only logged, so a person can act on it. + Still no case folding. Args: plugin_id: Plugin identifier @@ -421,9 +428,55 @@ class PluginStoreManager(_RegistryMixin, _InstallMixin, _UpdateMixin): Returns: Path to plugin directory if found, None otherwise """ - return resolve_plugin_dir( - plugin_id, self._candidate_plugin_dirs(), prefix=False, - case_insensitive=False) + return self._find_with_proof(plugin_id, fetch=False) + + def _find_with_proof(self, plugin_id: str, fetch: bool) -> Optional[Path]: + """`_find_plugin_path`; with ``fetch``, a ``ledmatrix-`` folder + found while no registry is loaded makes it fetch the registry and + look again, since only the registry can prove the folder is this + plugin. Uninstall passes False (it must work offline); update, which + needs the network anyway, passes True.""" + search_dirs = self._candidate_plugin_dirs() + found = self._resolve_installed(plugin_id, search_dirs) + if found is not None: + return found + folder = self._unproven_prefix_folder(plugin_id, search_dirs) + if folder is not None and fetch and not getattr(self, 'registry_cache', None): + try: + self.fetch_registry() + except Exception as e: # noqa: BLE001 - fall through to "not found" + self.logger.debug("Registry fetch while looking for %s failed: %s", plugin_id, e) + found = self._resolve_installed(plugin_id, search_dirs) + if found is not None: + return found + if folder is not None: + self.logger.warning( + "Plugin %s not found. %s may be it, but nothing in the plugin " + "registry says so (no alias), so the store leaves it alone; " + "if it is this plugin, manage it as %s.", + plugin_id, folder, prefix_hint(plugin_id)) + return None + + @staticmethod + def _unproven_prefix_folder(plugin_id: str, search_dirs: List[Path]) -> Optional[Path]: + """A ``ledmatrix-`` folder, which the store names but won't touch.""" + hint = prefix_hint(plugin_id) + if hint is None: + return None + return resolve_plugin_dir(hint, search_dirs, prefix=False, by_manifest=False) + + def _resolve_installed(self, plugin_id: str, search_dirs: List[Path]) -> Optional[Path]: + """The first of ``plugin_id``'s candidate ids found in ``search_dirs``. + + The id itself is looked for in every directory before any alias is, + so an exact install anywhere beats an alias in the configured one. + """ + for candidate in self._installed_id_candidates(plugin_id): + found = resolve_plugin_dir( + candidate, search_dirs, prefix=False, case_insensitive=False) + if found is not None: + return found + return None def _candidate_plugin_dirs(self) -> List[Path]: """Directories that may hold installed plugins, configured one first.""" diff --git a/src/plugin_system/store_registry.py b/src/plugin_system/store_registry.py index 0214c7b5..99c2e5ef 100644 --- a/src/plugin_system/store_registry.py +++ b/src/plugin_system/store_registry.py @@ -13,11 +13,62 @@ from datetime import datetime from pathlib import Path from typing import List, Dict, Optional, Any from jsonschema import Draft7Validator, ValidationError +from src.plugin_system.plugin_dirs import PLUGIN_DIR_PREFIX from src.plugin_system.repo_urls import ( github_api_headers, github_owner_repo, normalize_repo_url, ) +# Registry entry fields the plugin monorepo's update_registry.py added after +# 3.7.0. All optional: an older plugins.json has none of them, and every +# reader here treats a missing or malformed one as "not stated". +# +# - ``ledmatrix_min_version``: the floor the plugin's manifest declares, so an +# incompatible install or update is refused before the download +# (`registry_incompatibility`). The post-download gate stays as the fallback. +# - ``aliases``: other ids the plugin goes by (the manifest id when it differs +# from the registry id, e.g. ``ledmatrix-weather`` for ``weather``). With +# ``plugin_path``'s name, the only proof the store accepts that a folder +# under another name is this plugin (`alternate_ids`). +# - ``commit``: the monorepo commit that introduced ``latest_version``. +# Informational only -- installs still come from the branch head. + + +def declared_aliases(entry: Dict[str, Any]) -> Optional[List[str]]: + """The entry's ``aliases``, or None when it carries no such list.""" + aliases = entry.get('aliases') + if not isinstance(aliases, list): + return None + own = entry.get('id') + return [a for a in aliases if isinstance(a, str) and a and a != own] + + +def alternate_ids(entry: Dict[str, Any]) -> List[str]: + """Ids other than the registry id that the registry *proves* an installed + copy may carry: the entry's ``aliases``, then its ``plugin_path`` + directory name (all an older registry has). + + Never ``ledmatrix-`` on its own say-so. Store operations delete and + replace what these ids resolve to, and an unrelated plugin can live in a + folder of that name (owner decision on #686). A guess is only a hint: + see `prefix_hint`. + """ + own = entry.get('id') + ids: List[str] = list(declared_aliases(entry) or []) + path = entry.get('plugin_path') + if isinstance(path, str) and path.strip('/'): + ids.append(path.rstrip('/').rsplit('/', 1)[-1]) + return [g for i, g in enumerate(ids) if g and g != own and g not in ids[:i]] + + +def prefix_hint(plugin_id: Any) -> Optional[str]: + """``ledmatrix-``: the legacy folder name worth *mentioning* when + ``plugin_id`` is not found -- never one to act on without registry proof.""" + if isinstance(plugin_id, str) and plugin_id and not plugin_id.startswith(PLUGIN_DIR_PREFIX): + return PLUGIN_DIR_PREFIX + plugin_id + return None + + class _RegistryMixin: """PluginStoreManager methods: see the module docstring.""" @@ -821,19 +872,117 @@ class _RegistryMixin: Matching ``plugin_path`` fixes it without renaming any published id, which would orphan ``plugin_state.json`` entries keyed on the old ones. Exact id always wins, so an entry whose *path* happens to collide with - another entry's id cannot shadow it. + another entry's id cannot shadow it. An entry's ``aliases`` (registries + from after 3.7.0) come next, then ``plugin_path``, which is what an + older registry has to go on. """ if not plugin_id: return None exact = next((p for p in plugins if p.get('id') == plugin_id), None) if exact is not None: return exact + for entry in plugins: + if plugin_id in (declared_aliases(entry) or ()): + return entry for entry in plugins: path = (entry.get('plugin_path') or '').rstrip('/') if path and path.rsplit('/', 1)[-1] == plugin_id: return entry return None + def registry_incompatibility(self, plugin_id: str, + entry: Optional[Dict[str, Any]] = None) -> Optional[str]: + """Why the registry says this core cannot run the plugin's latest + release, or None when it says nothing against it. + + Reads the entry's ``ledmatrix_min_version`` and asks + ``compatibility.check`` -- the same function, and so the same wording + and the same leniency (an untrustworthy or unparseable core version + allows), as the gate that runs on the downloaded manifest. That gate + stays: it also sees ``compatible_versions``, and an older registry + without the field says nothing here. + + ``entry`` defaults to the registry entry for ``plugin_id``. Any + failure to read the registry answers None: the pre-check exists to + refuse early on evidence, never to block on a guess. + """ + if entry is None: + try: + entry = self.get_registry_info(plugin_id) + except Exception as e: # noqa: BLE001 - never block an install on this + self.logger.debug("Registry lookup for %s failed: %s", plugin_id, e) + return None + if not isinstance(entry, dict): + return None + floor = entry.get('ledmatrix_min_version') + if not isinstance(floor, str) or not floor.strip(): + return None + from src.plugin_system import compatibility + compatible, reason = compatibility.check( + {'id': entry.get('id') or plugin_id, 'name': entry.get('name'), + 'min_ledmatrix_version': floor.strip()}, + compatibility.current_core_version()) + return None if compatible else reason + + def _refuse_if_registry_incompatible(self, plugin_id: str, entry: Optional[Dict[str, Any]], + action: str, record_as: Optional[str] = None) -> bool: + """Log and record a registry-based refusal; True when refused. + + ``record_as`` is the id the caller will ask `pop_refusal` about (the + id it was handed, which may be an alias of ``plugin_id``). + """ + reason = self.registry_incompatibility(plugin_id, entry) + if reason is None: + return False + self.logger.error("Refusing to %s %s before downloading it: %s", + action, plugin_id, reason) + self._note_refusal(record_as or plugin_id, reason) + return True + + def _note_refusal(self, plugin_id: str, reason: str) -> None: + """Remember why an install or update of ``plugin_id`` was refused, so + the web route can say so instead of "check logs for details".""" + refusals = self.__dict__.setdefault('_refusals', {}) + refusals[plugin_id] = reason + + def pop_refusal(self, *plugin_ids: str) -> Optional[str]: + """The compatibility refusal recorded for any of ``plugin_ids`` since + the last call, clearing them all; None when there was none.""" + refusals = self.__dict__.get('_refusals') or {} + found = None + for plugin_id in plugin_ids: + reason = refusals.pop(plugin_id, None) + if found is None and reason: + found = reason + return found + + def _installed_id_candidates(self, plugin_id: str) -> List[str]: + """``plugin_id`` and the other ids the registry proves its installed + copy may carry. + + From the registry already in memory -- no fetch, because uninstall + and the update lookup must work offline. With an entry: its id and + `alternate_ids` (``aliases``, ``plugin_path`` name). Without one (no + registry loaded yet, or a plugin that isn't in it): the id alone. + A folder whose manifest declares one of these ids is found by the + resolver's manifest pass whatever it is called. + """ + ids: List[str] = [plugin_id] + cache = getattr(self, 'registry_cache', None) + plugins = cache.get('plugins') if isinstance(cache, dict) else None + entry = None + if isinstance(plugins, list) and isinstance(plugin_id, str): + entry = self._match_registry_entry( + [p for p in plugins if isinstance(p, dict)], plugin_id) + if entry is not None: + ids.append(entry.get('id')) + ids.extend(alternate_ids(entry)) + unique: List[str] = [] + for candidate in ids: + if isinstance(candidate, str) and candidate and candidate not in unique: + unique.append(candidate) + return unique + def get_registry_info(self, plugin_id: str) -> Optional[Dict]: """ Get plugin information from the registry cache only (no GitHub API calls). diff --git a/src/plugin_system/store_update.py b/src/plugin_system/store_update.py index d84600d4..89f63a4b 100644 --- a/src/plugin_system/store_update.py +++ b/src/plugin_system/store_update.py @@ -195,10 +195,12 @@ class _UpdateMixin: surfaces as one line in the journal and a scoreboard that silently stopped appearing. - Checked after the pull rather than before it, for the same reason - ``_install_plugin_impl`` checks after the download: the registry - carries no compatibility field, so the incoming floor is only knowable - once the new commit is on disk. + The registry's ``ledmatrix_min_version`` refuses most of these before + the pull (``update_plugin``). This is the fallback, for the same cases + ``_install_plugin_impl``'s post-download gate covers: a registry + without the field, a checkout on another branch than the registry's, + and ``compatible_versions`` -- all only knowable once the new commit + is on disk. Undone with ``git reset --hard`` rather than by removing the directory. This is a live checkout, the previous commit is still in the object @@ -233,6 +235,7 @@ class _UpdateMixin: return True self.logger.error("Refusing the update to %s: %s", plugin_id, reason) + self._note_refusal(plugin_id, reason) if not previous_sha: self.logger.error( @@ -310,8 +313,10 @@ class _UpdateMixin: """ Update a plugin to the latest commit on its upstream branch. """ - plugin_path = self._find_plugin_path(plugin_id) - + # fetch=True: an update needs the registry anyway, and only it can + # prove a ledmatrix-/ folder is this plugin. + plugin_path = self._find_with_proof(plugin_id, fetch=True) + if plugin_path is None or not plugin_path.exists(): self.logger.error(f"Plugin not installed: {plugin_id}") return False @@ -368,6 +373,11 @@ class _UpdateMixin: f"Plugin {resolved_id} git remote ({local_remote}) differs from registry ({registry_repo}). " f"Reinstalling from registry to migrate to new source." ) + # Before the old copy is moved aside: the reinstall + # would only refuse after a download and a restore. + if self._refuse_if_registry_incompatible( + resolved_id, plugin_info_remote, "update", record_as=plugin_id): + return False return self._reinstall_with_rollback(resolved_id, plugin_path) # Check if already up to date @@ -375,6 +385,14 @@ class _UpdateMixin: self.logger.info(f"Plugin {plugin_id} already matches remote commit {remote_sha[:7]}") return True + # The registry's floor describes its branch; a checkout + # on another branch pulls another release, and the gate + # after the pull (_gate_pulled_commit) still covers it. + if (not remote_branch or remote_branch == local_branch) and \ + self._refuse_if_registry_incompatible( + resolved_id, plugin_info_remote, "update", record_as=plugin_id): + return False + # Update via git pull self.logger.info(f"Updating {plugin_id} via git pull (local branch: {local_branch})...") try: @@ -718,6 +736,12 @@ class _UpdateMixin: except Exception as e: self.logger.debug(f"Could not compare versions for {plugin_id}: {e}") + # A newer version this core cannot run: refuse now, while the + # installed copy is untouched, rather than after a download. + if self._refuse_if_registry_incompatible( + registry_id, plugin_info_remote, "update", record_as=plugin_id): + return False + # Plugin is not a git repo but is in registry and has a newer version - reinstall self.logger.info(f"Plugin {plugin_id} not installed via git; re-installing latest archive (registry id: {registry_id})") diff --git a/test/js/README.md b/test/js/README.md index b7e1196e..770ea40f 100644 --- a/test/js/README.md +++ b/test/js/README.md @@ -50,6 +50,7 @@ server has none. | `unit/test_style_editor_layout_leaf_columns.js` | no | `columnsFor()` from `widgets/style-editor.js`: a layout-only key whose own value is a leaf (no x/y sub-object, e.g. a `show_logo` toggle) gets a self-keyed column instead of a blank, uneditable row | | `unit/test_style_editor_layout_leaf_collision.js` | no | `columnsFor()` from `widgets/style-editor.js`: a layout-only leaf key still gets its own column even when its name collides with an unrelated element's style sub-field or another layout axis's sub-field | | `unit/test_inline_handler_escaping.js` | no | The store, saved-repository and custom-registry inline `onclick` handlers and the live `window.updateImageList` from `plugins_manager.js`: a registry id, URL or uploaded file name carrying `'`, `"` or entities adds no attributes and reaches the handler intact, and the store's View button opens only http(s) links | +| `unit/test_store_registry_fields.js` | no | The store card's registry fields from `plugins_manager.js`: the commit that introduced the listed version (a hex SHA only, linked to that tree), the "Needs LEDMatrix X+" warning, a card from an older registry without either, and `isStorePluginInstalled` answering to `aliases` | | `unit/test_plugin_action_delegation.js` | no | The document-level card-action delegation and `handlePluginAction` from `plugins_manager.js`, run with the handler inside an IIFE as in the real file: each action is handled once, a Starlark app uninstall goes to `DELETE /starlark/apps/`, and an uninstall is confirmed once | | `dom/test_installed_dom.js` | yes | The toolbar in a real DOM: pill/search/sort interaction, the HTMX partial re-swap, and a `getComputedStyle` check that `.filter-pill[data-active]` really matches the emitted markup | | `dom/test_store_dom.js` | yes | Store pagination, per-page, category, tri-state Installed button, and persistence across a re-boot, against the live registry | diff --git a/test/js/run_all.js b/test/js/run_all.js index 240b95d6..6557d089 100755 --- a/test/js/run_all.js +++ b/test/js/run_all.js @@ -21,7 +21,8 @@ const UNIT = ['unit/test_list_filter.js', 'unit/test_render_cards.js', 'unit/test_style_editor_layout_leaf_columns.js', 'unit/test_style_editor_layout_leaf_collision.js', 'unit/test_update_all.js', 'unit/test_inline_handler_escaping.js', - 'unit/test_plugin_action_delegation.js', 'unit/test_file_upload_widget.js']; + 'unit/test_plugin_action_delegation.js', 'unit/test_file_upload_widget.js', + 'unit/test_store_registry_fields.js']; const DOM = ['dom/test_installed_dom.js', 'dom/test_store_dom.js', 'dom/test_no_double_fetch.js', 'dom/test_tools_sections.js']; diff --git a/test/js/unit/test_store_registry_fields.js b/test/js/unit/test_store_registry_fields.js new file mode 100644 index 00000000..54a57ec5 --- /dev/null +++ b/test/js/unit/test_store_registry_fields.js @@ -0,0 +1,105 @@ +// The store card shows the registry fields added after 3.7.0 -- the commit +// that introduced the listed version and a warning when the plugin needs a +// newer core -- and still renders a card from an older registry that has +// neither. isStorePluginInstalled also answers to an entry's `aliases`. +// +// Rendered with the shipped functions (extracted from plugins_manager.js). + +const fs = require('fs'); +const path = require('path'); + +const SRC = fs.readFileSync( + path.resolve(__dirname, '../../../web_interface/static/v3/plugins_manager.js'), 'utf8'); + +let pass = 0, fail = 0; +const ok = (label, cond, extra) => cond + ? (pass++, console.log(' ok ' + label)) + : (fail++, console.log(' FAIL ' + label + (extra !== undefined ? ' -> ' + JSON.stringify(extra).slice(0, 400) : ''))); + +function extract(opener) { + const start = SRC.indexOf(opener); + if (start < 0) { console.error('FAIL: cannot find ' + JSON.stringify(opener)); process.exit(1); } + let depth = 0; + for (let j = SRC.indexOf('{', start); j < SRC.length; j++) { + if (SRC[j] === '{') depth++; + else if (SRC[j] === '}' && --depth === 0) return SRC.slice(start, j + 1); + } + console.error('FAIL: unbalanced braces after ' + opener); process.exit(1); +} + +class FakeEl { + constructor() { this.innerHTML = ''; this.value = ''; this.textContent = ''; } +} +class TextEl { + set textContent(v) { this._t = String(v == null ? '' : v); } + get innerHTML() { + return (this._t || '').replace(/&/g, '&').replace(//g, '>'); + } +} +const els = {}; +global.document = { + getElementById: id => (els[id] ||= new FakeEl()), + createElement: () => new TextEl(), +}; +global.window = global; +require('../led_escape').install(window); +global.pluginLog = () => {}; +global.isNewPlugin = () => false; +global.formatDate = () => ''; +global.setGridHtmlIfChanged = (container, html) => { container.innerHTML = html; }; +global.installedPlugins = []; + +// eslint-disable-next-line no-eval +eval([ + 'function escapeHtml(text) {', 'function escapeAttribute(text) {', 'function jsStringAttr(value) {', + 'function isStorePluginInstalled(pluginIdOrPlugin) {', 'function renderPluginStore(plugins) {', +].map(extract).join('\n') + '\nglobal.renderPluginStore = renderPluginStore;' + + '\nglobal.isStorePluginInstalled = isStorePluginInstalled;'); + +function render(plugin) { + renderPluginStore([plugin]); + return els['plugin-store-grid'].innerHTML; +} + +const SHA = '843588025a81197056f8d96779ccb2be19337ab8'; +const base = { + id: 'weather', name: 'Weather', author: 'ChuckBuilds', category: 'weather', + description: 'Forecasts', version: '2.1.0', + repo: 'https://github.com/ChuckBuilds/ledmatrix-plugins', plugin_path: 'plugins/ledmatrix-weather', +}; + +console.log('\n1. a registry with the new fields'); +let html = render({ ...base, commit: SHA, ledmatrix_min_version: '3.7.0', aliases: ['ledmatrix-weather'] }); +ok('shows the short commit', html.includes('>8435880<'), html.match(/v2\.1\.0[^\n]*/)); +ok('links it to the plugin at that commit', + html.includes(`href="https://github.com/ChuckBuilds/ledmatrix-plugins/tree/${SHA}/plugins/ledmatrix-weather"`)); +ok('no compatibility warning when the core is new enough', !html.includes('Needs LEDMatrix')); + +console.log('\n2. a plugin this core cannot run'); +html = render({ ...base, ledmatrix_min_version: '9.0.0', + incompatible_reason: 'Weather requires LEDMatrix 9.0.0 or newer' }); +ok('warns with the floor', html.includes('Needs LEDMatrix 9.0.0+')); +ok('and the reason as its title', html.includes('title="Weather requires LEDMatrix 9.0.0 or newer"')); + +console.log('\n3. an older registry: no commit, floor or aliases'); +html = render({ ...base }); +ok('still renders the card and its version', html.includes('class="plugin-card"') && html.includes('v2.1.0')); +ok('shows no commit and no warning', !html.includes('font-mono') && !html.includes('Needs LEDMatrix')); + +console.log('\n4. a commit value that is not a SHA is not rendered'); +html = render({ ...base, commit: 'javascript:alert(1)' }); +ok('dropped', !html.includes('javascript:') && !html.includes('font-mono')); +html = render({ ...base, commit: SHA, repo: 'javascript:alert(1)' }); +ok('without a web repo link it is plain text, not a link', + html.includes('>8435880<') && !html.includes('/tree/')); + +console.log('\n5. installed under an alias'); +global.installedPlugins = [{ id: 'ledmatrix-weather' }]; +ok('aliases count as installed', + isStorePluginInstalled({ id: 'weather', plugin_path: '', aliases: ['ledmatrix-weather'] })); +ok('plugin_path still does (older registry)', + isStorePluginInstalled({ id: 'weather', plugin_path: 'plugins/ledmatrix-weather' })); +ok('a different plugin is not installed', !isStorePluginInstalled({ id: 'stocks', plugin_path: '' })); + +console.log(`\n${pass} passed, ${fail} failed`); +process.exit(fail ? 1 : 0); diff --git a/test/test_plugin_dirs.py b/test/test_plugin_dirs.py index f80d5e5d..e942b863 100644 --- a/test/test_plugin_dirs.py +++ b/test/test_plugin_dirs.py @@ -11,7 +11,8 @@ Callers: discovery PluginManager._scan_directory_for_plugins -> plugin_directories pm_get PluginManager.get_plugin_directory before discovery has run loader PluginLoader.find_plugin_directory (no discovery mapping) - store PluginStoreManager._find_plugin_path + store PluginStoreManager._find_plugin_path (which also tries the ids the + registry proves: aliases, plugin_path name; never a bare prefix) """ import json @@ -144,7 +145,9 @@ TABLE = [ # manifest id != dir name: the manifest finds it; pm_get by prefix ("stocks", R + "ledmatrix-stocks", R + "ledmatrix-stocks", R + "ledmatrix-stocks", R + "ledmatrix-stocks"), - # ledmatrix- prefix: name-only callers with prefix=True; the store has none + # ledmatrix- prefix: name-only callers with prefix=True. The store only + # with registry proof (aliases / plugin_path), and none is loaded here; + # see test_store_registry_fields.py ("legacy", None, R + "ledmatrix-legacy", R + "ledmatrix-legacy", None), ("ledmatrix-legacy", R + "ledmatrix-legacy", R + "ledmatrix-legacy", R + "ledmatrix-legacy", R + "ledmatrix-legacy"), diff --git a/test/test_store_registry_fields.py b/test/test_store_registry_fields.py new file mode 100644 index 00000000..4ea7da0d --- /dev/null +++ b/test/test_store_registry_fields.py @@ -0,0 +1,388 @@ +"""The registry fields added after 3.7.0: ``ledmatrix_min_version``, +``aliases`` and ``commit``. + +- ``ledmatrix_min_version`` lets the store refuse an install or update this + core cannot run *before* downloading anything. The gate on the downloaded + manifest stays as the fallback. +- ``aliases`` (or, for a registry without it, the ``plugin_path`` name and the + ``ledmatrix-`` prefix) lets update, uninstall and reinstall find a plugin + installed under its manifest id: registry ``weather`` lives in + ``ledmatrix-weather/``. Before this, update by the registry id said "not + installed", uninstall by it reported success and deleted nothing, and a + reinstall backed up ``weather/`` -- which did not exist -- then deleted + ``ledmatrix-weather/`` to make room, so a post-download refusal left the user + with no plugin at all. +- ``commit`` is shown in the store; nothing installs from it. + +Every field is optional: an older plugins.json must behave exactly as before. +""" + +import json +import subprocess +from pathlib import Path +from typing import Any, Dict, List, Optional +from unittest.mock import MagicMock + +import pytest + +from src.plugin_system import compatibility +from src.plugin_system.store_manager import PluginStoreManager +from src.plugin_system.store_registry import alternate_ids, declared_aliases +from test._api_v3_test_helpers import api_v3_client, api_v3_module # noqa: F401 + +REPO = "https://github.com/ChuckBuilds/ledmatrix-plugins" +CORE = "3.7.0" + +# Shaped like the real registry before and after the monorepo change. +OLD_WEATHER = {"id": "weather", "name": "Weather", "repo": REPO, "branch": "main", + "plugin_path": "plugins/ledmatrix-weather", "latest_version": "2.0.0"} +NEW_WEATHER = {**OLD_WEATHER, "ledmatrix_min_version": "3.5.0", + "aliases": ["ledmatrix-weather"], + "commit": "843588025a81197056f8d96779ccb2be19337ab8"} + + +def manifest(version: str, floor: str = "3.0.0", plugin_id: str = "ledmatrix-weather") -> Dict[str, Any]: + return {"id": plugin_id, "name": "Weather", "class_name": "Weather", + "display_modes": ["weather"], "version": version, + "versions": [{"version": version, "ledmatrix_min_version": floor}]} + + +def write_install(plugins_dir: Path, name: str, content: Dict[str, Any], marker: str) -> Path: + path = plugins_dir / name + path.mkdir(parents=True) + (path / "manifest.json").write_text(json.dumps(content), encoding="utf-8") + (path / "marker.txt").write_text(marker, encoding="utf-8") + return path + + +def markers(plugins_dir: Path) -> Dict[str, Optional[str]]: + out = {} + for d in sorted(plugins_dir.iterdir()): + m = d / "marker.txt" + out[d.name] = m.read_text(encoding="utf-8") if m.exists() else None + return out + + +class FakeStore: + """A PluginStoreManager with the registry and the download faked.""" + + def __init__(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, + entries: List[Dict[str, Any]], downloaded: Dict[str, Any]): + self.plugins_dir = tmp_path / "plugin-repos" + self.plugins_dir.mkdir() + self.store = PluginStoreManager(plugins_dir=str(self.plugins_dir)) + self.store.logger = MagicMock() + self.downloads: List[Path] = [] + # Tests may replace registry["plugins"]; the fakes read it per call. + self.registry: Dict[str, Any] = {"plugins": entries} + store = self.store + + def fetch_registry(*_a, **_k): + store.registry_cache = self.registry + return self.registry + + def get_plugin_info(pid, **_k): + entry = store._match_registry_entry(fetch_registry()["plugins"], pid) + return dict(entry) if entry else None + + monkeypatch.setattr(store, "fetch_registry", fetch_registry) + monkeypatch.setattr(store, "get_plugin_info", get_plugin_info) + monkeypatch.setattr(store, "_install_dependencies", lambda _p: True) + + def fake_download(_url, _subpath, target: Path) -> bool: + self.downloads.append(target) + target.mkdir(parents=True, exist_ok=True) + (target / "manifest.json").write_text(json.dumps(downloaded), encoding="utf-8") + (target / "marker.txt").write_text("new", encoding="utf-8") + return True + + monkeypatch.setattr(store, "_install_from_monorepo", fake_download) + monkeypatch.setattr(compatibility, "current_core_version", lambda: CORE) + + +@pytest.fixture +def make(tmp_path, monkeypatch): + def _make(entries, downloaded=None): + return FakeStore(tmp_path, monkeypatch, entries, downloaded or manifest("2.0.0")) + return _make + + +class TestAliasHelpers: + def test_declared_aliases_distinguishes_absent_from_empty(self): + assert declared_aliases(OLD_WEATHER) is None + assert declared_aliases({**OLD_WEATHER, "aliases": []}) == [] + assert declared_aliases(NEW_WEATHER) == ["ledmatrix-weather"] + + def test_declared_aliases_drops_junk_and_the_own_id(self): + assert declared_aliases({"id": "a", "aliases": ["a", "", 3, None, "b"]}) == ["b"] + assert declared_aliases({"id": "a", "aliases": "b"}) is None + + def test_only_registry_proof_counts(self): + """aliases and the plugin_path name; never a bare ledmatrix-.""" + assert alternate_ids(OLD_WEATHER) == ["ledmatrix-weather"] + assert alternate_ids(NEW_WEATHER) == ["ledmatrix-weather"] + assert alternate_ids({"id": "clock", "plugin_path": "plugins/clock-simple"}) == ["clock-simple"] + assert alternate_ids({"id": "ext", "plugin_path": ""}) == [] + assert alternate_ids({"id": "foo", "plugin_path": "plugins/foo", "aliases": []}) == [] + assert alternate_ids({"id": "m", "plugin_path": "plugins/m", "aliases": ["x"]}) == ["x"] + + +class TestRegistryLookup: + def test_alias_resolves_an_entry_whose_path_does_not_say_so(self): + entry = {"id": "music", "plugin_path": "plugins/music", "aliases": ["ledmatrix-music"]} + assert PluginStoreManager._match_registry_entry([entry], "ledmatrix-music") is entry + + def test_exact_id_still_beats_an_alias(self): + decoy = {"id": "decoy", "aliases": ["weather"]} + real = {"id": "weather"} + assert PluginStoreManager._match_registry_entry([decoy, real], "weather") is real + + +class TestFindInstalledPlugin: + def test_registry_id_finds_the_manifest_id_install(self, make): + fake = make([NEW_WEATHER]) + path = write_install(fake.plugins_dir, "ledmatrix-weather", manifest("1.0.0"), "old") + fake.store.fetch_registry() + assert fake.store._find_plugin_path("weather") == path + + def test_old_registry_still_finds_it(self, make): + fake = make([OLD_WEATHER]) + path = write_install(fake.plugins_dir, "ledmatrix-weather", manifest("1.0.0"), "old") + fake.store.fetch_registry() + assert fake.store._find_plugin_path("weather") == path + + def test_no_registry_loaded_is_no_proof(self, make): + """Offline, a ledmatrix-weather/ folder is only named in the log.""" + fake = make([]) + path = write_install(fake.plugins_dir, "ledmatrix-weather", manifest("1.0.0"), "old") + assert fake.store.registry_cache is None + assert fake.store._find_plugin_path("weather") is None + hint = " ".join(str(a) for c in fake.store.logger.warning.call_args_list for a in c.args) + assert str(path) in hint and "ledmatrix-weather" in hint + + def test_a_folder_whose_manifest_declares_the_id_needs_no_proof(self, make): + fake = make([]) + path = write_install(fake.plugins_dir, "ledmatrix-weather", + manifest("1.0.0", plugin_id="weather"), "old") + assert fake.store._find_plugin_path("weather") == path + + def test_an_empty_aliases_list_means_no_guessing(self, make): + fake = make([{"id": "foo", "repo": REPO, "plugin_path": "plugins/foo", "aliases": []}]) + write_install(fake.plugins_dir, "ledmatrix-foo", manifest("1.0.0", plugin_id="ledmatrix-foo"), "x") + fake.store.fetch_registry() + assert fake.store._find_plugin_path("foo") is None + + def test_the_exact_install_wins_over_an_alias(self, make): + fake = make([NEW_WEATHER]) + exact = write_install(fake.plugins_dir, "weather", manifest("1.0.0", plugin_id="weather"), "a") + write_install(fake.plugins_dir, "ledmatrix-weather", manifest("1.0.0"), "b") + fake.store.fetch_registry() + assert fake.store._find_plugin_path("weather") == exact + + def test_uninstall_by_registry_id_removes_the_install(self, make): + fake = make([NEW_WEATHER]) + write_install(fake.plugins_dir, "ledmatrix-weather", manifest("1.0.0"), "old") + fake.store.fetch_registry() + assert fake.store.uninstall_plugin("weather") is True + assert markers(fake.plugins_dir) == {} + + +# An unrelated plugin that happens to live in ledmatrix-foo/ (its manifest id +# is ledmatrix-foo) and that no registry entry names. Owner decision on #686: +# without registry proof the store must never replace or remove it. +FOO = {"id": "foo", "name": "Foo", "repo": REPO, "branch": "main", "plugin_path": "plugins/foo", + "latest_version": "2.0.0"} + + +class TestUnprovenPrefixFolderIsLeftAlone: + @pytest.fixture(params=["old registry", "empty aliases", "no registry"]) + def fake(self, request, make): + entries = {"old registry": [FOO], "empty aliases": [{**FOO, "aliases": []}], + "no registry": []}[request.param] + fake = make(entries, downloaded=manifest("2.0.0", plugin_id="foo")) + write_install(fake.plugins_dir, "ledmatrix-foo", + manifest("1.0.0", plugin_id="ledmatrix-foo"), "unrelated") + return fake + + def test_uninstall_foo_leaves_it(self, fake): + if fake.registry["plugins"]: + fake.store.fetch_registry() + assert fake.store.uninstall_plugin("foo") is True # "already uninstalled" + assert markers(fake.plugins_dir) == {"ledmatrix-foo": "unrelated"} + + def test_install_foo_does_not_replace_it(self, fake): + if fake.registry["plugins"]: + fake.store.fetch_registry() + else: + # Nothing loaded when install_plugin looks for a copy to protect; + # the entry to install from is fetched afterwards. + fake.registry["plugins"] = [FOO] + assert fake.store.install_plugin("foo") is True + assert markers(fake.plugins_dir) == {"foo": "new", "ledmatrix-foo": "unrelated"} + + def test_update_foo_finds_nothing_to_update(self, fake): + if fake.registry["plugins"]: + fake.store.fetch_registry() + assert fake.store.update_plugin("foo") is False + assert markers(fake.plugins_dir) == {"ledmatrix-foo": "unrelated"} + assert fake.downloads == [] + + +class TestInstall: + def test_registry_floor_refuses_before_downloading(self, make): + fake = make([{**NEW_WEATHER, "ledmatrix_min_version": "9.0.0"}]) + write_install(fake.plugins_dir, "ledmatrix-weather", manifest("1.0.0"), "old") + assert fake.store.install_plugin("weather") is False + assert fake.downloads == [], "nothing may be downloaded for a refused install" + assert markers(fake.plugins_dir) == {"ledmatrix-weather": "old"} + reason = fake.store.pop_refusal("weather") + assert reason and "requires LEDMatrix 9.0.0 or newer" in reason and CORE in reason + assert fake.store.pop_refusal("weather") is None, "a refusal is reported once" + + def test_a_compatible_floor_installs(self, make): + fake = make([NEW_WEATHER]) + assert fake.store.install_plugin("weather") is True + assert markers(fake.plugins_dir) == {"ledmatrix-weather": "new"} + + def test_old_registry_downloads_and_the_manifest_gate_still_refuses(self, make): + """No field means no early answer: the post-download gate decides, + and the copy it would have replaced -- found under its alias -- is put + back. This used to delete ledmatrix-weather/ outright.""" + fake = make([OLD_WEATHER], downloaded=manifest("2.0.0", floor="9.0.0")) + write_install(fake.plugins_dir, "ledmatrix-weather", manifest("1.0.0"), "old") + assert fake.store.install_plugin("weather") is False + assert len(fake.downloads) == 1 + assert markers(fake.plugins_dir) == {"ledmatrix-weather": "old"} + assert "9.0.0" in (fake.store.pop_refusal("weather") or "") + + def test_old_registry_reinstall_over_an_alias_succeeds(self, make): + fake = make([OLD_WEATHER]) + write_install(fake.plugins_dir, "ledmatrix-weather", manifest("1.0.0"), "old") + assert fake.store.install_plugin("weather") is True + assert markers(fake.plugins_dir) == {"ledmatrix-weather": "new"} + + def test_another_branch_skips_the_registry_floor(self, make): + """The floor describes the registry's branch; a named other branch is + another release, left to the post-download gate.""" + fake = make([{**NEW_WEATHER, "ledmatrix_min_version": "9.0.0"}]) + assert fake.store.install_plugin("weather", branch="dev") is True + assert len(fake.downloads) == 1 + + @pytest.mark.parametrize("floor", [None, "", 3, ["9.0.0"]]) + def test_a_missing_or_malformed_floor_says_nothing(self, make, floor): + entry = {**NEW_WEATHER, "ledmatrix_min_version": floor} + assert make([entry]).store.registry_incompatibility("weather", entry) is None + + def test_an_untrustworthy_core_version_is_not_refused(self, make, monkeypatch): + """Same leniency as the manifest gate: a core reporting 1.0.0 (the + v3.1.0 release) is unknown, not old -- unless the floor is above 2.0.0, + which that release cannot meet either.""" + entry = {**NEW_WEATHER, "ledmatrix_min_version": "2.0.0"} + store = make([entry]).store + monkeypatch.setattr(compatibility, "current_core_version", lambda: "1.0.0") + assert store.registry_incompatibility("weather", entry) is None + + +class TestUpdate: + def test_update_by_registry_id_finds_and_updates_the_install(self, make): + fake = make([NEW_WEATHER]) + write_install(fake.plugins_dir, "ledmatrix-weather", manifest("1.0.0"), "old") + assert fake.store.update_plugin("weather") is True + assert markers(fake.plugins_dir) == {"ledmatrix-weather": "new"} + + def test_update_by_manifest_id_still_works(self, make): + fake = make([NEW_WEATHER]) + write_install(fake.plugins_dir, "ledmatrix-weather", manifest("1.0.0"), "old") + assert fake.store.update_plugin("ledmatrix-weather") is True + assert markers(fake.plugins_dir) == {"ledmatrix-weather": "new"} + + def test_incompatible_update_is_refused_before_anything_moves(self, make, monkeypatch): + fake = make([{**NEW_WEATHER, "ledmatrix_min_version": "9.0.0"}]) + write_install(fake.plugins_dir, "ledmatrix-weather", manifest("1.0.0"), "old") + rollback = MagicMock() + monkeypatch.setattr(fake.store, "_reinstall_with_rollback", rollback) + assert fake.store.update_plugin("ledmatrix-weather") is False + rollback.assert_not_called() + assert fake.downloads == [] + assert markers(fake.plugins_dir) == {"ledmatrix-weather": "old"} + assert "9.0.0" in (fake.store.pop_refusal("ledmatrix-weather") or "") + + def test_up_to_date_plugin_is_not_refused(self, make): + """The floor belongs to latest_version; a plugin already on it is + running, so there is nothing to refuse.""" + fake = make([{**NEW_WEATHER, "ledmatrix_min_version": "9.0.0"}]) + write_install(fake.plugins_dir, "ledmatrix-weather", manifest("2.0.0"), "current") + assert fake.store.update_plugin("weather") is True + assert fake.store.pop_refusal("weather") is None + + def test_old_registry_update_falls_back_to_the_manifest_gate(self, make): + fake = make([OLD_WEATHER], downloaded=manifest("2.0.0", floor="9.0.0")) + write_install(fake.plugins_dir, "ledmatrix-weather", manifest("1.0.0"), "old") + assert fake.store.update_plugin("weather") is False + assert len(fake.downloads) == 1 + assert markers(fake.plugins_dir) == {"ledmatrix-weather": "old"} + + def test_git_checkout_is_not_pulled_when_the_registry_refuses(self, make, monkeypatch): + fake = make([{**NEW_WEATHER, "ledmatrix_min_version": "9.0.0"}]) + write_install(fake.plugins_dir, "ledmatrix-weather", manifest("1.0.0"), "old") + monkeypatch.setattr(fake.store, "_get_local_git_info", lambda _p: { + "sha": "a" * 40, "branch": "main", "remote_url": REPO}) + calls = [] + monkeypatch.setattr(subprocess, "run", lambda *a, **k: calls.append(a) or MagicMock( + returncode=0, stdout="", stderr="")) + assert fake.store.update_plugin("weather") is False + assert not any("pull" in (a[0] if a else []) for a in calls), calls + assert "9.0.0" in (fake.store.pop_refusal("weather") or "") + + +# -- web routes -------------------------------------------------------------- + +@pytest.fixture +def web_store(api_v3_module, tmp_path, monkeypatch): + fake = FakeStore(tmp_path, monkeypatch, [], manifest("2.0.0")) + api_v3_module.api_v3.plugin_store_manager = fake.store + api_v3_module.api_v3.operation_queue = None + # No plugin manager: the routes skip discovery and reload, and the update + # route finds the directory through the store alone. + api_v3_module.api_v3.plugin_manager = None + return fake + + +def test_store_list_carries_the_new_fields(api_v3_client, web_store, monkeypatch): + listing = [ + dict(NEW_WEATHER), + {**NEW_WEATHER, "id": "later", "aliases": [], "ledmatrix_min_version": "9.0.0"}, + {"id": "old", "name": "Old", "repo": REPO, "plugin_path": "plugins/old"}, + ] + monkeypatch.setattr(web_store.store, "search_plugins", lambda **_k: listing) + resp = api_v3_client.get("/api/v3/plugins/store/list") + assert resp.status_code == 200 + by_id = {p["id"]: p for p in resp.get_json()["data"]["plugins"]} + assert by_id["weather"]["commit"] == NEW_WEATHER["commit"] + assert by_id["weather"]["ledmatrix_min_version"] == "3.5.0" + assert by_id["weather"]["aliases"] == ["ledmatrix-weather"] + assert by_id["weather"]["incompatible_reason"] is None + assert "9.0.0" in by_id["later"]["incompatible_reason"] + # An older registry: every new key present, and empty. + assert (by_id["old"]["commit"], by_id["old"]["ledmatrix_min_version"], + by_id["old"]["aliases"], by_id["old"]["incompatible_reason"]) == (None, None, [], None) + + +def test_install_route_says_why_it_refused(api_v3_client, web_store): + web_store.registry["plugins"] = [{**NEW_WEATHER, "ledmatrix_min_version": "9.0.0"}] + resp = api_v3_client.post("/api/v3/plugins/install", data=json.dumps({"plugin_id": "weather"}), + content_type="application/json") + assert resp.status_code == 409 + message = resp.get_json()["message"] + assert "requires LEDMatrix 9.0.0 or newer" in message + assert web_store.downloads == [] + + +def test_update_route_says_why_it_refused(api_v3_client, web_store): + web_store.registry["plugins"] = [{**NEW_WEATHER, "ledmatrix_min_version": "9.0.0"}] + write_install(web_store.plugins_dir, "ledmatrix-weather", manifest("1.0.0"), "old") + resp = api_v3_client.post("/api/v3/plugins/update", data=json.dumps({"plugin_id": "weather"}), + content_type="application/json") + assert resp.status_code == 409 + assert "requires LEDMatrix 9.0.0 or newer" in resp.get_json()["message"] + assert markers(web_store.plugins_dir) == {"ledmatrix-weather": "old"} diff --git a/web_interface/blueprints/api_v3/plugin_store.py b/web_interface/blueprints/api_v3/plugin_store.py index c4603258..3580097f 100644 --- a/web_interface/blueprints/api_v3/plugin_store.py +++ b/web_interface/blueprints/api_v3/plugin_store.py @@ -14,6 +14,33 @@ from src.common.path_safety import resolve_under, safe_path_component from typing import Optional +def _compatibility_refusal(plugin_id: str) -> Optional[str]: + """Why the store just refused ``plugin_id`` as incompatible, or None. + + The store records the reason under the id it was handed and, for a + reinstall, under the registry id it resolved to; both are cleared here. + """ + store = api_v3.plugin_store_manager + ids = [plugin_id] + try: + entry = store.get_registry_info(plugin_id) + except Exception: # noqa: BLE001 - only used to phrase an error + entry = None + if isinstance(entry, dict) and isinstance(entry.get('id'), str): + ids.append(entry['id']) + reason = store.pop_refusal(*ids) + return reason if isinstance(reason, str) and reason else None + + +def _store_incompatibility(plugin: dict) -> Optional[str]: + """The pre-download compatibility verdict for a store listing entry.""" + try: + reason = api_v3.plugin_store_manager.registry_incompatibility(plugin.get('id'), plugin) + except Exception: # noqa: BLE001 - a listing must not fail over a hint + return None + return reason if isinstance(reason, str) and reason else None + + def _listed_plugin_dir(base: Path, name: str) -> Optional[Path]: """The entry of ``base`` called ``name``, or None. @@ -145,7 +172,9 @@ def update_plugin(): remote_commit = remote_info.get('last_commit_sha') if remote_info else None remote_branch = remote_info.get('branch') if remote_info else None - # Update the plugin + # Update the plugin. A refusal left over from an earlier attempt + # (say, the automatic updater's) must not explain this one. + _compatibility_refusal(plugin_id) success = api_v3.plugin_store_manager.update_plugin(plugin_id) if success: @@ -235,7 +264,11 @@ def update_plugin(): message=message ) else: - if plugin_dir is None or not plugin_dir.exists(): + refusal = _compatibility_refusal(plugin_id) + if refusal: + # The plugin is untouched; say why rather than "check logs". + client_msg = f'Plugin update refused: {refusal}' + elif plugin_dir is None or not plugin_dir.exists(): client_msg = 'Plugin update failed: plugin not found' else: git_info = api_v3.plugin_store_manager._get_local_git_info(plugin_dir) @@ -264,7 +297,8 @@ def update_plugin(): return error_response( ErrorCode.PLUGIN_UPDATE_FAILED, client_msg, - status_code=500 + # A refusal is the plugin's requirement, not a server fault. + status_code=409 if refusal else 500 ) except Exception as e: @@ -406,6 +440,7 @@ def install_plugin(): if api_v3.operation_queue: def install_callback(operation): """Callback to execute plugin installation.""" + _compatibility_refusal(plugin_id) # clear any stale one success = api_v3.plugin_store_manager.install_plugin(plugin_id, branch=branch) if success: @@ -438,8 +473,10 @@ def install_plugin(): error_msg = f'Failed to install plugin {plugin_id}' if branch: error_msg += f' (branch: {branch})' - plugin_info = api_v3.plugin_store_manager.get_plugin_info(plugin_id) - if not plugin_info: + refusal = _compatibility_refusal(plugin_id) + if refusal: + error_msg += f': {refusal}' + elif not api_v3.plugin_store_manager.get_plugin_info(plugin_id): error_msg += ' (plugin not found in registry)' # Record failure in history @@ -468,6 +505,7 @@ def install_plugin(): ) else: # Fallback to direct installation + _compatibility_refusal(plugin_id) # clear any stale one success = api_v3.plugin_store_manager.install_plugin(plugin_id, branch=branch) if success: @@ -493,8 +531,10 @@ def install_plugin(): error_msg = f'Failed to install plugin {plugin_id}' if branch: error_msg += f' (branch: {branch})' - plugin_info = api_v3.plugin_store_manager.get_plugin_info(plugin_id) - if not plugin_info: + refusal = _compatibility_refusal(plugin_id) + if refusal: + error_msg += f': {refusal}' + elif not api_v3.plugin_store_manager.get_plugin_info(plugin_id): error_msg += ' (plugin not found in registry)' if api_v3.operation_history: @@ -509,7 +549,7 @@ def install_plugin(): return error_response( ErrorCode.PLUGIN_INSTALL_FAILED, error_msg, - status_code=500 + status_code=409 if refusal else 500 ) @@ -719,7 +759,17 @@ def list_plugin_store(): 'version': plugin.get('latest_version') or plugin.get('version', ''), 'branch': plugin.get('branch') or plugin.get('default_branch'), 'default_branch': plugin.get('default_branch'), - 'plugin_path': plugin.get('plugin_path', '') + 'plugin_path': plugin.get('plugin_path', ''), + # Registry fields from after 3.7.0; absent (None) in an older + # plugins.json. `commit` is the one that introduced `version`. + 'commit': plugin.get('commit') if isinstance(plugin.get('commit'), str) else None, + 'ledmatrix_min_version': (plugin.get('ledmatrix_min_version') + if isinstance(plugin.get('ledmatrix_min_version'), str) else None), + 'aliases': [a for a in plugin.get('aliases') or [] if isinstance(a, str)] + if isinstance(plugin.get('aliases'), list) else [], + # What Install would answer, without trying: the same check the + # store runs before downloading. + 'incompatible_reason': _store_incompatibility(plugin), }) return jsonify({'status': 'success', 'data': {'plugins': formatted_plugins}}) diff --git a/web_interface/static/v3/plugins_manager.js b/web_interface/static/v3/plugins_manager.js index 53000918..d2cd4745 100644 --- a/web_interface/static/v3/plugins_manager.js +++ b/web_interface/static/v3/plugins_manager.js @@ -2267,7 +2267,10 @@ function isStorePluginInstalled(pluginIdOrPlugin) { // Derive the actual installed directory name from plugin_path (e.g. "plugins/ledmatrix-weather" → "ledmatrix-weather") const pluginPath = pluginIdOrPlugin.plugin_path || ''; const pathDerivedId = pluginPath ? pluginPath.split('/').pop() : null; - return installed.some(p => p.id === storeId || (pathDerivedId && p.id === pathDerivedId)); + // Newer registries also list the other ids outright (the manifest id). + const aliases = Array.isArray(pluginIdOrPlugin.aliases) ? pluginIdOrPlugin.aliases : []; + return installed.some(p => p.id === storeId || (pathDerivedId && p.id === pathDerivedId) + || aliases.includes(p.id)); } // ── Plugin Store: search / filter / sort ──────────────────────────────── @@ -2425,6 +2428,16 @@ function renderPluginStore(plugins) { const installed = isStorePluginInstalled(plugin); // Registry data: only open real web links, never javascript: URLs. const repoLink = plugin.repo && /^https?:\/\//i.test(plugin.repo) ? plugin.repo : ''; + // The commit that introduced this version (newer registries only). + // Checked as a hex SHA before it goes anywhere near a URL. + const commit = typeof plugin.commit === 'string' && /^[0-9a-f]{7,40}$/i.test(plugin.commit) ? plugin.commit : ''; + const commitUrl = commit && repoLink + ? repoLink.replace(/\/+$/, '').replace(/\.git$/, '') + '/tree/' + commit + + (plugin.plugin_path ? '/' + plugin.plugin_path.split('/').map(encodeURIComponent).join('/') : '') + : ''; + const commitHtml = !commit ? '' : (commitUrl + ? `${escapeHtml(commit.slice(0, 7))}` + : `${escapeHtml(commit.slice(0, 7))}`); return `
@@ -2435,10 +2448,11 @@ function renderPluginStore(plugins) { ${installed ? 'Installed' : ''} ${isNewPlugin(plugin.last_updated) ? 'New' : ''} ${plugin._source === 'custom_repository' ? `Custom` : ''} + ${plugin.incompatible_reason ? `Needs LEDMatrix ${escapeHtml(plugin.ledmatrix_min_version || 'update')}+` : ''}

${escapeHtml(plugin.author || 'Unknown')}

- ${plugin.version ? `

v${escapeHtml(plugin.version)}

` : ''} + ${plugin.version ? `

v${escapeHtml(plugin.version)}${commitHtml}

` : ''}

${escapeHtml(plugin.category || 'General')}

${escapeHtml(plugin.description || 'No description available')}