diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b8aa961..d70034fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -142,6 +142,40 @@ read any of them: on the store card and linked to the plugin's source at that commit. Informational only; installs still come from the branch head. +### Changes + +- The web interface no longer loads or runs plugins (web plugin catalog, + stage 1). It built its own `PluginManager` and loaded plugins into the web + process: store installs and updates loaded or reloaded a web-side copy, and + config saves and enable/disable called `on_config_change`, `on_enable` and + `on_disable` on it. None of that reached the panel. The web process now + reads plugins as files through the new `PluginCatalog` + (`src/plugin_system/plugin_catalog.py`); only the display runs them, and + config changes reach them through its config watcher, as they already did. + - A plugin update, an install of a plugin that is already enabled, or an + uninstall that keeps an enabled plugin's config now answers + `restart_required: true` and shows the restart banner, because the + running display keeps the code it loaded until it restarts. Before, the + update looked applied and the panel kept the old version. + - The restart banner follows `restart_required` in any response + (`POST /api/v3/config/main` sends it) rather than the URL that was + called. + - `/api/v3/plugins/installed` reports `loaded`, `state` and `error_info` + as `null`: the display does not publish them, and the old values + described web-side copies. `enabled` follows the display's rule, so a + plugin whose config has no `enabled` flag shows as disabled (it never + ran). `vegas_mode` is the configured value only. + - `vegas_participation` there is the user's setting, else the manifest's + declaration, with a new `vegas_participation_source` (`config` or + `manifest`). When only the plugin's code decides it (a + `get_vegas_participation()` override or the legacy Vegas hooks) it is + `null` with source `runtime`: the display derives it, and the web no + longer asks a web-side plugin instance. + - Starlark routes always use their on-disk path. The one place the web + process still imports plugin code -- the Starlark helper modules and an + `oauth_flow` action script -- is `_import_plugin_code_in_web_process()`, + until a plugin web-entry contract replaces it. + ### Fixes - Reinstalling a plugin by its registry id when it is installed under its diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 78e990d1..82ae99e7 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -57,6 +57,61 @@ The on-demand start route starts `ledmatrix.service` when it is not running reads the mailbox every `ON_DEMAND_POLL_INTERVAL` (0.25s), from its dwell sleep, its render loops and Vegas's interrupt check as well as the main loop. +### Web and display processes: who runs plugins + +Only the display process imports plugin code, instantiates plugins and calls +their lifecycle hooks (`update`, `display`, `on_config_change`, `on_enable`, +`on_disable`). The web process is metadata-only: it reads plugins as files +through `PluginCatalog` +([`src/plugin_system/plugin_catalog.py`](../src/plugin_system/plugin_catalog.py)) +-- manifests, config schemas (through `SchemaManager`), each plugin's +section of `config.json`, and installed versions. The catalog keeps the +read-only method names of `PluginManager` and has nothing that can run a +plugin (no `load_plugin`, `get_plugin` or `plugins`). + +How a web-side change reaches the running plugins: + +| Change | How the display picks it up | +|---|---| +| Plugin settings saved, config reset | `ConfigService` sees the new `config.json` and calls the plugin's `on_config_change` with the prepared section | +| Plugin enabled or disabled | `ConfigService` → `_controller_config_change` flags a reconcile; `_reconcile_enabled_plugins` loads it (fresh from disk) or unloads it on the render thread | +| Plugin uninstalled (config removed) | the removed section flips its `enabled` flag, and the reconcile unloads it | +| Plugin installed, not enabled | nothing to do until it is enabled, which loads it | +| Plugin installed while already enabled, updated while enabled, or uninstalled with its config kept | **not picked up**: the display keeps running what it loaded. The route answers `restart_required: true` and the UI shows its restart banner | + +`display_restart_required()` in `plugin_catalog.py` holds that last rule; +routes return it as `restart_required` (with the banner's wording in +`restart_message`), and `window.noteRestartRequired()` in +`static/v3/app.js` raises the banner for any response that carries it, +`POST /api/v3/config/main` included. + +Runtime state shown in the UI comes from what the display publishes (the +table above): health and metrics (`/api/v3/plugins/health`, +`/plugins/metrics`), errors (`/api/v3/errors/*`) and the current mode. The +display does not publish which plugins it has loaded or its plugin state +machine, so `/api/v3/plugins/installed` reports `loaded`, `state` and +`error_info` as `null` rather than guessing; `enabled` is read from +`config.json` by the display's rule (a missing flag is disabled). + +Plugin code still runs in the web process in one place, +`_import_plugin_code_in_web_process()` in +[`api_v3/__init__.py`](../web_interface/blueprints/api_v3/__init__.py): the +Starlark routes import the starlark-apps plugin's `tronbyte_repository` and +`pixlet_renderer` helper modules (never the plugin class), and a web-UI +action with `oauth_flow` imports its script for `get_auth_url()`. Every +other web-UI action runs its script as a subprocess. A later, explicit +**plugin web-entry contract** -- a declared entry point for plugin web code +-- replaces that function. + +Remaining plugin state outside the display: `data/plugin_state.json` +(`PluginStateManager` in `state_manager.py`, written by the web process), +a second class also named `PluginStateManager` in `plugin_state.py` (the +display's in-memory state machine), and `state_reconciliation.py`, which +compares config, disk and `plugin_state.json` at web startup. These are the +next stages: retire `plugin_state.json`, merge the two state classes, and +give the web process a control socket to the display (reload one plugin, +report the loaded set) in place of `restart_required`. + ## Display loop [`src/display_controller.py`](../src/display_controller.py), class @@ -128,7 +183,8 @@ then normal rotation. |---|---| | Base class plugins implement | [`base_plugin.py`](../src/plugin_system/base_plugin.py) (`BasePlugin`, `VegasDisplayMode`) | | Finding a plugin's directory | [`plugin_dirs.py`](../src/plugin_system/plugin_dirs.py): manifest `id` first, then directory `` or `ledmatrix-` | -| Discovery, load, unload, scheduled updates | [`plugin_manager.py`](../src/plugin_system/plugin_manager.py) (`PluginManager`) | +| Discovery, load, unload, scheduled updates (display process) | [`plugin_manager.py`](../src/plugin_system/plugin_manager.py) (`PluginManager`) | +| Manifest, schema, config and version reads (web process) | [`plugin_catalog.py`](../src/plugin_system/plugin_catalog.py) (`PluginCatalog`; see [who runs plugins](#web-and-display-processes-who-runs-plugins)) | | Import and instantiate | [`plugin_loader.py`](../src/plugin_system/plugin_loader.py) (`PluginLoader.load_plugin()`: dependencies, module, class) | | Timeouts | [`plugin_executor.py`](../src/plugin_system/plugin_executor.py) (`PluginExecutor`, 30 s default; a timed-out thread is abandoned, not killed) | | Circuit breaker | [`plugin_health.py`](../src/plugin_system/plugin_health.py) (`PluginHealthTracker`: 3 consecutive failures open the circuit for 300 s) | @@ -155,8 +211,9 @@ everything else through `_reinstall_with_rollback()`. ## Web interface - **App.** [`web_interface/app.py`](../web_interface/app.py) builds the - Flask `app` at import time, creates the managers, and registers two - blueprints. `web_interface/start.py` runs it on port 5000. + Flask `app` at import time, creates the managers -- a `PluginCatalog`, + never a `PluginManager` -- and registers two blueprints. + `web_interface/start.py` runs it on port 5000. - **Pages.** [`blueprints/pages_v3.py`](../web_interface/blueprints/pages_v3.py) serves the shell `templates/v3/base.html` at `/` and each tab as a partial at `/partials/` (templates in diff --git a/docs/PLUGIN_API_REFERENCE.md b/docs/PLUGIN_API_REFERENCE.md index a663cdfb..9cfd4a9c 100644 --- a/docs/PLUGIN_API_REFERENCE.md +++ b/docs/PLUGIN_API_REFERENCE.md @@ -149,7 +149,11 @@ Clean up resources when plugin is unloaded. Override to close connections, stop #### `on_config_change(new_config: Dict[str, Any]) -> None` -Called after plugin configuration is updated via web API. +Called after the plugin's section of `config.json` changes -- a save in the +web UI, say. Every lifecycle hook runs in the display process, which is the +only process that runs plugins: the web interface writes `config.json`, and +the display's config watcher calls this with the prepared section. See +[ARCHITECTURE.md](ARCHITECTURE.md#web-and-display-processes-who-runs-plugins). In the display service it runs on the config watcher thread while holding the plugin's lock, so it never overlaps your `update()` or `display()`. If @@ -159,11 +163,13 @@ from the update thread: as soon as the plugin is free, and before its next #### `on_enable() -> None` -Called when plugin is enabled. +Called when the display loads the plugin enabled: at startup, or when it is +switched on in the web UI. #### `on_disable() -> None` -Called when plugin is disabled. +Called when the display unloads the plugin, e.g. when it is switched off in +the web UI. #### `get_update_interval() -> Optional[float]` diff --git a/docs/REST_API_REFERENCE.md b/docs/REST_API_REFERENCE.md index 243d4d88..541dde50 100644 --- a/docs/REST_API_REFERENCE.md +++ b/docs/REST_API_REFERENCE.md @@ -154,10 +154,17 @@ there an unchecked checkbox — which the browser omits — is saved as ```json { "status": "success", - "message": "Configuration saved successfully" + "message": "Configuration saved successfully", + "restart_required": true } ``` +`restart_required` is always true here: display hardware, rotation, +durations and general settings take effect when the display restarts, and +the web UI shows its restart banner on the flag. (Plugin sections saved +through this route reach the running plugin live, like +`POST /plugins/config`.) + Invalid values (e.g. an out-of-range `target_fps`, a hardware option the Raspberry Pi 5 driver cannot use) are rejected with `400` and nothing is saved. @@ -502,8 +509,8 @@ List all installed plugins with their status and metadata. "tags": ["sports", "football", "nfl"], "enabled": true, "verified": true, - "loaded": true, - "state": "loaded", + "loaded": null, + "state": null, "error_info": null, "last_updated": "2025-01-15T10:30:00Z", "last_commit": "abc1234", @@ -512,19 +519,34 @@ List all installed plugins with their status and metadata. "web_ui_actions": [], "vegas_mode": null, "vegas_content_type": null, - "vegas_participation": "scroll" + "vegas_participation": "scroll", + "vegas_participation_source": "manifest" } ] } } ``` +Metadata comes from each plugin's files on disk; `enabled` is the plugin's +`enabled` flag in `config.json` (missing means disabled, as the display +reads it). `loaded`, `state` and `error_info` are always `null`: the web +process runs no plugin code, and the display does not publish which plugins +it has loaded. What the display does publish is at +[`/plugins/health`](#get-plugin-health), `/plugins/metrics` and `/errors/*`. + `vegas_participation` is what Vegas mode does with the plugin: `"scroll"`, `"pause"` or `"exclude"` (see -[PLUGIN_API_REFERENCE.md](PLUGIN_API_REFERENCE.md#vegas-participation)). -For a plugin that is not loaded it is only the user's own -`vegas_participation` setting, or `null`. `vegas_mode` and -`vegas_content_type` are the legacy hooks' raw answers. +[PLUGIN_API_REFERENCE.md](PLUGIN_API_REFERENCE.md#vegas-participation)), +and `vegas_participation_source` says where it came from. The web reads it +the way the display resolves it, as far as files can tell: the user's own +`vegas_participation` setting (`"config"`), else the manifest's declared +`vegas_participation` (`"manifest"`). Past those the display derives it from +the plugin's code -- a `get_vegas_participation()` override or the legacy +Vegas hooks -- which the web process never runs, so `vegas_participation` +is `null` and the source is `"runtime"`. A plugin that overrides +`get_vegas_participation()` decides at run time and can differ from its +manifest's declaration. `vegas_mode` is the plugin's configured +`vegas_mode`, or `null`; `vegas_content_type` is always `null`. ### Get Plugin Configuration @@ -685,7 +707,13 @@ Install a plugin from the plugin store. ``` When the operation queue is unavailable the install runs synchronously and -the response has only a `message`. +the response has only a `message` and the restart fields below. + +The finished operation's `result` (from `/plugins/operation/`) +carries `restart_required`: true when the plugin is already enabled in +`config.json`, because the running display does not load newly installed +files by itself; `restart_message` then holds the restart banner's wording. +A plugin that is not enabled needs no restart: enabling it loads it. A plugin whose registry entry (or downloaded manifest) needs a newer LEDMatrix is refused: the synchronous install answers `409` with a message @@ -716,6 +744,11 @@ Remove an installed plugin. } ``` +The finished operation's `result` carries `restart_required`. Removing the +plugin's config (the default) lets the display unload it by itself, so it is +false; with `preserve_config: true` an enabled plugin keeps running until +the display restarts, and it is true. + ### Update Plugin **POST** `/api/v3/plugins/update` @@ -736,11 +769,18 @@ Update a plugin to the latest version. Runs synchronously. "message": "Plugin football-scoreboard updated ...", "data": { "last_updated": "2025-01-15T10:30:00Z", - "commit": "abc1234..." - } + "commit": "abc1234...", + "update_status": "updated" + }, + "restart_required": true, + "restart_message": "Plugin updated — restart the display to run the new version" } ``` +`update_status` is `updated`, `up_to_date` or `local_only`. +`restart_required` is true when the plugin changed and is enabled: the +running display keeps the code it loaded until it restarts. + An update this core cannot run answers `409` with `Plugin update refused:` and the reason; the installed version is left as it was. @@ -772,10 +812,13 @@ Install a plugin directly from a GitHub repository URL. Runs synchronously. "message": "Plugin my-plugin installed successfully", "plugin_id": "my-plugin", "name": "My Plugin", - "branch": "main" + "branch": "main", + "restart_required": false } ``` +`restart_required` follows the same rule as `/plugins/install`. + ### Load Registry from URL **POST** `/api/v3/plugins/registry-from-url` diff --git a/mypy-clean.txt b/mypy-clean.txt index bd7330c4..2e969bae 100644 --- a/mypy-clean.txt +++ b/mypy-clean.txt @@ -54,6 +54,7 @@ src/plugin_system/compatibility.py src/plugin_system/operation_history.py src/plugin_system/operation_queue.py src/plugin_system/operation_types.py +src/plugin_system/plugin_catalog.py src/plugin_system/plugin_dirs.py src/plugin_system/plugin_executor.py src/plugin_system/plugin_health.py diff --git a/src/plugin_system/plugin_catalog.py b/src/plugin_system/plugin_catalog.py new file mode 100644 index 00000000..ddd6f231 --- /dev/null +++ b/src/plugin_system/plugin_catalog.py @@ -0,0 +1,256 @@ +""" +Plugin catalog: what the web process knows about installed plugins. + +The web interface and the display run as two processes. Only the display +imports plugin code and runs it; the web process reads plugins as files -- +manifest, config schema, the plugin's section of config.json, the installed +version -- and never imports a plugin module, instantiates a plugin class or +calls a plugin lifecycle hook. This class is that read side. + +It keeps the method names of the read-only part of :class:`PluginManager` +(``discover_plugins``, ``plugin_manifests``, ``get_plugin_info``, +``get_plugin_directory``, ``get_plugin_display_modes``, +``find_plugin_for_mode``), so code that only ever read through a manager +reads through a catalog unchanged. It has nothing that runs a plugin: no +``load_plugin``, ``get_plugin`` or ``plugins``. + +Runtime state -- whether the display has a plugin loaded, its health, its +errors -- is not here either. The display process publishes what it knows to +the shared cache (health and resource metrics, the current mode, the error +aggregator snapshot), and the web routes read those publications. What the +display does not publish (which plugins it has loaded, its plugin state +machine) the web cannot know, and reports as unknown. + +See docs/ARCHITECTURE.md ("Web and display processes"). +""" + +import json +import threading +from pathlib import Path +from typing import Any, Dict, List, Optional, Union, cast + +from src.common.permission_utils import ( + ensure_directory_permissions, get_plugin_dir_mode, +) +from src.logging_config import get_logger +from src.plugin_system.plugin_dirs import ( + ManifestStatus, PluginDirectoryIndex, resolve_plugin_dir, +) + +PathLike = Union[str, Path] + + +class PluginCatalog: + """Manifests, schemas, config and versions of the installed plugins. + + Discovery is explicit and cheap to repeat: :meth:`discover_plugins` + rescans the plugins directory and replaces the manifest map, so an + uninstalled plugin disappears and a new one appears. + """ + + def __init__(self, plugins_dir: PathLike, config_manager: Optional[Any] = None, + schema_manager: Optional[Any] = None) -> None: + self.plugins_dir: Path = Path(plugins_dir) + self.config_manager = config_manager + self.schema_manager = schema_manager + self.logger = get_logger(__name__) + + # Guards plugin_manifests/plugin_directories: request threads read + # them while another request (or startup reconciliation) rescans. + self._lock = threading.RLock() + self.plugin_manifests: Dict[str, Dict[str, Any]] = {} + self.plugin_directories: Dict[str, Path] = {} + self._skip_reported: set = set() + + # The Plugin Store installs into this directory, so it has to exist. + # The display service logs its own error if it cannot use it; the web + # interface stays up either way. + try: + ensure_directory_permissions(self.plugins_dir, get_plugin_dir_mode()) + except OSError as exc: + self.logger.warning("Could not create plugins directory %s: %s", + self.plugins_dir, exc) + + # -- discovery -------------------------------------------------------- + + def discover_plugins(self) -> List[str]: + """Rescan the plugins directory; return the discovered plugin ids. + + The rules for what counts as a plugin and which directory wins for a + duplicated id are :class:`PluginDirectoryIndex`'s, the same ones the + display process loads by. Only the configured directory is scanned. + """ + index = PluginDirectoryIndex.scan(self.plugins_dir) + if index.error is not None: + self.logger.error("Error scanning plugins directory %s: %s", + self.plugins_dir, index.error) + for entry in index.entries: + if entry.status in (ManifestStatus.UNREADABLE, ManifestStatus.NOT_OBJECT, + ManifestStatus.NO_ID): + # The display logs these at load time; once per process is + # enough here, since discovery runs on page loads. + if entry.name not in self._skip_reported: + self._skip_reported.add(entry.name) + self.logger.info("Not listing %s: its manifest.json is unusable (%s)", + entry.name, entry.status) + + plugins = index.plugins() + manifests = {pid: entry.manifest for pid, entry in plugins.items()} + directories = {pid: entry.path for pid, entry in plugins.items()} + with self._lock: + self.plugin_manifests.clear() + self.plugin_manifests.update(manifests) + self.plugin_directories.clear() + self.plugin_directories.update(directories) + return list(plugins) + + def discovered_plugin_ids(self) -> set: + """Snapshot of the discovered ids, taken under the lock.""" + with self._lock: + return set(self.plugin_manifests) + + # -- manifests -------------------------------------------------------- + + def get_manifest(self, plugin_id: str) -> Optional[Dict[str, Any]]: + """A copy of the manifest discovery read for ``plugin_id``, or None.""" + with self._lock: + manifest = self.plugin_manifests.get(plugin_id) + return dict(manifest) if manifest else None + + def get_plugin_info(self, plugin_id: str) -> Optional[Dict[str, Any]]: + """The plugin's manifest, as a new dict -- metadata only. + + Unlike ``PluginManager.get_plugin_info`` there are no ``loaded``, + ``runtime_info`` or ``state`` keys: those described plugin instances + in this process, which no longer exist. + """ + return self.get_manifest(plugin_id) + + def get_all_plugin_info(self) -> List[Dict[str, Any]]: + """:meth:`get_plugin_info` for every discovered plugin.""" + with self._lock: + ids = list(self.plugin_manifests) + return [info for info in (self.get_plugin_info(pid) for pid in ids) if info] + + def read_manifest(self, plugin_id: str) -> Optional[Dict[str, Any]]: + """The manifest as it is on disk now, not as discovery last saw it. + + For reads that must reflect a change made since the last scan -- the + version just after an update, say. None when the plugin has no + directory or its manifest is missing, unreadable or not an object. + """ + plugin_dir = self.get_plugin_directory(plugin_id) + if plugin_dir is None: + return None + try: + with open(Path(plugin_dir) / 'manifest.json', 'r', encoding='utf-8') as f: + manifest = json.load(f) + except (OSError, ValueError) as exc: + self.logger.debug("Could not read manifest for %s: %s", plugin_id, exc) + return None + return manifest if isinstance(manifest, dict) else None + + def get_installed_version(self, plugin_id: str) -> str: + """The installed version from the on-disk manifest, or ''.""" + manifest = self.read_manifest(plugin_id) or {} + version = manifest.get('version', '') + return version if isinstance(version, str) else str(version) + + def get_plugin_directory(self, plugin_id: str) -> Optional[str]: + """Where ``plugin_id`` is installed, or None. + + Same rules as ``PluginManager.get_plugin_directory``: the discovered + directory, else ```` then ``ledmatrix-`` by name within the + plugins directory. An id that is not one plain path segment is + refused rather than joined onto the plugins directory. + """ + with self._lock: + if plugin_id in self.plugin_directories: + return str(self.plugin_directories[plugin_id]) + plugin_dir = resolve_plugin_dir( + plugin_id, [self.plugins_dir], prefix=True, case_insensitive=False, + by_manifest=False) + return str(plugin_dir) if plugin_dir is not None else None + + def get_plugin_display_modes(self, plugin_id: str) -> List[str]: + """The manifest's ``display_modes``, or []. + + What the display actually rotates can differ: a plugin may compute + its modes at run time (``plugin.modes``). This is the declared list. + """ + with self._lock: + manifest = self.plugin_manifests.get(plugin_id) + modes = (manifest or {}).get('display_modes', []) + return list(modes) if isinstance(modes, list) else [] + + def find_plugin_for_mode(self, mode: str) -> Optional[str]: + """The plugin whose manifest declares ``mode`` (case-insensitive).""" + wanted = mode.strip().lower() + with self._lock: + manifests = dict(self.plugin_manifests) + for plugin_id, manifest in manifests.items(): + modes = manifest.get('display_modes') + if isinstance(modes, list) and any( + isinstance(m, str) and m.lower() == wanted for m in modes): + return plugin_id + return None + + # -- schema and config ------------------------------------------------ + + def get_schema(self, plugin_id: str, use_cache: bool = True) -> Optional[Dict[str, Any]]: + """The plugin's config schema through SchemaManager, or None.""" + if self.schema_manager is None: + return None + schema = self.schema_manager.load_schema(plugin_id, use_cache=use_cache) + return cast(Optional[Dict[str, Any]], schema) + + def get_config(self, plugin_id: str) -> Dict[str, Any]: + """The plugin's section of config.json (secrets merged), or {}.""" + if self.config_manager is None: + return {} + section = (self.config_manager.load_config() or {}).get(plugin_id) + return section if isinstance(section, dict) else {} + + def is_enabled(self, plugin_id: str) -> bool: + """Whether config.json enables the plugin, by the display's rule. + + The display loads a plugin only when its section says + ``"enabled": true``; a missing flag or section means disabled + (``DisplayController._reconcile_enabled_plugins``). + """ + return bool(self.get_config(plugin_id).get('enabled', False)) + + +def display_restart_required(action: str, plugin_enabled: bool, *, + changed: bool = True, + preserve_config: bool = False) -> bool: + """Whether a store operation needs a display restart to reach the panel. + + The display loads and unloads plugins live only through its config + watcher: when a plugin's ``enabled`` flag changes it reconciles the + running set (``DisplayController._reconcile_enabled_plugins``), and + loading reads the plugin fresh from disk. Nothing makes it reload a + plugin it is already running, and nothing tells it about files changing + under a plugin whose flag did not move. So: + + - ``install``: a plugin that is not enabled needs nothing -- enabling it + later loads it. One already enabled in config (a reinstall, or a + config carried over) is not picked up until a restart. + - ``update``: the display keeps running the code it loaded until it + restarts, if it runs the plugin at all -- only when it is enabled. + ``changed=False`` (already up to date) needs nothing. + - ``uninstall``: removing the plugin's config section flips its enabled + flag, and the reconcile unloads it. With ``preserve_config`` the flag + stays, and an enabled plugin keeps running until a restart. + + ``plugin_enabled`` is the config flag as it was before the operation. + """ + if not plugin_enabled: + return False + if action == 'install': + return True + if action == 'update': + return changed + if action == 'uninstall': + return preserve_config + raise ValueError(f"unknown store action: {action!r}") diff --git a/src/web_interface/api_helpers.py b/src/web_interface/api_helpers.py index 2008befb..b51d2b78 100644 --- a/src/web_interface/api_helpers.py +++ b/src/web_interface/api_helpers.py @@ -15,7 +15,8 @@ from src.web_interface.errors import ErrorCode, WebInterfaceError def success_response( data: Any = None, message: Optional[str] = None, - metadata: Optional[Dict] = None + metadata: Optional[Dict] = None, + extra: Optional[Dict[str, Any]] = None ): """ Create a standardized success response. @@ -24,11 +25,15 @@ def success_response( data: Response data message: Optional success message metadata: Optional metadata (timing, version, etc.) + extra: Optional top-level fields beside ``status``/``data``, such as + ``restart_required``; they cannot replace the standard keys Returns: Flask jsonify response """ response_data = create_success_response(data, message, metadata) + for key, value in (extra or {}).items(): + response_data.setdefault(key, value) # Timing is merged into whatever the caller passed, without inventing a # metadata block for responses that have neither. diff --git a/test/_api_v3_test_helpers.py b/test/_api_v3_test_helpers.py index 9bcb7984..02c1c563 100644 --- a/test/_api_v3_test_helpers.py +++ b/test/_api_v3_test_helpers.py @@ -21,14 +21,32 @@ from flask import Flask # Every manager attribute the blueprint reads. Anything missing here keeps # whatever a previously-run test left on the singleton. API_V3_MANAGER_ATTRS = ( - 'config_manager', 'plugin_manager', 'plugin_store_manager', + 'config_manager', 'plugin_catalog', 'plugin_store_manager', 'plugin_state_manager', 'saved_repositories_manager', 'schema_manager', 'operation_queue', 'operation_history', 'cache_manager', + 'health_tracker', 'resource_monitor', ) _SENTINEL = object() +def mock_plugin_catalog(): + """A MagicMock shaped like PluginCatalog, and only like it. + + ``spec`` makes anything a PluginCatalog lacks raise AttributeError -- + ``get_plugin``, ``load_plugin``, ``plugins`` -- so a route that reached + for a plugin instance in the web process fails the test that drives it + instead of quietly calling a mock. The instance attributes the spec + cannot see are set explicitly. + """ + from src.plugin_system.plugin_catalog import PluginCatalog + catalog = MagicMock(spec=PluginCatalog) + for name in ('plugins_dir', 'config_manager', 'schema_manager', + 'plugin_manifests', 'plugin_directories'): + setattr(catalog, name, MagicMock()) + return catalog + + def build_app(blueprint): app = Flask(__name__) app.config['TESTING'] = True @@ -52,7 +70,8 @@ def api_v3_module(): for name in API_V3_MANAGER_ATTRS } for name in API_V3_MANAGER_ATTRS: - setattr(module.api_v3, name, MagicMock()) + setattr(module.api_v3, name, + mock_plugin_catalog() if name == 'plugin_catalog' else MagicMock()) # Default to the direct path; queue tests opt in explicitly. module.api_v3.operation_queue = None diff --git a/test/js/run_all.js b/test/js/run_all.js index 6557d089..79543649 100755 --- a/test/js/run_all.js +++ b/test/js/run_all.js @@ -22,7 +22,7 @@ const UNIT = ['unit/test_list_filter.js', 'unit/test_render_cards.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_store_registry_fields.js']; + 'unit/test_store_registry_fields.js', 'unit/test_restart_banner.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_restart_banner.js b/test/js/unit/test_restart_banner.js new file mode 100644 index 00000000..454edfbb --- /dev/null +++ b/test/js/unit/test_restart_banner.js @@ -0,0 +1,104 @@ +// The "restart the display" banner is raised by the server's answer, not by +// which URL was called. +// +// app.js used to show it after any successful POST to /api/v3/config/main and +// nothing else. A plugin update, install or uninstall that the running display +// cannot pick up live now answers `restart_required: true` (with the banner's +// wording in `restart_message`), and so does a main-config save. The htmx +// after-request handler and window.noteRestartRequired must both follow the +// flag. Runs the shipped app.js in a vm with a minimal fake DOM -- no jsdom and +// no server needed, so it runs under test/test_js_unit_suites.py too. + +const fs = require('fs'); +const path = require('path'); +const vm = require('vm'); +const V3 = path.resolve(__dirname, '../../../web_interface/static/v3'); + +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) : ''))); + +function load() { + const handlers = {}; + const listen = (target) => (type, fn) => { (handlers[target + ':' + type] ||= []).push(fn); }; + const banner = { style: { display: 'none' } }; + const text = { dataset: {}, textContent: ' Configuration saved — restart the display to apply the changes ' }; + const store = {}; + const document = { + body: { addEventListener: listen('body') }, + addEventListener: listen('document'), + getElementById: (id) => ({ 'restart-pending-banner': banner, 'restart-pending-text': text })[id] || null, + querySelector: () => null, + querySelectorAll: () => [], + }; + const window = { + addEventListener: listen('window'), + getApp: () => null, + }; + const context = { + window, document, console, + sessionStorage: { + setItem: (k, v) => { store[k] = String(v); }, + removeItem: (k) => { delete store[k]; }, + getItem: (k) => (k in store ? store[k] : null), + }, + showNotification: () => {}, + setTimeout: () => 0, + }; + vm.createContext(context); + vm.runInContext(fs.readFileSync(path.join(V3, 'app.js'), 'utf8'), context); + const afterRequest = (handlers['body:htmx:afterRequest'] || [])[0]; + const fire = ({ status = 200, body, path: reqPath = '/api/v3/anything', reportsItself = false }) => { + const elt = { closest: () => (reportsItself ? {} : null) }; + afterRequest({ + target: { closest: () => null }, + detail: { + xhr: { status, responseText: body === undefined ? '' : (typeof body === 'string' ? body : JSON.stringify(body)) }, + elt, + requestConfig: { verb: 'post', path: reqPath }, + }, + }); + }; + return { window, banner, text, store, fire, afterRequest }; +} + +console.log('\nwindow.noteRestartRequired'); +{ + const t = load(); + ok('app.js defines it', typeof t.window.noteRestartRequired === 'function'); + ok('no flag, no banner', t.window.noteRestartRequired({ status: 'success' }) === false + && t.banner.style.display === 'none'); + ok('a missing body is ignored', t.window.noteRestartRequired(null) === false); + ok('restart_required: false is not a request', + t.window.noteRestartRequired({ restart_required: false }) === false && t.banner.style.display === 'none'); + ok('restart_required: true shows the banner', + t.window.noteRestartRequired({ restart_required: true }) === true && t.banner.style.display === 'block'); + ok('without a message the template wording stays', + t.text.textContent === 'Configuration saved — restart the display to apply the changes', t.text.textContent); + t.window.noteRestartRequired({ restart_required: true, restart_message: 'Plugin updated — restart the display to run the new version' }); + ok('restart_message becomes the wording', + t.text.textContent === 'Plugin updated — restart the display to run the new version', t.text.textContent); + ok('and survives a reload with the flag', t.store['ledmatrix-restart-pending'] === '1' + && t.store['ledmatrix-restart-pending-text'] === 'Plugin updated — restart the display to run the new version'); +} + +console.log('\nhtmx after-request follows the flag, not the URL'); +{ + const t = load(); + ok('the handler is registered', typeof t.afterRequest === 'function'); + t.fire({ path: '/api/v3/config/main', body: { status: 'success', message: 'Configuration saved successfully' } }); + ok('a /config/main answer without the flag raises nothing', t.banner.style.display === 'none'); + t.fire({ path: '/api/v3/plugins/update', status: 500, body: { status: 'error', restart_required: true } }); + ok('an error answer never raises it', t.banner.style.display === 'none'); + t.fire({ path: '/api/v3/whatever', body: 'not json' }); + ok('a non-JSON answer is ignored', t.banner.style.display === 'none'); + // The main-config forms report their own result (hx-on after-request), so + // the flag must be read even when the toast is not this handler's to show. + t.fire({ path: '/api/v3/config/main', reportsItself: true, + body: { status: 'success', message: 'Configuration saved successfully', restart_required: true } }); + ok('a flagged answer raises it, even from a form that reports itself', t.banner.style.display === 'block'); +} + +console.log(`\n${pass} passed, ${fail} failed`); +process.exit(fail ? 1 : 0); diff --git a/test/js/unit/test_update_all.js b/test/js/unit/test_update_all.js index 8fb9bf64..ef0cf610 100644 --- a/test/js/unit/test_update_all.js +++ b/test/js/unit/test_update_all.js @@ -229,6 +229,78 @@ const noSleep = { sleep: async () => {} }; allNoop.type === 'success' && allNoop.text === '2 already up to date', allNoop); } + console.log('\nrestart banner: driven by the server\'s restart_required'); + { + const body = (restart_required, restart_message) => ({ + success: true, + result: { status: 'success', data: { update_status: 'updated' }, restart_required, restart_message }, + }); + const needed = body(true, 'Plugin updated — restart the display to run the new version'); + ok('an update the display is running asks for the banner, with its wording', + Manager.restartRequest([body(false), needed, body(true, 'second')]) === needed.result); + ok('updates the display does not run need no restart', + Manager.restartRequest([body(false), body(false)]) === null); + ok('a failed request never raises the banner', + Manager.restartRequest([{ success: false, error: { restart_required: true } }]) === null); + ok('an older server that sends no flag raises nothing', + Manager.restartRequest([{ success: true, result: { status: 'success' } }]) === null); + ok('no results, no banner', Manager.restartRequest(undefined) === null); + + // The first request's answer was lost; the re-sent one finds nothing to do. + const lost = (enabled, update_status = 'up_to_date') => ({ + pluginId: 'clock', success: true, afterLostAnswer: true, enabled, + result: { status: 'success', data: { update_status }, restart_required: false }, + }); + const maybe = Manager.restartRequest([body(false), lost(true)]); + ok('an enabled plugin up to date after a lost answer may have been updated: banner', + maybe && maybe.restart_required === true && /clock/.test(maybe.restart_message), maybe); + ok('...but an explicit answer still wins, with its wording', + Manager.restartRequest([lost(true), needed]) === needed.result); + ok('a disabled one needs no restart (enabling it loads it)', + Manager.restartRequest([lost(false)]) === null); + ok('nor does one that was not retried', + Manager.restartRequest([{ ...lost(true), afterLostAnswer: undefined }]) === null); + } + + console.log('\nupdateAll keeps what the banner needs'); + { + // ledmatrix-flights (enabled) loses its first answer, then is up to date. + const api = fakeApi({ + 'ledmatrix-flights': (n) => { + if (n === 1) throw netErr(); + return { status: 'success', data: { update_status: 'up_to_date' }, restart_required: false }; + }, + }); + setup(api, { windowList: INSTALLED }); + const results = await Manager.updateAll(null, noSleep); + const flights = results.find(r => r.pluginId === 'ledmatrix-flights'); + ok('a retried entry is marked, with the plugin\'s enabled flag', + flights.afterLostAnswer === true && flights.enabled === true, flights); + ok('an entry answered first time is not marked', + results.filter(r => r.afterLostAnswer).length === 1, results); + ok('...so the run asks for a restart', Manager.restartRequest(results) !== null); + } + { + const answer = { status: 'success', data: { update_status: 'updated' }, restart_required: true }; + const api = fakeApi({ 'ledmatrix-flights': () => answer }); + setup(api, { stateList: INSTALLED }); + window.PluginStateManager.loadInstalledPlugins = async () => { throw new Error('refresh failed'); }; + const warn = console.warn; + console.warn = () => {}; + let results; + try { + results = await Manager.updateAll(null, noSleep); + } catch (e) { + results = e; + } finally { + console.warn = warn; + } + ok('a failed list refresh still returns the results', + Array.isArray(results) && results.length === EXPECTED.length, String(results)); + ok('...with the restart flag intact', + Array.isArray(results) && Manager.restartRequest(results) === answer); + } + console.log(`\n${pass} passed, ${fail} failed`); process.exit(fail ? 1 : 0); })().catch(e => { console.error(e); process.exit(1); }); diff --git a/test/test_api_v3_bool_coercion.py b/test/test_api_v3_bool_coercion.py index c8a60502..8899206f 100644 --- a/test/test_api_v3_bool_coercion.py +++ b/test/test_api_v3_bool_coercion.py @@ -52,7 +52,7 @@ class TestPluginToggle: class TestOnDemandStart: @pytest.fixture def service(self, api_v3_module): - api_v3_module.api_v3.plugin_manager = None + api_v3_module.api_v3.plugin_catalog = None api_v3_module.api_v3.config_manager = None with patch("web_interface.blueprints.api_v3.display._get_display_service_status", return_value={"active": True}), \ diff --git a/test/test_api_v3_calendar_credentials.py b/test/test_api_v3_calendar_credentials.py index 48274e01..816543df 100644 --- a/test/test_api_v3_calendar_credentials.py +++ b/test/test_api_v3_calendar_credentials.py @@ -45,7 +45,7 @@ VALID_CREDENTIALS = { def plugin_dir(tmp_path, api_v3_module): directory = tmp_path / "plugins" / "calendar" directory.mkdir(parents=True) - api_v3_module.api_v3.plugin_manager.get_plugin_directory.return_value = str(directory) + api_v3_module.api_v3.plugin_catalog.get_plugin_directory.return_value = str(directory) return directory @@ -98,7 +98,7 @@ class TestRequestValidation: assert not (plugin_dir / "credentials.json").exists() def test_missing_plugin_directory_is_a_404(self, api_v3_client, api_v3_module, tmp_path): - api_v3_module.api_v3.plugin_manager.get_plugin_directory.return_value = str( + api_v3_module.api_v3.plugin_catalog.get_plugin_directory.return_value = str( tmp_path / "not-installed") assert upload(api_v3_client, VALID_CREDENTIALS).status_code == 404 diff --git a/test/test_api_v3_display_hardware.py b/test/test_api_v3_display_hardware.py index 245df46c..1f612a8a 100644 --- a/test/test_api_v3_display_hardware.py +++ b/test/test_api_v3_display_hardware.py @@ -229,7 +229,7 @@ def display_page(monkeypatch): config_manager.get_config_path.return_value = 'config/config.json' config_manager.get_secrets_path.return_value = 'config/config_secrets.json' monkeypatch.setattr(pv.pages_v3, 'config_manager', config_manager, raising=False) - monkeypatch.setattr(pv.pages_v3, 'plugin_manager', MagicMock(plugins={}), raising=False) + monkeypatch.setattr(pv.pages_v3, 'plugin_catalog', MagicMock(), raising=False) app.register_blueprint(pv.pages_v3, url_prefix='/v3') response = app.test_client().get('/v3/partials/display') assert response.status_code == 200 diff --git a/test/test_api_v3_display_modes.py b/test/test_api_v3_display_modes.py index d77546fa..a59913f0 100644 --- a/test/test_api_v3_display_modes.py +++ b/test/test_api_v3_display_modes.py @@ -34,7 +34,7 @@ CONFIG = { @pytest.fixture def client(api_v3_module, api_v3_client): - pm = api_v3_module.api_v3.plugin_manager + pm = api_v3_module.api_v3.plugin_catalog pm.plugin_manifests = MANIFESTS pm.discover_plugins = MagicMock(return_value=list(MANIFESTS)) pm.get_plugin_display_modes = MagicMock( @@ -85,17 +85,17 @@ class TestItWorksForACallerThatNeverOpensTheDashboard: """Discovery is lazy and normally runs because a person loaded the dashboard; a bridge or script would otherwise get an empty list.""" client.get('/api/v3/display/modes') - api_v3_module.api_v3.plugin_manager.discover_plugins.assert_called_once() + api_v3_module.api_v3.plugin_catalog.discover_plugins.assert_called_once() def test_no_plugin_manager_is_a_clean_error(self, api_v3_module, api_v3_client): - api_v3_module.api_v3.plugin_manager = None + api_v3_module.api_v3.plugin_catalog = None response = api_v3_client.get('/api/v3/display/modes') assert response.status_code == 500 assert response.get_json()['status'] == 'error' def test_a_plugin_with_no_declared_modes_still_appears(self, client, api_v3_module): """Its mode is its own id -- the same fallback the controller uses.""" - pm = api_v3_module.api_v3.plugin_manager + pm = api_v3_module.api_v3.plugin_catalog pm.plugin_manifests = {'starlark-apps': {'name': 'Starlark Apps', 'display_modes': []}} pm.get_plugin_display_modes = MagicMock(return_value=[]) api_v3_module.api_v3.config_manager.load_config = MagicMock( @@ -116,7 +116,7 @@ class TestOneBadConfigSectionDoesNotBlankTheList: @pytest.fixture def client_with_bad_section(self, api_v3_module, api_v3_client): - pm = api_v3_module.api_v3.plugin_manager + pm = api_v3_module.api_v3.plugin_catalog pm.plugin_manifests = MANIFESTS pm.discover_plugins = MagicMock(return_value=list(MANIFESTS)) pm.get_plugin_display_modes = MagicMock( @@ -143,7 +143,7 @@ class TestOneBadConfigSectionDoesNotBlankTheList: self, api_v3_module, api_v3_client): """describe_exception, per test_web_error_detail's contract -- an opaque "see logs for details" is what that test exists to prevent.""" - api_v3_module.api_v3.plugin_manager.discover_plugins = MagicMock( + api_v3_module.api_v3.plugin_catalog.discover_plugins = MagicMock( side_effect=RuntimeError("disk is gone")) resp = api_v3_client.get('/api/v3/display/modes') assert resp.status_code == 500 @@ -151,7 +151,7 @@ class TestOneBadConfigSectionDoesNotBlankTheList: def test_credentials_in_the_exception_are_redacted(self, api_v3_module, api_v3_client): """describe_exception is what makes returning detail safe.""" - api_v3_module.api_v3.plugin_manager.discover_plugins = MagicMock( + api_v3_module.api_v3.plugin_catalog.discover_plugins = MagicMock( side_effect=RuntimeError("GET https://x/y?api_key=SEC123 failed")) body = api_v3_client.get('/api/v3/display/modes').get_json() assert 'SEC123' not in json.dumps(body) diff --git a/test/test_api_v3_health.py b/test/test_api_v3_health.py index d4921c67..888b3101 100644 --- a/test/test_api_v3_health.py +++ b/test/test_api_v3_health.py @@ -32,7 +32,7 @@ def _checks(client): def test_plugin_count_is_the_number_of_discovered_plugins(api_v3_client, api_v3_module): - api_v3_module.api_v3.plugin_manager.plugin_manifests = { + api_v3_module.api_v3.plugin_catalog.plugin_manifests = { "clock": {"id": "clock"}, "weather": {"id": "weather"}, "stocks": {"id": "stocks"}, } @@ -42,7 +42,7 @@ def test_plugin_count_is_the_number_of_discovered_plugins(api_v3_client, api_v3_ def test_plugin_count_discovers_when_nothing_is_discovered_yet(api_v3_client, api_v3_module): - pm = api_v3_module.api_v3.plugin_manager + pm = api_v3_module.api_v3.plugin_catalog pm.plugin_manifests = {} def discover(): diff --git a/test/test_api_v3_installed_plugin_icon.py b/test/test_api_v3_installed_plugin_icon.py index 984a9160..7f661a19 100644 --- a/test/test_api_v3_installed_plugin_icon.py +++ b/test/test_api_v3_installed_plugin_icon.py @@ -20,9 +20,8 @@ def installed(api_v3_module, api_v3_client, tmp_path): api = api_v3_module.api_v3 info = {'id': 'demo', 'name': 'Demo', 'version': '1.0.0', 'loaded': False} info.update(manifest_extra) - api.plugin_manager.plugins_dir = str(tmp_path) # no manifest on disk - api.plugin_manager.get_all_plugin_info = MagicMock(return_value=[info]) - api.plugin_manager.get_plugin = MagicMock(return_value=None) + api.plugin_catalog.plugins_dir = str(tmp_path) # no manifest on disk + api.plugin_catalog.get_all_plugin_info = MagicMock(return_value=[info]) api.plugin_store_manager.get_registry_info = MagicMock(return_value=None) api.config_manager.load_config = MagicMock(return_value={}) response = api_v3_client.get('/api/v3/plugins/installed') diff --git a/test/test_api_v3_lazy_plugin_discovery.py b/test/test_api_v3_lazy_plugin_discovery.py index 1dffae81..c3c30e1c 100644 --- a/test/test_api_v3_lazy_plugin_discovery.py +++ b/test/test_api_v3_lazy_plugin_discovery.py @@ -16,7 +16,7 @@ callers (the Home Assistant MQTT bridge, scripts) saw it after every restart. undiscovered plugin section skipped secret separation and wrote its API key into config.json in plain text. -A real PluginManager over a temporary plugins directory, so "empty until +A real PluginCatalog over a temporary plugins directory, so "empty until discovered" is the real behaviour rather than a mock's. """ @@ -30,7 +30,7 @@ import pytest sys.path.insert(0, str(Path(__file__).parent.parent)) from src.config_manager import ConfigManager # noqa: E402 -from src.plugin_system.plugin_manager import PluginManager # noqa: E402 +from src.plugin_system.plugin_catalog import PluginCatalog # noqa: E402 from src.plugin_system.schema_manager import SchemaManager # noqa: E402 from test._api_v3_test_helpers import api_v3_client, api_v3_module # noqa: F401,E402 @@ -69,10 +69,10 @@ def plugins_dir(tmp_path): @pytest.fixture def fresh_web_process(api_v3_module, plugins_dir): """The web process right after a restart: nothing discovered yet.""" - manager = PluginManager(plugins_dir=str(plugins_dir)) - assert not manager.plugin_manifests - api_v3_module.api_v3.plugin_manager = manager - return manager + catalog = PluginCatalog(plugins_dir=plugins_dir) + assert not catalog.plugin_manifests + api_v3_module.api_v3.plugin_catalog = catalog + return catalog @pytest.fixture diff --git a/test/test_api_v3_on_demand_restart.py b/test/test_api_v3_on_demand_restart.py index df6c810b..17d31edd 100644 --- a/test/test_api_v3_on_demand_restart.py +++ b/test/test_api_v3_on_demand_restart.py @@ -47,7 +47,7 @@ def service(api_v3_module): resolution (not what is under test here). The cache is the blueprint's MagicMock cache_manager, so mailbox writes are visible as set() calls. """ - api_v3_module.api_v3.plugin_manager = None + api_v3_module.api_v3.plugin_catalog = None api_v3_module.api_v3.config_manager = None state = {"active": True} diff --git a/test/test_api_v3_plugin_health_single.py b/test/test_api_v3_plugin_health_single.py index 455914ea..19f1a616 100644 --- a/test/test_api_v3_plugin_health_single.py +++ b/test/test_api_v3_plugin_health_single.py @@ -35,9 +35,8 @@ class SharedCache: @pytest.fixture def shared_cache(api_v3_module): cache = SharedCache() - pm = api_v3_module.api_v3.plugin_manager - pm.health_tracker = PluginHealthTracker(cache) - pm.resource_monitor = PluginResourceMonitor(cache) + api_v3_module.api_v3.health_tracker = PluginHealthTracker(cache) + api_v3_module.api_v3.resource_monitor = PluginResourceMonitor(cache) return cache diff --git a/test/test_api_v3_plugin_install_endpoints.py b/test/test_api_v3_plugin_install_endpoints.py index 29f7f7dc..07e384d7 100644 --- a/test/test_api_v3_plugin_install_endpoints.py +++ b/test/test_api_v3_plugin_install_endpoints.py @@ -5,6 +5,11 @@ Both were only ever tested at the PluginStoreManager layer, so the route logic — the queue-vs-direct branch, schema invalidation, plugin discovery, state and history recording — was unexercised. +Neither route loads the plugin: the web process only lists it (the catalog +has no load_plugin, so calling one fails these tests). The display loads it +when it is enabled; restart_required says when that won't happen by itself +(test/web_interface/test_web_process_runs_no_plugin_code.py). + /plugins/install carries the same install logic twice: once inside the operation-queue callback and once in the direct fallback. The paired tests below assert both branches produce the same side effects, so the @@ -44,8 +49,7 @@ def side_effects(module): api = module.api_v3 return { "schema_invalidated": api.schema_manager.invalidate_cache.call_args_list, - "discovered": api.plugin_manager.discover_plugins.call_count, - "loaded": api.plugin_manager.load_plugin.call_args_list, + "discovered": api.plugin_catalog.discover_plugins.call_count, "state_set": api.plugin_state_manager.set_plugin_installed.call_args_list, "history": api.operation_history.record_operation.call_args_list, } @@ -83,7 +87,6 @@ class TestInstallDirectPath: effects = side_effects(api_v3_module) assert effects["schema_invalidated"] == [(("clock",), {})] assert effects["discovered"] == 1 - assert effects["loaded"] == [(("clock",), {})] assert effects["state_set"] == [(("clock",), {})] assert effects["history"][0].kwargs["status"] == "success" @@ -130,7 +133,7 @@ class TestInstallDirectPath: api_v3_client.post(INSTALL, json={"plugin_id": "clock"}) effects = side_effects(api_v3_module) assert effects["schema_invalidated"] == [] - assert effects["loaded"] == [] + assert effects["discovered"] == 0 assert effects["state_set"] == [] @@ -154,7 +157,6 @@ class TestInstallQueuedPath: effects = side_effects(api_v3_module) assert effects["schema_invalidated"] == [(("clock",), {})] assert effects["discovered"] == 1 - assert effects["loaded"] == [(("clock",), {})] assert effects["state_set"] == [(("clock",), {})] assert effects["history"][0].kwargs["status"] == "success" @@ -196,7 +198,7 @@ class TestInstallPathsAgree: # Reset and re-run through the queue. for mock in (api_v3_module.api_v3.schema_manager, - api_v3_module.api_v3.plugin_manager, + api_v3_module.api_v3.plugin_catalog, api_v3_module.api_v3.plugin_state_manager, api_v3_module.api_v3.operation_history): mock.reset_mock() @@ -208,7 +210,6 @@ class TestInstallPathsAgree: assert direct["schema_invalidated"] == queued["schema_invalidated"] assert direct["discovered"] == queued["discovered"] - assert direct["loaded"] == queued["loaded"] assert direct["state_set"] == queued["state_set"] assert (direct["history"][0].kwargs["status"] == queued["history"][0].kwargs["status"]) @@ -260,21 +261,22 @@ class TestInstallFromUrl: branch="dev", ) - def test_success_invalidates_schema_and_loads_plugin(self, api_v3_client, api_v3_module): + def test_success_invalidates_schema_and_lists_plugin(self, api_v3_client, api_v3_module): api_v3_module.api_v3.plugin_store_manager.install_from_url.return_value = { "success": True, "plugin_id": "clock"} - api_v3_client.post(FROM_URL, json={"repo_url": "http://x"}) + response = api_v3_client.post(FROM_URL, json={"repo_url": "http://x"}) + assert response.status_code == 200, response.get_json() api_v3_module.api_v3.schema_manager.invalidate_cache.assert_called_once_with("clock") - api_v3_module.api_v3.plugin_manager.load_plugin.assert_called_once_with("clock") + api_v3_module.api_v3.plugin_catalog.discover_plugins.assert_called_once_with() def test_success_without_plugin_id_skips_discovery(self, api_v3_client, api_v3_module): # install_from_url can succeed without naming the plugin; there is - # then nothing to invalidate or load. + # then nothing to invalidate or list. api_v3_module.api_v3.plugin_store_manager.install_from_url.return_value = { "success": True, "plugin_id": None} api_v3_client.post(FROM_URL, json={"repo_url": "http://x"}) api_v3_module.api_v3.schema_manager.invalidate_cache.assert_not_called() - api_v3_module.api_v3.plugin_manager.load_plugin.assert_not_called() + api_v3_module.api_v3.plugin_catalog.discover_plugins.assert_not_called() def test_branch_from_result_included(self, api_v3_client, api_v3_module): api_v3_module.api_v3.plugin_store_manager.install_from_url.return_value = { diff --git a/test/test_api_v3_upload_writes.py b/test/test_api_v3_upload_writes.py index e26dec3d..dc68603b 100644 --- a/test/test_api_v3_upload_writes.py +++ b/test/test_api_v3_upload_writes.py @@ -110,7 +110,7 @@ class TestCalendarCredentials: def plugin_dir(self, tmp_path, api_v3_module): directory = tmp_path / "plugins" / "calendar" directory.mkdir(parents=True) - api_v3_module.api_v3.plugin_manager.get_plugin_directory.return_value = str(directory) + api_v3_module.api_v3.plugin_catalog.get_plugin_directory.return_value = str(directory) return directory def _post(self, client): diff --git a/test/test_auto_update.py b/test/test_auto_update.py index 7a1b7f40..4b97d4c7 100644 --- a/test/test_auto_update.py +++ b/test/test_auto_update.py @@ -793,7 +793,7 @@ def api_client(monkeypatch): cm.get_raw_file_content.return_value = {} cm.save_config_atomic.return_value = MagicMock(status=MagicMock(value='success'), message=None) api_v3.config_manager = cm - api_v3.plugin_manager = MagicMock(plugins={}) + api_v3.plugin_catalog = MagicMock() # Never restart a real display from a test run. setup_calls = [] monkeypatch.setattr(au, 'start_setup_if_needed', diff --git a/test/test_core_config_keys.py b/test/test_core_config_keys.py index 9ff938de..e6436184 100644 --- a/test/test_core_config_keys.py +++ b/test/test_core_config_keys.py @@ -241,7 +241,7 @@ class TestTheStatusEndpoint: pm = MagicMock() pm.plugins_dir = str(plugins_dir) monkeypatch.setattr(api_v3, "config_manager", cm, raising=False) - monkeypatch.setattr(api_v3, "plugin_manager", pm, raising=False) + monkeypatch.setattr(api_v3, "plugin_catalog", pm, raising=False) app = Flask(__name__) app.config["TESTING"] = True app.register_blueprint(api_v3, url_prefix="/api/v3") diff --git a/test/test_onboarding_checklist.py b/test/test_onboarding_checklist.py index 1a6c15e7..07dd0e40 100644 --- a/test/test_onboarding_checklist.py +++ b/test/test_onboarding_checklist.py @@ -57,7 +57,7 @@ def render(config): # pages_v3 is a module-level singleton shared across the test process; # restore whatever the previous test left on it. original_cm = getattr(pv.pages_v3, "config_manager", None) - original_pm = getattr(pv.pages_v3, "plugin_manager", None) + original_pm = getattr(pv.pages_v3, "plugin_catalog", None) mock_cm = MagicMock() mock_cm.load_config.return_value = config @@ -65,10 +65,9 @@ def render(config): pv.pages_v3.config_manager = mock_cm mock_pm = MagicMock() - mock_pm.plugins = {} mock_pm.get_all_plugin_info.return_value = [] mock_pm.get_plugin_display_modes.side_effect = lambda pid: [] - pv.pages_v3.plugin_manager = mock_pm + pv.pages_v3.plugin_catalog = mock_pm app.register_blueprint(pv.pages_v3, url_prefix="") try: @@ -77,7 +76,7 @@ def render(config): return resp.get_data(as_text=True) finally: pv.pages_v3.config_manager = original_cm - pv.pages_v3.plugin_manager = original_pm + pv.pages_v3.plugin_catalog = original_pm def timezone_step(body): diff --git a/test/test_pages_v3_partials.py b/test/test_pages_v3_partials.py index 3aa54566..67de2f2c 100644 --- a/test/test_pages_v3_partials.py +++ b/test/test_pages_v3_partials.py @@ -17,7 +17,7 @@ from web_interface.blueprints import pages_v3 as module # noqa: E402 def client(tmp_path, monkeypatch): plugin_manager = MagicMock() plugin_manager.plugins_dir = tmp_path - monkeypatch.setattr(module.pages_v3, "plugin_manager", plugin_manager, raising=False) + monkeypatch.setattr(module.pages_v3, "plugin_catalog", plugin_manager, raising=False) monkeypatch.setattr(module.pages_v3, "config_manager", MagicMock(load_config=lambda: {}), raising=False) app = Flask(__name__, template_folder=str( diff --git a/test/test_pages_v3_path_guards.py b/test/test_pages_v3_path_guards.py index 2ec1c3e2..962fbbc3 100644 --- a/test/test_pages_v3_path_guards.py +++ b/test/test_pages_v3_path_guards.py @@ -39,19 +39,18 @@ def pages(tmp_path): "

panel

", encoding="utf-8" ) - original_pm = getattr(module.pages_v3, "plugin_manager", None) + original_pm = getattr(module.pages_v3, "plugin_catalog", None) original_cm = getattr(module.pages_v3, "config_manager", None) plugin_manager = MagicMock() plugin_manager.plugins_dir = plugins_dir plugin_manager.get_plugin_info.return_value = {"name": "Weather", "version": "1.0.0"} - plugin_manager.get_plugin.return_value = None - module.pages_v3.plugin_manager = plugin_manager + module.pages_v3.plugin_catalog = plugin_manager module.pages_v3.config_manager = MagicMock(load_config=lambda: {}) yield module, plugins_dir - module.pages_v3.plugin_manager = original_pm + module.pages_v3.plugin_catalog = original_pm module.pages_v3.config_manager = original_cm diff --git a/test/test_path_traversal_guards.py b/test/test_path_traversal_guards.py index 6dadcca5..5af7bb55 100644 --- a/test/test_path_traversal_guards.py +++ b/test/test_path_traversal_guards.py @@ -132,8 +132,8 @@ class TestServePluginStatic: candidate = plugins / plugin_id return str(candidate) if candidate.exists() else None - api_v3_module.api_v3.plugin_manager = MagicMock() - api_v3_module.api_v3.plugin_manager.get_plugin_directory = get_plugin_directory + api_v3_module.api_v3.plugin_catalog = MagicMock() + api_v3_module.api_v3.plugin_catalog.get_plugin_directory = get_plugin_directory return plugins def test_a_real_plugin_file_is_still_served( @@ -397,10 +397,13 @@ class TestPluginActionDirectory: '../elsewhere' ran a script from any directory holding a manifest. """ - @pytest.fixture - def tree(self, tmp_path): + @pytest.fixture(params=["catalog", "manager"]) + def tree(self, request, tmp_path): + """The web process resolves through PluginCatalog; the display + through PluginManager. Both apply the same rules.""" import threading + from src.plugin_system.plugin_catalog import PluginCatalog from src.plugin_system.plugin_manager import PluginManager plugins = tmp_path / "plugin-repos" @@ -415,6 +418,8 @@ class TestPluginActionDirectory: (outside / "s.py").write_text( "open(%r, 'w').write('ran')\n" % str(marker), encoding="utf-8" ) + if request.param == "catalog": + return PluginCatalog(plugins), plugins, marker manager = MagicMock() manager.plugins_dir = plugins manager._discovery_lock = threading.Lock() @@ -446,7 +451,7 @@ class TestPluginActionDirectory: self, api_v3_client, api_v3_module, tree ): manager, _, marker = tree - api_v3_module.api_v3.plugin_manager = manager + api_v3_module.api_v3.plugin_catalog = manager response = api_v3_client.post( "/api/v3/plugins/action", json={"plugin_id": "../elsewhere", "action_id": "go", "params": {}}, @@ -457,7 +462,7 @@ class TestPluginActionDirectory: def test_the_fallback_without_a_plugin_manager_is_guarded_too( self, api_v3_client, api_v3_module ): - api_v3_module.api_v3.plugin_manager = None + api_v3_module.api_v3.plugin_catalog = None response = api_v3_client.post( "/api/v3/plugins/action", json={"plugin_id": "../elsewhere", "action_id": "go", "params": {}}, diff --git a/test/test_reconciliation_status_endpoint.py b/test/test_reconciliation_status_endpoint.py index 885548ce..64bf48ea 100644 --- a/test/test_reconciliation_status_endpoint.py +++ b/test/test_reconciliation_status_endpoint.py @@ -64,7 +64,7 @@ def client(tmp_path, monkeypatch): # singleton, so assigning them directly leaks mocks -- pointing at a # deleted tmp_path -- into every later test that imports api_v3. monkeypatch.setattr(api_v3, "config_manager", cm, raising=False) - monkeypatch.setattr(api_v3, "plugin_manager", pm, raising=False) + monkeypatch.setattr(api_v3, "plugin_catalog", pm, raising=False) app = Flask(__name__) app.config["TESTING"] = True diff --git a/test/test_resource_limits_validation.py b/test/test_resource_limits_validation.py index 213ed69a..7569aa95 100644 --- a/test/test_resource_limits_validation.py +++ b/test/test_resource_limits_validation.py @@ -39,7 +39,7 @@ class SharedCache: @pytest.fixture def shared_cache(api_v3_module): cache = SharedCache() - api_v3_module.api_v3.plugin_manager.resource_monitor = PluginResourceMonitor( + api_v3_module.api_v3.resource_monitor = PluginResourceMonitor( cache, enable_monitoring=False) return cache diff --git a/test/test_store_registry_fields.py b/test/test_store_registry_fields.py index 4ea7da0d..a767aaa5 100644 --- a/test/test_store_registry_fields.py +++ b/test/test_store_registry_fields.py @@ -342,9 +342,9 @@ 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 + # No plugin catalog: the routes skip discovery, and the update route + # finds the directory through the store alone. + api_v3_module.api_v3.plugin_catalog = None return fake diff --git a/test/test_system_status_available_memory.py b/test/test_system_status_available_memory.py index 0113ea67..f5b41550 100644 --- a/test/test_system_status_available_memory.py +++ b/test/test_system_status_available_memory.py @@ -33,7 +33,7 @@ def client(): app = Flask(__name__) app.config["TESTING"] = True from web_interface.blueprints.api_v3 import api_v3 - for attr in ("config_manager", "plugin_manager", "cache_manager"): + for attr in ("config_manager", "plugin_catalog", "cache_manager"): setattr(api_v3, attr, MagicMock()) if "api_v3" not in app.blueprints: app.register_blueprint(api_v3, url_prefix="/api/v3") diff --git a/test/test_uninstall_and_reconcile_endpoint.py b/test/test_uninstall_and_reconcile_endpoint.py index 27494e28..99534acd 100644 --- a/test/test_uninstall_and_reconcile_endpoint.py +++ b/test/test_uninstall_and_reconcile_endpoint.py @@ -28,11 +28,14 @@ sys.path.insert(0, str(project_root)) from flask import Flask +from test._api_v3_test_helpers import mock_plugin_catalog + _API_V3_MOCKED_ATTRS = ( - 'config_manager', 'plugin_manager', 'plugin_store_manager', + 'config_manager', 'plugin_catalog', 'plugin_store_manager', 'plugin_state_manager', 'saved_repositories_manager', 'schema_manager', 'operation_queue', 'operation_history', 'cache_manager', + 'health_tracker', 'resource_monitor', ) @@ -58,9 +61,8 @@ def _make_client(): api_v3.config_manager = MagicMock() api_v3.config_manager.get_raw_file_content.return_value = {} api_v3.config_manager.secrets_path = "/tmp/nonexistent_secrets.json" - api_v3.plugin_manager = MagicMock() - api_v3.plugin_manager.plugins = {} - api_v3.plugin_manager.plugins_dir = "/tmp" + api_v3.plugin_catalog = mock_plugin_catalog() + api_v3.plugin_catalog.plugins_dir = "/tmp" api_v3.plugin_store_manager = MagicMock() api_v3.plugin_state_manager = MagicMock() api_v3.plugin_state_manager.get_all_states.return_value = {} @@ -202,22 +204,20 @@ class TestTransactionalUninstall(unittest.TestCase): self.assertEqual(response.status_code, 200) self.api_v3.plugin_store_manager.uninstall_plugin.assert_called_once_with('thing') - def test_file_removal_failure_reloads_previously_loaded_plugin(self): - """Regression: rollback must restore BOTH config AND runtime state. + def test_file_removal_failure_rolls_back_config_only(self): + """The web process has no running plugin to unload or reload. - If the plugin was loaded at runtime before the uninstall - request, and file removal fails after unload has already - succeeded, the rollback must call ``load_plugin`` so the user - doesn't end up in a state where the files exist and the config - exists but the plugin is no longer loaded. + The display runs the plugin, and unloads it when the removed config + section reaches its config watcher. A failed file removal restores + that section, so the display sees no change at all; there is nothing + in this process to put back. (The catalog mock has no + unload_plugin/load_plugin: calling either fails with a different + message than the one asserted.) """ - # Plugin is currently loaded. - self.api_v3.plugin_manager.plugins = {'thing': MagicMock()} self.api_v3.config_manager.get_raw_file_content.return_value = { 'thing': {'enabled': True} } self.api_v3.config_manager.cleanup_plugin_config.return_value = None - self.api_v3.plugin_manager.unload_plugin.return_value = None self.api_v3.plugin_store_manager.uninstall_plugin.return_value = False response = self.client.post( @@ -227,11 +227,10 @@ class TestTransactionalUninstall(unittest.TestCase): ) self.assertEqual(response.status_code, 500) - # Unload did happen (it's part of the uninstall sequence)... - self.api_v3.plugin_manager.unload_plugin.assert_called_once_with('thing') - # ...and because file removal failed, the rollback must have - # called load_plugin to restore runtime state. - self.api_v3.plugin_manager.load_plugin.assert_called_once_with('thing') + self.assertIn('Failed to uninstall plugin thing', response.get_json()['message']) + calls = self.api_v3.config_manager.save_raw_file_content.call_args_list + self.assertTrue(any(c.args[0] == 'main' for c in calls), + "main config was not restored after the removal failed") def test_snapshot_survives_config_read_error(self): """Regression: if get_raw_file_content raises an expected error @@ -283,33 +282,6 @@ class TestTransactionalUninstall(unittest.TestCase): # exception bubbled up before we got that far. self.api_v3.plugin_store_manager.uninstall_plugin.assert_not_called() - def test_unload_failure_restores_config_and_does_not_call_uninstall(self): - """If unload_plugin itself raises, config must be restored and - uninstall_plugin must NOT be called.""" - self.api_v3.plugin_manager.plugins = {'thing': MagicMock()} - self.api_v3.config_manager.get_raw_file_content.return_value = { - 'thing': {'enabled': True} - } - self.api_v3.config_manager.cleanup_plugin_config.return_value = None - self.api_v3.plugin_manager.unload_plugin.side_effect = RuntimeError("unload boom") - - response = self.client.post( - '/api/v3/plugins/uninstall', - data=json.dumps({'plugin_id': 'thing'}), - content_type='application/json', - ) - - self.assertEqual(response.status_code, 500) - self.api_v3.plugin_store_manager.uninstall_plugin.assert_not_called() - # Config should have been restored. - calls = self.api_v3.config_manager.save_raw_file_content.call_args_list - self.assertTrue( - any(c.args[0] == 'main' for c in calls), - "main config was not restored after unload_plugin raised", - ) - # load_plugin must NOT have been called — unload didn't succeed, - # so runtime state is still what it was. - self.api_v3.plugin_manager.load_plugin.assert_not_called() class TestReconcileEndpointPayload(unittest.TestCase): @@ -402,7 +374,7 @@ class TestNonPluginIdsAreRefused(unittest.TestCase): self.client, self.mod, _cleanup = _make_client() self.addCleanup(_cleanup) self.api_v3 = self.mod.api_v3 - self.api_v3.plugin_manager.plugin_manifests = {'thing': {'id': 'thing'}} + self.api_v3.plugin_catalog.plugin_manifests = {'thing': {'id': 'thing'}} def _post(self, url, body): return self.client.post(url, data=json.dumps(body), @@ -427,7 +399,7 @@ class TestNonPluginIdsAreRefused(unittest.TestCase): 'gone-plugin', remove_secrets=True) def test_secrets_only_core_key_is_allowed_when_a_plugin_has_that_id(self): - self.api_v3.plugin_manager.plugin_manifests = {'youtube': {'id': 'youtube'}} + self.api_v3.plugin_catalog.plugin_manifests = {'youtube': {'id': 'youtube'}} self.api_v3.plugin_store_manager.uninstall_plugin.return_value = True response = self._post('/api/v3/plugins/uninstall', {'plugin_id': 'youtube'}) diff --git a/test/test_vegas_participation.py b/test/test_vegas_participation.py index a7a5d375..93949e06 100644 --- a/test/test_vegas_participation.py +++ b/test/test_vegas_participation.py @@ -434,28 +434,46 @@ class TestSchema: class TestInstalledPluginsApi: + """The web process runs no plugin code (it has a PluginCatalog, not a + PluginManager), so it reports what the files say -- the user's setting, + then the manifest -- and leaves the rest to the display.""" + @pytest.fixture - def installed(self, api_v3_module, api_v3_client, tmp_path): - def _get(instance, config): + def installed(self, api_v3_module, api_v3_client): + def _get(config, manifest_extra=None): api = api_v3_module.api_v3 - info = {'id': 'demo', 'name': 'Demo', 'version': '1.0.0', 'loaded': True} - api.plugin_manager.plugins_dir = str(tmp_path) - api.plugin_manager.get_all_plugin_info = MagicMock(return_value=[info]) - api.plugin_manager.get_plugin = MagicMock(return_value=instance) + info = {'id': 'demo', 'name': 'Demo', 'version': '1.0.0', + **(manifest_extra or {})} + api.plugin_catalog.get_all_plugin_info = MagicMock(return_value=[info]) + api.plugin_catalog.get_plugin_directory = MagicMock(return_value=None) api.plugin_store_manager.get_registry_info = MagicMock(return_value=None) api.config_manager.load_config = MagicMock(return_value={'demo': config}) response = api_v3_client.get('/api/v3/plugins/installed') assert response.status_code == 200 - return [p for p in response.get_json()['data']['plugins'] - if p['id'] == 'demo'][0] + entry = [p for p in response.get_json()['data']['plugins'] + if p['id'] == 'demo'][0] + return entry['vegas_participation'], entry['vegas_participation_source'] return _get - def test_a_loaded_plugin_reports_its_participation(self, installed): - plugin = _plugin({'vegas_mode': 'static'}) - assert installed(plugin, plugin.config)['vegas_participation'] == 'pause' + def test_the_user_setting_wins(self, installed): + assert installed({'vegas_participation': 'exclude'}, + {'vegas_participation': 'pause'}) == ('exclude', 'config') - def test_an_unloaded_plugin_reports_only_the_user_setting(self, installed): - assert installed(None, {'vegas_participation': 'exclude'})[ - 'vegas_participation'] == 'exclude' - assert installed(None, {})['vegas_participation'] is None + def test_then_the_manifest_declaration(self, installed): + assert installed({}, {'vegas_participation': 'Pause'}) == ('pause', 'manifest') + # An invalid setting is ignored, as the display ignores it. + assert installed({'vegas_participation': 'fixed'}, + {'vegas_participation': 'scroll'}) == ('scroll', 'manifest') + + def test_otherwise_it_is_decided_at_run_time(self, installed): + # The legacy hooks would say 'pause' for vegas_mode 'static', but + # only the display runs them: nothing is guessed here. + assert installed({'vegas_mode': 'static'}) == (None, 'runtime') + assert installed({}, {'vegas_participation': 'sometimes'}) == (None, 'runtime') + + def test_it_matches_the_display_where_the_files_decide(self, installed): + for config, manifest in (({'vegas_participation': 'pause'}, None), + ({}, {'vegas_participation': 'exclude'})): + plugin = _plugin(config, manifest) + assert installed(config, manifest)[0] == resolve_vegas_participation(plugin) diff --git a/test/test_web_api.py b/test/test_web_api.py index ebc0cfdd..eb94d25d 100644 --- a/test/test_web_api.py +++ b/test/test_web_api.py @@ -43,18 +43,15 @@ def mock_config_manager(): @pytest.fixture -def mock_plugin_manager(): - """Create a mock plugin manager.""" +def mock_plugin_catalog(): + """Create a mock plugin catalog.""" mock = MagicMock() - mock.plugins = {} mock.discover_plugins.return_value = [] - mock.health_tracker = MagicMock() - mock.health_tracker.get_health_status.return_value = {'healthy': True} return mock @pytest.fixture -def client(mock_config_manager, mock_plugin_manager): +def client(mock_config_manager, mock_plugin_catalog): """Create a Flask test client with mocked dependencies.""" # Create a minimal Flask app for testing test_app = Flask(__name__) @@ -66,7 +63,7 @@ def client(mock_config_manager, mock_plugin_manager): # Mock the managers on the blueprint api_v3.config_manager = mock_config_manager - api_v3.plugin_manager = mock_plugin_manager + api_v3.plugin_catalog = mock_plugin_catalog api_v3.plugin_store_manager = MagicMock() api_v3.saved_repositories_manager = MagicMock() api_v3.schema_manager = MagicMock() @@ -74,7 +71,10 @@ def client(mock_config_manager, mock_plugin_manager): api_v3.plugin_state_manager = MagicMock() api_v3.operation_history = MagicMock() api_v3.cache_manager = MagicMock() - + # Readers of what the display publishes (app.py wires real ones). + api_v3.health_tracker = MagicMock() + api_v3.resource_monitor = MagicMock() + # Setup operation queue mocks mock_operation = MagicMock() mock_operation.operation_id = 'test-op-123' @@ -128,7 +128,7 @@ class TestConfigAPI: mock_config_manager.save_config_atomic.assert_called_once() def test_save_main_config_fails_closed_when_schema_path_is_unresolvable( - self, client, mock_config_manager, mock_plugin_manager + self, client, mock_config_manager, mock_plugin_catalog ): """A plugin id whose schema path fails safe-resolution must not have its config saved with secret_fields left empty. @@ -140,7 +140,7 @@ class TestConfigAPI: the plugin's submitted config -- credentials included -- as ordinary, unencrypted configuration instead of refusing the request. """ - mock_plugin_manager.plugin_manifests = {'evil': {}} + mock_plugin_catalog.plugin_manifests = {'evil': {}} with patch('web_interface.blueprints.api_v3.config.resolve_under', return_value=None): response = client.post( @@ -545,38 +545,35 @@ class TestDisplayAPI: class TestPluginsAPI: """Test plugins API endpoints.""" - def test_get_installed_plugins(self, client, mock_plugin_manager): + def test_get_installed_plugins(self, client, mock_plugin_catalog): """Test getting list of installed plugins.""" from web_interface.blueprints.api_v3 import api_v3 - api_v3.plugin_manager = mock_plugin_manager - - mock_plugin_manager.plugins = { - 'weather': MagicMock(plugin_id='weather'), - 'clock': MagicMock(plugin_id='clock') - } - mock_plugin_manager.get_plugin_metadata.return_value = { - 'id': 'weather', - 'name': 'Weather Plugin' - } + api_v3.plugin_catalog = mock_plugin_catalog + mock_plugin_catalog.plugins_dir = '/nonexistent-plugins-dir' + mock_plugin_catalog.get_plugin_directory.return_value = None + mock_plugin_catalog.get_all_plugin_info.return_value = [ + {'id': 'weather', 'name': 'Weather Plugin'} + ] + api_v3.plugin_store_manager.get_registry_info.return_value = None + response = client.get('/api/v3/plugins/installed') assert response.status_code == 200 data = json.loads(response.data) - assert isinstance(data, (list, dict)) + names = [p['name'] for p in data['data']['plugins']] + assert 'Weather Plugin' in names - def test_installed_plugins_report_update_available(self, client, mock_plugin_manager): + def test_installed_plugins_report_update_available(self, client, mock_plugin_catalog): """Installed-plugin entries surface latest_version + update_available by comparing the on-disk manifest version to the registry.""" from web_interface.blueprints.api_v3 import api_v3 - api_v3.plugin_manager = mock_plugin_manager + api_v3.plugin_catalog = mock_plugin_catalog # No on-disk manifest to merge — keep the version we hand in below. - mock_plugin_manager.plugins_dir = '/nonexistent-plugins-dir' - mock_plugin_manager.get_all_plugin_info.return_value = [ + mock_plugin_catalog.plugins_dir = '/nonexistent-plugins-dir' + mock_plugin_catalog.get_all_plugin_info.return_value = [ {'id': 'weather', 'name': 'Weather', 'version': '1.0.0'} ] - # Avoid touching plugin instances (Vegas hooks, enabled fallback). - mock_plugin_manager.get_plugin.return_value = None # Registry advertises a newer version than the installed one. api_v3.plugin_store_manager.get_registry_info.return_value = { 'verified': True, 'latest_version': '1.2.0' @@ -591,15 +588,14 @@ class TestPluginsAPI: assert entry['latest_version'] == '1.2.0' assert entry['update_available'] is True - def test_installed_plugins_no_update_when_current(self, client, mock_plugin_manager): + def test_installed_plugins_no_update_when_current(self, client, mock_plugin_catalog): """No update is flagged when installed version matches the registry.""" from web_interface.blueprints.api_v3 import api_v3 - api_v3.plugin_manager = mock_plugin_manager - mock_plugin_manager.plugins_dir = '/nonexistent-plugins-dir' - mock_plugin_manager.get_all_plugin_info.return_value = [ + api_v3.plugin_catalog = mock_plugin_catalog + mock_plugin_catalog.plugins_dir = '/nonexistent-plugins-dir' + mock_plugin_catalog.get_all_plugin_info.return_value = [ {'id': 'weather', 'name': 'Weather', 'version': '1.2.0'} ] - mock_plugin_manager.get_plugin.return_value = None api_v3.plugin_store_manager.get_registry_info.return_value = { 'verified': True, 'latest_version': '1.2.0' } @@ -625,17 +621,17 @@ class TestPluginsAPI: # mismatch rather than hiding a possible update. assert _is_plugin_update_available('1.0.0', 'not-a-semver') is True - def test_get_plugin_health(self, client, mock_plugin_manager): + def test_get_plugin_health(self, client, mock_plugin_catalog): """Test getting plugin health information.""" from web_interface.blueprints.api_v3 import api_v3 - api_v3.plugin_manager = mock_plugin_manager + api_v3.plugin_catalog = mock_plugin_catalog # Setup health tracker mock_health_tracker = MagicMock() mock_health_tracker.get_all_health_summaries.return_value = { 'weather': {'healthy': True} } - mock_plugin_manager.health_tracker = mock_health_tracker + api_v3.health_tracker = mock_health_tracker response = client.get('/api/v3/plugins/health') @@ -643,10 +639,10 @@ class TestPluginsAPI: data = json.loads(response.data) assert isinstance(data, (list, dict)) - def test_get_plugin_health_single(self, client, mock_plugin_manager): + def test_get_plugin_health_single(self, client, mock_plugin_catalog): """Test getting health for single plugin.""" from web_interface.blueprints.api_v3 import api_v3 - api_v3.plugin_manager = mock_plugin_manager + api_v3.plugin_catalog = mock_plugin_catalog # Setup health tracker with proper method (endpoint calls get_health_summary) mock_health_tracker = MagicMock() @@ -655,7 +651,7 @@ class TestPluginsAPI: 'failures': 0, 'last_success': '2024-01-01T00:00:00' } - mock_plugin_manager.health_tracker = mock_health_tracker + api_v3.health_tracker = mock_health_tracker response = client.get('/api/v3/plugins/health/weather') @@ -663,16 +659,16 @@ class TestPluginsAPI: data = json.loads(response.data) assert 'healthy' in data.get('data', {}) or 'data' in data - def test_toggle_plugin(self, client, mock_config_manager, mock_plugin_manager): + def test_toggle_plugin(self, client, mock_config_manager, mock_plugin_catalog): """Test toggling plugin enabled state.""" from web_interface.blueprints.api_v3 import api_v3 api_v3.config_manager = mock_config_manager - api_v3.plugin_manager = mock_plugin_manager + api_v3.plugin_catalog = mock_plugin_catalog api_v3.plugin_state_manager = MagicMock() api_v3.operation_history = MagicMock() # Setup plugin manifests - mock_plugin_manager.plugin_manifests = {'weather': {}} + mock_plugin_catalog.plugin_manifests = {'weather': {}} request_data = { 'plugin_id': 'weather', @@ -1006,15 +1002,15 @@ class TestPluginHealthRoutes: """Phase 1: /plugins/health and /plugins/metrics build per-installed-id so they surface cross-process data persisted by the display service.""" - def test_health_route_builds_per_installed_id(self, client, mock_plugin_manager): + def test_health_route_builds_per_installed_id(self, client, mock_plugin_catalog): from web_interface.blueprints.api_v3 import api_v3 from src.plugin_system.plugin_health import PluginHealthTracker cache = MagicMock() cache.get.return_value = None - api_v3.plugin_manager = mock_plugin_manager - mock_plugin_manager.plugin_manifests = {'p1': {}, 'p2': {}} - mock_plugin_manager.health_tracker = PluginHealthTracker(cache) + api_v3.plugin_catalog = mock_plugin_catalog + mock_plugin_catalog.plugin_manifests = {'p1': {}, 'p2': {}} + api_v3.health_tracker = PluginHealthTracker(cache) resp = client.get('/api/v3/plugins/health') assert resp.status_code == 200 @@ -1023,10 +1019,10 @@ class TestPluginHealthRoutes: assert data['p1']['is_healthy'] is True assert data['p1']['degraded'] is False - def test_health_route_reports_not_available_without_tracker(self, client, mock_plugin_manager): + def test_health_route_reports_not_available_without_tracker(self, client, mock_plugin_catalog): from web_interface.blueprints.api_v3 import api_v3 - api_v3.plugin_manager = mock_plugin_manager - mock_plugin_manager.health_tracker = None + api_v3.plugin_catalog = mock_plugin_catalog + api_v3.health_tracker = None resp = client.get('/api/v3/plugins/health') assert resp.status_code == 200 @@ -1034,15 +1030,15 @@ class TestPluginHealthRoutes: assert body['data'] == {} assert 'not available' in body['message'].lower() - def test_metrics_route_builds_per_installed_id(self, client, mock_plugin_manager): + def test_metrics_route_builds_per_installed_id(self, client, mock_plugin_catalog): from web_interface.blueprints.api_v3 import api_v3 from src.plugin_system.resource_monitor import PluginResourceMonitor cache = MagicMock() cache.get.return_value = None - api_v3.plugin_manager = mock_plugin_manager - mock_plugin_manager.plugin_manifests = {'p1': {}} - mock_plugin_manager.resource_monitor = PluginResourceMonitor( + api_v3.plugin_catalog = mock_plugin_catalog + mock_plugin_catalog.plugin_manifests = {'p1': {}} + api_v3.resource_monitor = PluginResourceMonitor( cache, enable_monitoring=False ) diff --git a/test/test_web_plugin_dir_resolution.py b/test/test_web_plugin_dir_resolution.py index 135a935c..aea37af3 100644 --- a/test/test_web_plugin_dir_resolution.py +++ b/test/test_web_plugin_dir_resolution.py @@ -46,8 +46,8 @@ def prefixed_plugin(tmp_path, api_v3_module): }), encoding="utf-8") api = api_v3_module.api_v3 - api.plugin_manager.plugins_dir = str(plugins_dir) - api.plugin_manager.get_plugin_directory = MagicMock(side_effect=_resolver(plugins_dir)) + api.plugin_catalog.plugins_dir = str(plugins_dir) + api.plugin_catalog.get_plugin_directory = MagicMock(side_effect=_resolver(plugins_dir)) api.plugin_store_manager.plugins_dir = str(plugins_dir) return plugin_dir @@ -58,8 +58,7 @@ class TestInstalledList: api = api_v3_module.api_v3 info = {"id": "demo", "name": "Demo", "version": "1.0.0", "description": "stale cached copy", "loaded": False} - api.plugin_manager.get_all_plugin_info = MagicMock(return_value=[info]) - api.plugin_manager.get_plugin = MagicMock(return_value=None) + api.plugin_catalog.get_all_plugin_info = MagicMock(return_value=[info]) api.plugin_store_manager.get_registry_info = MagicMock(return_value=None) api.plugin_store_manager._get_local_git_info = MagicMock(return_value=None) api.config_manager.load_config = MagicMock(return_value={}) @@ -87,7 +86,6 @@ class TestUpdateRoute: api.plugin_store_manager.get_plugin_info = MagicMock(return_value=None) api.plugin_store_manager.update_plugin = MagicMock(return_value=True) api.plugin_store_manager._get_local_git_info = MagicMock(return_value=None) - api.plugin_manager.plugins = {} api.schema_manager = None api.plugin_state_manager = None api.operation_history = None @@ -121,7 +119,6 @@ class TestUpdateRoute: api.plugin_store_manager.get_plugin_info = MagicMock(return_value=None) api.plugin_store_manager.update_plugin = MagicMock(return_value=False) api.plugin_store_manager._get_local_git_info = MagicMock(return_value=None) - api.plugin_manager.plugins = {} api.operation_history = None response = api_v3_client.post("/api/v3/plugins/update", json={"plugin_id": "ghost"}) @@ -140,8 +137,7 @@ def pages(tmp_path, monkeypatch): plugins_dir.mkdir() plugin_manager = MagicMock() plugin_manager.plugins_dir = plugins_dir - plugin_manager.get_plugin.return_value = None - monkeypatch.setattr(module.pages_v3, "plugin_manager", plugin_manager, raising=False) + monkeypatch.setattr(module.pages_v3, "plugin_catalog", plugin_manager, raising=False) monkeypatch.setattr(module.pages_v3, "config_manager", MagicMock(load_config=lambda: {}), raising=False) monkeypatch.setattr(module.pages_v3, "schema_manager", None, raising=False) diff --git a/test/test_web_settings_ui.py b/test/test_web_settings_ui.py index d6852447..bd3f2531 100644 --- a/test/test_web_settings_ui.py +++ b/test/test_web_settings_ui.py @@ -78,7 +78,7 @@ def client(): mock_cm.get_config_path.return_value = "config/config.json" mock_cm.get_secrets_path.return_value = "config/config_secrets.json" pv.pages_v3.config_manager = mock_cm - pv.pages_v3.plugin_manager = MagicMock(plugins={}) + pv.pages_v3.plugin_catalog = MagicMock() app.register_blueprint(pv.pages_v3, url_prefix="/v3") return app.test_client() diff --git a/test/test_web_smoke.py b/test/test_web_smoke.py index 5bcb37b4..6ccac074 100644 --- a/test/test_web_smoke.py +++ b/test/test_web_smoke.py @@ -82,7 +82,7 @@ def client(): # the originals and restore them on teardown so this fixture can't leak # its mocks into tests that run afterward. original_config_manager = getattr(pv.pages_v3, "config_manager", None) - original_plugin_manager = getattr(pv.pages_v3, "plugin_manager", None) + original_plugin_manager = getattr(pv.pages_v3, "plugin_catalog", None) mock_cm = MagicMock() mock_cm.load_config.return_value = SMOKE_CONFIG @@ -92,7 +92,6 @@ def client(): pv.pages_v3.config_manager = mock_cm mock_pm = MagicMock() - mock_pm.plugins = {} mock_pm.get_all_plugin_info.return_value = [ {"id": "clock", "name": "Clock"}, {"id": "ledmatrix-weather", "name": "Weather"}, @@ -100,7 +99,7 @@ def client(): mock_pm.get_plugin_display_modes.side_effect = ( lambda pid: PLUGIN_MODES.get(pid, []) ) - pv.pages_v3.plugin_manager = mock_pm + pv.pages_v3.plugin_catalog = mock_pm # Same dual registration as web_interface/app.py: un-prefixed primary, # /v3 kept as a working legacy alias. @@ -110,7 +109,7 @@ def client(): yield app.test_client() finally: pv.pages_v3.config_manager = original_config_manager - pv.pages_v3.plugin_manager = original_plugin_manager + pv.pages_v3.plugin_catalog = original_plugin_manager # (path, [markers that must appear in the body]) diff --git a/test/web_interface/test_api_v3_backup_paths.py b/test/web_interface/test_api_v3_backup_paths.py index 73dc47d1..c929e20c 100644 --- a/test/web_interface/test_api_v3_backup_paths.py +++ b/test/web_interface/test_api_v3_backup_paths.py @@ -29,7 +29,7 @@ from web_interface.blueprints import api_v3 as api_v3_module # noqa: E402 from web_interface.blueprints.api_v3 import api_v3 # noqa: E402 _MANAGER_ATTRS = ( - 'config_manager', 'plugin_manager', 'plugin_store_manager', + 'config_manager', 'plugin_catalog', 'plugin_store_manager', 'plugin_state_manager', 'saved_repositories_manager', 'schema_manager', 'operation_queue', 'operation_history', 'cache_manager', ) diff --git a/test/web_interface/test_api_v3_backup_restore.py b/test/web_interface/test_api_v3_backup_restore.py index 3a0dd2ce..db6fe38f 100644 --- a/test/web_interface/test_api_v3_backup_restore.py +++ b/test/web_interface/test_api_v3_backup_restore.py @@ -32,7 +32,7 @@ from web_interface.blueprints.api_v3 import api_v3 # noqa: E402 URL = "/api/v3/backup/restore" _MANAGER_ATTRS = ( - 'config_manager', 'plugin_manager', 'plugin_store_manager', + 'config_manager', 'plugin_catalog', 'plugin_store_manager', 'plugin_state_manager', 'saved_repositories_manager', 'schema_manager', 'operation_queue', 'operation_history', 'cache_manager', ) diff --git a/test/web_interface/test_api_v3_config_raw.py b/test/web_interface/test_api_v3_config_raw.py index ff9e0bbb..6a711d4a 100644 --- a/test/web_interface/test_api_v3_config_raw.py +++ b/test/web_interface/test_api_v3_config_raw.py @@ -43,7 +43,7 @@ def env(tmp_path): config_manager.template_path = str(tmp_path / "no-template.json") _SENTINEL = object() - attrs = ('config_manager', 'plugin_manager', 'plugin_store_manager', + attrs = ('config_manager', 'plugin_catalog', 'plugin_store_manager', 'plugin_state_manager', 'saved_repositories_manager', 'schema_manager', 'operation_queue', 'operation_history', 'cache_manager') diff --git a/test/web_interface/test_api_v3_plugin_config_save.py b/test/web_interface/test_api_v3_plugin_config_save.py index c0de008c..9c8e0405 100644 --- a/test/web_interface/test_api_v3_plugin_config_save.py +++ b/test/web_interface/test_api_v3_plugin_config_save.py @@ -93,13 +93,12 @@ def env(tmp_path, api_v3_module): api = api_v3_module.api_v3 api.config_manager = config_manager api.schema_manager = SchemaManager(plugins_dir=plugins_dir, project_root=tmp_path) - api.plugin_manager.plugin_manifests = {PLUGIN_ID: {"id": PLUGIN_ID}, + api.plugin_catalog.plugin_manifests = {PLUGIN_ID: {"id": PLUGIN_ID}, NEWS_ID: {"id": NEWS_ID}} - api.plugin_manager.get_plugin.return_value = None class Env: client = build_app(api).test_client() - plugin_manager = api.plugin_manager + plugin_catalog = api.plugin_catalog @staticmethod def stored(plugin_id=PLUGIN_ID): @@ -183,19 +182,18 @@ class TestReset: assert calls == [True] assert env.stored()["stock_symbols"] == ["AAPL"] - def test_reset_notifies_the_plugin_with_its_prepared_config(self, env): - plugin = MagicMock() - env.plugin_manager.get_plugin.return_value = plugin - env.plugin_manager.prepare_plugin_config.side_effect = ( - lambda _pid, raw: {**raw, "prepared": True}) + def test_reset_runs_no_plugin_code_in_the_web_process(self, env): + # The running plugin gets the reset config from the display's config + # watcher (on_config_change there, with the prepared section). The + # catalog has no get_plugin, so a route that still reached for a + # plugin instance here would fail this request. + assert not hasattr(env.plugin_catalog, "get_plugin") response = env.client.post("/api/v3/plugins/config/reset", json={"plugin_id": PLUGIN_ID}) assert response.status_code == 200, response.get_json() - handed_over = plugin.on_config_change.call_args.args[0] - assert handed_over["prepared"] is True - assert handed_over["stock_symbols"] == ["AAPL"] + assert env.stored()["stock_symbols"] == ["AAPL"] def test_a_failed_save_is_reported(self, env, monkeypatch): failed = MagicMock(message="disk full") diff --git a/test/web_interface/test_api_v3_secret_roundtrip.py b/test/web_interface/test_api_v3_secret_roundtrip.py index 9d07a011..3c218775 100644 --- a/test/web_interface/test_api_v3_secret_roundtrip.py +++ b/test/web_interface/test_api_v3_secret_roundtrip.py @@ -79,11 +79,10 @@ def env(tmp_path): plugin_manager = MagicMock() plugin_manager.plugin_manifests = {PLUGIN_ID: {"id": PLUGIN_ID}} plugin_manager.plugins_dir = plugins_dir - plugin_manager.get_plugin.return_value = None api_v3.config_manager = config_manager api_v3.schema_manager = schema_manager - api_v3.plugin_manager = plugin_manager + api_v3.plugin_catalog = plugin_manager api_v3.plugin_store_manager = MagicMock() api_v3.saved_repositories_manager = MagicMock() api_v3.operation_queue = MagicMock() diff --git a/test/web_interface/test_api_v3_unhandled_errors.py b/test/web_interface/test_api_v3_unhandled_errors.py index 6b90c944..067cd476 100644 --- a/test/web_interface/test_api_v3_unhandled_errors.py +++ b/test/web_interface/test_api_v3_unhandled_errors.py @@ -39,7 +39,7 @@ EXPECTED = { 'details': describe_exception(FORCED), } -MANAGERS = ("config_manager", "plugin_manager", "plugin_store_manager", +MANAGERS = ("config_manager", "plugin_catalog", "plugin_store_manager", "saved_repositories_manager", "schema_manager", "operation_queue", "plugin_state_manager", "operation_history", "cache_manager") @@ -222,7 +222,7 @@ class TestPluginActionStep1: from unittest.mock import MagicMock manager = MagicMock() manager.get_plugin_directory.return_value = str(plugin_dir) - monkeypatch.setattr(api_v3, "plugin_manager", manager, raising=False) + monkeypatch.setattr(api_v3, "plugin_catalog", manager, raising=False) app = Flask(__name__) app.register_blueprint(api_v3, url_prefix="/api/v3") diff --git a/test/web_interface/test_array_of_objects_roundtrip.py b/test/web_interface/test_array_of_objects_roundtrip.py index 5cf28be9..ac39b0aa 100644 --- a/test/web_interface/test_array_of_objects_roundtrip.py +++ b/test/web_interface/test_array_of_objects_roundtrip.py @@ -113,7 +113,7 @@ def post(tmp_path): from web_interface.blueprints import api_v3 as a originals = {k: getattr(a.api_v3, k, None) - for k in ("config_manager", "schema_manager", "plugin_manager")} + for k in ("config_manager", "schema_manager", "plugin_catalog")} pdir = tmp_path / "plugins" / "demo" pdir.mkdir(parents=True) @@ -137,9 +137,7 @@ def post(tmp_path): a.api_v3.schema_manager = SchemaManager(plugins_dir=tmp_path / "plugins", project_root=tmp_path) pm = MagicMock() - pm.plugins = {} - pm.get_plugin.return_value = None - a.api_v3.plugin_manager = pm + a.api_v3.plugin_catalog = pm app = Flask(__name__) app.config["TESTING"] = True diff --git a/test/web_interface/test_partial_save_booleans.py b/test/web_interface/test_partial_save_booleans.py index 4a4ff984..615ddb2c 100644 --- a/test/web_interface/test_partial_save_booleans.py +++ b/test/web_interface/test_partial_save_booleans.py @@ -99,7 +99,7 @@ def post(tmp_path): from web_interface.blueprints import api_v3 as a originals = {k: getattr(a.api_v3, k, None) - for k in ("config_manager", "schema_manager", "plugin_manager")} + for k in ("config_manager", "schema_manager", "plugin_catalog")} pdir = tmp_path / "plugins" / "demo" pdir.mkdir(parents=True) @@ -122,9 +122,7 @@ def post(tmp_path): a.api_v3.schema_manager = SchemaManager(plugins_dir=tmp_path / "plugins", project_root=tmp_path) pm = MagicMock() - pm.plugins = {} - pm.get_plugin.return_value = None - a.api_v3.plugin_manager = pm + a.api_v3.plugin_catalog = pm app = Flask(__name__) app.config["TESTING"] = True diff --git a/test/web_interface/test_plugin_config_form_defaults.py b/test/web_interface/test_plugin_config_form_defaults.py index 9e513191..a4eb7655 100644 --- a/test/web_interface/test_plugin_config_form_defaults.py +++ b/test/web_interface/test_plugin_config_form_defaults.py @@ -115,19 +115,17 @@ def app_client(tmp_path): cm.save_config_atomic.side_effect = _save pm = MagicMock() - pm.plugins = {} pm.plugins_dir = plugins_dir - pm.get_plugin.return_value = None pm.get_plugin_info.return_value = {"name": "Demo", "version": "1.0.0"} sm = SchemaManager(plugins_dir=plugins_dir, project_root=tmp_path) - names = ("config_manager", "schema_manager", "plugin_manager") + names = ("config_manager", "schema_manager", "plugin_catalog") originals = {(bp, k): getattr(bp, k, None) for bp in (api.api_v3, pages.pages_v3) for k in names} for bp in (api.api_v3, pages.pages_v3): bp.config_manager = cm bp.schema_manager = sm - bp.plugin_manager = pm + bp.plugin_catalog = pm base = Path(pages.__file__).resolve().parent.parent app = Flask(__name__, template_folder=str(base / "templates"), diff --git a/test/web_interface/test_plugin_config_json_saves.py b/test/web_interface/test_plugin_config_json_saves.py index 6e3e9792..2be4989e 100644 --- a/test/web_interface/test_plugin_config_json_saves.py +++ b/test/web_interface/test_plugin_config_json_saves.py @@ -72,7 +72,7 @@ STORED = { "skin_options": {"accent": "#00ff00"}, } -_ATTRS = ('config_manager', 'plugin_manager', 'plugin_store_manager', +_ATTRS = ('config_manager', 'plugin_catalog', 'plugin_store_manager', 'plugin_state_manager', 'saved_repositories_manager', 'schema_manager', 'operation_queue', 'operation_history', 'cache_manager') @@ -96,13 +96,12 @@ def env(tmp_path): plugin_manager = MagicMock() plugin_manager.plugin_manifests = {PLUGIN_ID: {"id": PLUGIN_ID}} plugin_manager.plugins_dir = plugins_dir - plugin_manager.get_plugin.return_value = None for name in _ATTRS: setattr(api_v3, name, MagicMock()) api_v3.config_manager = config_manager api_v3.schema_manager = SchemaManager(plugins_dir=plugins_dir, project_root=tmp_path) - api_v3.plugin_manager = plugin_manager + api_v3.plugin_catalog = plugin_manager api_v3.operation_queue = None app = Flask(__name__) diff --git a/test/web_interface/test_plugin_config_legacy_values.py b/test/web_interface/test_plugin_config_legacy_values.py index 94cd4179..fc458486 100644 --- a/test/web_interface/test_plugin_config_legacy_values.py +++ b/test/web_interface/test_plugin_config_legacy_values.py @@ -110,7 +110,7 @@ def post(tmp_path): from web_interface.blueprints import api_v3 as a originals = {k: getattr(a.api_v3, k, None) - for k in ("config_manager", "schema_manager", "plugin_manager")} + for k in ("config_manager", "schema_manager", "plugin_catalog")} pdir = tmp_path / "plugins" / "demo" pdir.mkdir(parents=True) @@ -134,9 +134,7 @@ def post(tmp_path): a.api_v3.schema_manager = SchemaManager(plugins_dir=tmp_path / "plugins", project_root=tmp_path) pm = MagicMock() - pm.plugins = {} - pm.get_plugin.return_value = None - a.api_v3.plugin_manager = pm + a.api_v3.plugin_catalog = pm app = Flask(__name__) app.config["TESTING"] = True diff --git a/test/web_interface/test_plugin_config_schema_expansion.py b/test/web_interface/test_plugin_config_schema_expansion.py index 42af00b9..0db80ea8 100644 --- a/test/web_interface/test_plugin_config_schema_expansion.py +++ b/test/web_interface/test_plugin_config_schema_expansion.py @@ -71,7 +71,7 @@ def render(tmp_path): from web_interface.blueprints import pages_v3 as pv from src.plugin_system.schema_manager import SchemaManager - orig_pm = getattr(pv.pages_v3, "plugin_manager", None) + orig_pm = getattr(pv.pages_v3, "plugin_catalog", None) orig_cm = getattr(pv.pages_v3, "config_manager", None) orig_sm = getattr(pv.pages_v3, "schema_manager", None) @@ -86,8 +86,7 @@ def render(tmp_path): pm = MagicMock() pm.plugins_dir = str(tmp_path / "plugins") pm.get_plugin_info.return_value = {"id": plugin_id, "name": plugin_id} - pm.get_plugin.return_value = None - pv.pages_v3.plugin_manager = pm + pv.pages_v3.plugin_catalog = pm cm = MagicMock() cm.load_config.return_value = {plugin_id: {"enabled": True}} @@ -111,7 +110,7 @@ def render(tmp_path): try: yield _render finally: - pv.pages_v3.plugin_manager = orig_pm + pv.pages_v3.plugin_catalog = orig_pm pv.pages_v3.config_manager = orig_cm pv.pages_v3.schema_manager = orig_sm diff --git a/test/web_interface/test_plugin_widget_route.py b/test/web_interface/test_plugin_widget_route.py index c8ac276e..a3dca0c1 100644 --- a/test/web_interface/test_plugin_widget_route.py +++ b/test/web_interface/test_plugin_widget_route.py @@ -52,7 +52,7 @@ def make_client(tmp_path): """ from web_interface.blueprints import pages_v3 as pv - original_pm = getattr(pv.pages_v3, "plugin_manager", None) + original_pm = getattr(pv.pages_v3, "plugin_catalog", None) def _build(plugins_dir=None, plugin_manager=_UNSET): base = PROJECT_ROOT / "web_interface" @@ -64,7 +64,7 @@ def make_client(tmp_path): if plugin_manager is _UNSET: plugin_manager = MagicMock() plugin_manager.plugins_dir = str(plugins_dir or tmp_path) - pv.pages_v3.plugin_manager = plugin_manager + pv.pages_v3.plugin_catalog = plugin_manager app.register_blueprint(pv.pages_v3, url_prefix="") return app.test_client() @@ -72,7 +72,7 @@ def make_client(tmp_path): try: yield _build finally: - pv.pages_v3.plugin_manager = original_pm + pv.pages_v3.plugin_catalog = original_pm URL = "/static/plugin-widgets/{}/{}.js" @@ -211,7 +211,7 @@ def config_form(tmp_path): """Render a plugin's config partial with a temp plugin on disk.""" from web_interface.blueprints import pages_v3 as pv - orig_pm = getattr(pv.pages_v3, "plugin_manager", None) + orig_pm = getattr(pv.pages_v3, "plugin_catalog", None) orig_cm = getattr(pv.pages_v3, "config_manager", None) def _render(plugin_id="soccer-scoreboard", schema=None, widgets=None, @@ -226,8 +226,7 @@ def config_form(tmp_path): pm.get_plugin_info.return_value = {"id": plugin_id, "name": plugin_id} if version is not None: pm.get_plugin_info.return_value["version"] = version - pm.get_plugin.return_value = None - pv.pages_v3.plugin_manager = pm + pv.pages_v3.plugin_catalog = pm cm = MagicMock() cm.load_config.return_value = {plugin_id: {"enabled": True}} @@ -244,7 +243,7 @@ def config_form(tmp_path): try: yield _render finally: - pv.pages_v3.plugin_manager = orig_pm + pv.pages_v3.plugin_catalog = orig_pm pv.pages_v3.config_manager = orig_cm diff --git a/test/web_interface/test_style_editor_extra_fields.py b/test/web_interface/test_style_editor_extra_fields.py index ad823f4c..f5cc42f2 100644 --- a/test/web_interface/test_style_editor_extra_fields.py +++ b/test/web_interface/test_style_editor_extra_fields.py @@ -64,7 +64,7 @@ def post(tmp_path): from web_interface.blueprints import api_v3 as a originals = {k: getattr(a.api_v3, k, None) - for k in ("config_manager", "schema_manager", "plugin_manager")} + for k in ("config_manager", "schema_manager", "plugin_catalog")} pdir = tmp_path / "plugins" / "demo" pdir.mkdir(parents=True) @@ -86,9 +86,7 @@ def post(tmp_path): a.api_v3.schema_manager = SchemaManager(plugins_dir=tmp_path / "plugins", project_root=tmp_path) pm = MagicMock() - pm.plugins = {} - pm.get_plugin.return_value = None - a.api_v3.plugin_manager = pm + a.api_v3.plugin_catalog = pm app = Flask(__name__) app.config["TESTING"] = True diff --git a/test/web_interface/test_style_editor_save_roundtrip.py b/test/web_interface/test_style_editor_save_roundtrip.py index 50f91a6f..bf705a98 100644 --- a/test/web_interface/test_style_editor_save_roundtrip.py +++ b/test/web_interface/test_style_editor_save_roundtrip.py @@ -62,7 +62,7 @@ def post(tmp_path): from web_interface.blueprints import api_v3 as a originals = {k: getattr(a.api_v3, k, None) - for k in ("config_manager", "schema_manager", "plugin_manager")} + for k in ("config_manager", "schema_manager", "plugin_catalog")} pdir = tmp_path / "plugins" / "demo" pdir.mkdir(parents=True) @@ -85,9 +85,7 @@ def post(tmp_path): a.api_v3.schema_manager = SchemaManager(plugins_dir=tmp_path / "plugins", project_root=tmp_path) pm = MagicMock() - pm.plugins = {} - pm.get_plugin.return_value = None - a.api_v3.plugin_manager = pm + a.api_v3.plugin_catalog = pm app = Flask(__name__) app.config["TESTING"] = True diff --git a/test/web_interface/test_update_all_plugins.py b/test/web_interface/test_update_all_plugins.py index 39386948..8d88a012 100644 --- a/test/web_interface/test_update_all_plugins.py +++ b/test/web_interface/test_update_all_plugins.py @@ -41,7 +41,7 @@ def store(tmp_path): sm.get_plugin_info.return_value = None sm.update_plugin.return_value = True with patch.object(api_v3, 'plugin_store_manager', sm, create=True), \ - patch.object(api_v3, 'plugin_manager', None, create=True), \ + patch.object(api_v3, 'plugin_catalog', None, create=True), \ patch.object(api_v3, 'schema_manager', None, create=True), \ patch.object(api_v3, 'plugin_state_manager', None, create=True), \ patch.object(api_v3, 'operation_history', None, create=True): diff --git a/test/web_interface/test_web_process_runs_no_plugin_code.py b/test/web_interface/test_web_process_runs_no_plugin_code.py new file mode 100644 index 00000000..d802a48e --- /dev/null +++ b/test/web_interface/test_web_process_runs_no_plugin_code.py @@ -0,0 +1,460 @@ +"""The web process reads plugins; only the display process runs them. + +The web interface used to build its own PluginManager and load plugins into +the web process: store installs and updates loaded or reloaded a web-side +copy, and config saves and enable/disable called on_config_change, +on_enable and on_disable on it. None of that reached the panel -- the +display process runs its own instances -- and the web then reported +"runtime" state from copies nothing displayed. An update in particular +looked applied while the display kept running the old code until it +restarted, and nothing said so. + +Now the web process has a PluginCatalog (manifests, schemas, config, +installed versions) and nothing that can run a plugin: + +- every route a user drives for a plugin works without importing the + plugin's module at all -- the plugin below records any import and any + lifecycle call to a file, and the file must never appear; +- the catalog reads what is really installed, in plugin-repos/ and in the + test fixtures; +- a store install, update or uninstall answers ``restart_required`` exactly + when the running display will not pick the change up by itself. +""" + +import json +import shutil +import sys +from pathlib import Path +from unittest.mock import MagicMock + +import pytest +from flask import Flask + +PROJECT_ROOT = Path(__file__).resolve().parents[2] +sys.path.insert(0, str(PROJECT_ROOT)) + +from src.config_manager import ConfigManager # noqa: E402 +from src.plugin_system.plugin_catalog import ( # noqa: E402 + PluginCatalog, display_restart_required, +) +from src.plugin_system.schema_manager import SchemaManager # noqa: E402 +from test._api_v3_test_helpers import api_v3_module # noqa: F401,E402 + +PLUGIN_ID = "tripwire" + +# A plugin that leaves evidence of being run: importing its module, building +# it, or calling any lifecycle hook appends a line to TRIPWIRE_LOG. +MANAGER_PY = ''' +import os +LOG = os.environ.get("TRIPWIRE_LOG") or {log!r} + +def _note(what): + with open(LOG, "a", encoding="utf-8") as f: + f.write(what + "\\n") + +_note("imported") + +from src.plugin_system.base_plugin import BasePlugin + + +class TripwirePlugin(BasePlugin): + def __init__(self, *args, **kwargs): + _note("instantiated") + super().__init__(*args, **kwargs) + + def update(self): + _note("update") + + def display(self, force_clear=False): + _note("display") + + def on_config_change(self, new_config): + _note("on_config_change") + + def on_enable(self): + _note("on_enable") + + def on_disable(self): + _note("on_disable") +''' + +SCHEMA = { + "type": "object", + "properties": { + "enabled": {"type": "boolean", "default": False}, + "message": {"type": "string", "default": "hello"}, + }, +} + + +def _write_plugin(plugins_dir, log, version="1.0.0", plugin_id=PLUGIN_ID): + plugin_dir = plugins_dir / plugin_id + plugin_dir.mkdir(parents=True, exist_ok=True) + (plugin_dir / "manifest.json").write_text(json.dumps({ + "id": plugin_id, "name": "Tripwire", "version": version, + "entry_point": "manager.py", "class_name": "TripwirePlugin", + "display_modes": ["tripwire"], + }), encoding="utf-8") + (plugin_dir / "config_schema.json").write_text(json.dumps(SCHEMA), encoding="utf-8") + (plugin_dir / "manager.py").write_text(MANAGER_PY.format(log=str(log)), encoding="utf-8") + return plugin_dir + + +class Web: + """The api_v3 and pages_v3 blueprints over a real catalog and config.""" + + def __init__(self, api_v3_module, tmp_path, monkeypatch, enabled=True): + self.log = tmp_path / "tripwire.log" + monkeypatch.setenv("TRIPWIRE_LOG", str(self.log)) + self.plugins_dir = tmp_path / "plugin-repos" + self.plugins_dir.mkdir() + _write_plugin(self.plugins_dir, self.log) + + self.config_file = tmp_path / "config.json" + self.config_file.write_text(json.dumps( + {PLUGIN_ID: {"enabled": enabled, "message": "hi"}}), encoding="utf-8") + self.config_manager = ConfigManager( + config_path=str(self.config_file), + secrets_path=str(tmp_path / "config_secrets.json")) + self.config_manager.template_path = str(tmp_path / "no-template.json") + self.schema_manager = SchemaManager(plugins_dir=self.plugins_dir, project_root=tmp_path, + config_manager=self.config_manager) + self.catalog = PluginCatalog(self.plugins_dir, self.config_manager, self.schema_manager) + + api = self.api = api_v3_module.api_v3 + api.config_manager = self.config_manager + api.schema_manager = self.schema_manager + api.plugin_catalog = self.catalog + store = api.plugin_store_manager + store.plugins_dir = str(self.plugins_dir) + store.get_registry_info.return_value = None + store.get_plugin_info.return_value = None + store._get_local_git_info.return_value = None + store.install_plugin.return_value = True + store.uninstall_plugin.return_value = True + store.update_plugin.return_value = True + + from web_interface.blueprints import pages_v3 as pages_module + pages = pages_module.pages_v3 + for name, value in (("config_manager", self.config_manager), + ("schema_manager", self.schema_manager), + ("plugin_catalog", self.catalog)): + monkeypatch.setattr(pages, name, value, raising=False) + + app = Flask(__name__, template_folder=str(PROJECT_ROOT / "web_interface" / "templates")) + app.config["TESTING"] = True + app.register_blueprint(api, url_prefix="/api/v3") + app.register_blueprint(pages, url_prefix="") + self.client = app.test_client() + + def ran(self): + """What the plugin recorded: [] when none of its code ever ran.""" + return self.log.read_text(encoding="utf-8").split() if self.log.exists() else [] + + def stored(self): + return json.loads(self.config_file.read_text(encoding="utf-8")).get(PLUGIN_ID, {}) + + def post(self, url, body): + response = self.client.post(url, json=body) + assert response.status_code == 200, (url, response.get_json()) + return response.get_json() + + +@pytest.fixture +def web(api_v3_module, tmp_path, monkeypatch): + return Web(api_v3_module, tmp_path, monkeypatch) + + +@pytest.fixture +def disabled_web(api_v3_module, tmp_path, monkeypatch): + return Web(api_v3_module, tmp_path, monkeypatch, enabled=False) + + +def _bump_version(web, version): + """What a store update does to the files on disk.""" + def update(plugin_id): + _write_plugin(web.plugins_dir, web.log, version=version, plugin_id=plugin_id) + return True + web.api.plugin_store_manager.update_plugin.side_effect = update + + +class TestTheWebProcessNeverRunsAPlugin: + """Every plugin route, against a plugin that reports being run.""" + + def test_the_tripwire_works(self, web, tmp_path): + # Sanity: importing the module does leave the mark this suite checks for. + import importlib.util + spec = importlib.util.spec_from_file_location( + "tripwire_probe", web.plugins_dir / PLUGIN_ID / "manager.py") + spec.loader.exec_module(importlib.util.module_from_spec(spec)) + assert web.ran() == ["imported"] + + def test_listing_the_installed_plugins(self, web): + body = web.client.get("/api/v3/plugins/installed").get_json() + entry = next(p for p in body["data"]["plugins"] if p["id"] == PLUGIN_ID) + assert entry["version"] == "1.0.0" + assert entry["enabled"] is True + # Not published by the display, so not invented here. + assert entry["loaded"] is None and entry["state"] is None + # Nothing in its files declares a participation; the display derives + # one from its hooks, which are not called here. + assert (entry["vegas_participation"], entry["vegas_participation_source"]) == ( + None, "runtime") + assert web.ran() == [] + + def test_enabling_and_disabling(self, web): + web.post("/api/v3/plugins/toggle", {"plugin_id": PLUGIN_ID, "enabled": False}) + assert web.stored()["enabled"] is False + web.post("/api/v3/plugins/toggle", {"plugin_id": PLUGIN_ID, "enabled": True}) + assert web.stored()["enabled"] is True + assert web.ran() == [] + + def test_saving_its_config(self, web): + web.post("/api/v3/plugins/config", + {"plugin_id": PLUGIN_ID, "config": {"message": "changed"}}) + assert web.stored()["message"] == "changed" + assert web.ran() == [] + + def test_saving_its_section_through_the_main_config(self, web): + body = web.post("/api/v3/config/main", {PLUGIN_ID: {"message": "via main"}}) + assert web.stored()["message"] == "via main" + assert body["restart_required"] is True + assert web.ran() == [] + + def test_resetting_its_config(self, web): + web.post("/api/v3/plugins/config/reset", {"plugin_id": PLUGIN_ID}) + assert web.stored()["message"] == "hello" + assert web.ran() == [] + + def test_rendering_its_settings_page(self, web): + response = web.client.get(f"/partials/plugin-config/{PLUGIN_ID}") + assert response.status_code == 200 + assert "Tripwire" in response.get_data(as_text=True) + assert web.ran() == [] + + def test_updating_it(self, web): + _bump_version(web, "1.1.0") + body = web.post("/api/v3/plugins/update", {"plugin_id": PLUGIN_ID}) + assert body["data"]["update_status"] == "updated" + assert web.catalog.get_installed_version(PLUGIN_ID) == "1.1.0" + assert web.ran() == [] + + def test_installing_it(self, web): + web.post("/api/v3/plugins/install", {"plugin_id": PLUGIN_ID}) + assert PLUGIN_ID in web.catalog.plugin_manifests + assert web.ran() == [] + + def test_uninstalling_it(self, web): + web.post("/api/v3/plugins/uninstall", {"plugin_id": PLUGIN_ID}) + web.api.plugin_store_manager.uninstall_plugin.assert_called_once_with(PLUGIN_ID) + assert web.ran() == [] + + def test_no_plugin_module_is_registered_in_this_process(self, web): + web.client.get("/api/v3/plugins/installed") + web.post("/api/v3/plugins/toggle", {"plugin_id": PLUGIN_ID, "enabled": True}) + assert not [name for name, mod in list(sys.modules.items()) + if str(web.plugins_dir) in str(getattr(mod, "__file__", "") or "")] + + def test_the_web_app_has_a_catalog_and_no_plugin_manager(self): + """app.py wires a PluginCatalog, and nothing named like a manager.""" + source = (PROJECT_ROOT / "web_interface" / "app.py").read_text(encoding="utf-8") + assert "PluginCatalog(" in source + assert "PluginManager" not in source + + +class TestNoLifecycleCallsInWebCode: + """A static backstop for the behavioural tests above: the web process's + own code has no call that runs a plugin. Comments may mention the hooks; + calls may not.""" + + FORBIDDEN = (".on_enable(", ".on_disable(", ".on_config_change(", + ".load_plugin(", ".unload_plugin(", ".reload_plugin(", + ".get_plugin(", ".update(force", ".display(force_clear", + # Vegas participation past config and manifest calls the + # plugin's hooks; the installed route reports it from files. + "resolve_vegas_participation(", "legacy_vegas_participation(", + ".get_vegas_participation(") + + def _code_lines(self): + for path in sorted((PROJECT_ROOT / "web_interface").rglob("*.py")): + for number, line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1): + code = line.split("#", 1)[0] + yield path.relative_to(PROJECT_ROOT), number, code + + def test_no_web_module_calls_a_plugin_lifecycle_hook(self): + offenders = [f"{path}:{number}: {code.strip()}" + for path, number, code in self._code_lines() + if any(call in code for call in self.FORBIDDEN)] + assert offenders == [] + + def test_plugin_code_is_imported_in_exactly_one_place(self): + """exec_module in the web blueprints happens only in the seam named + for it (Starlark helpers and oauth_flow action scripts).""" + blueprints = PROJECT_ROOT / "web_interface" / "blueprints" + hits = [f"{path.name}:{n}" for path in sorted(blueprints.rglob("*.py")) + for n, line in enumerate(path.read_text(encoding="utf-8").splitlines(), 1) + if "exec_module(" in line.split("#", 1)[0]] + assert len(hits) == 1, hits + source = (blueprints / "api_v3" / "__init__.py").read_text(encoding="utf-8") + seam = source.index("def _import_plugin_code_in_web_process(") + assert source.index("exec_module(", seam) < source.index("\ndef ", seam + 1) + + +class TestStoreOperationsReachTheDisplay: + """Each answer says whether the display needs a restart to show it.""" + + def test_an_update_of_a_plugin_the_display_runs_asks_for_a_restart(self, web): + _bump_version(web, "2.0.0") + body = web.post("/api/v3/plugins/update", {"plugin_id": PLUGIN_ID}) + assert body["restart_required"] is True + assert "restart the display" in body["restart_message"] + + def test_an_update_of_a_disabled_plugin_does_not(self, disabled_web): + _bump_version(disabled_web, "2.0.0") + body = disabled_web.post("/api/v3/plugins/update", {"plugin_id": PLUGIN_ID}) + assert body["data"]["update_status"] == "updated" + assert body["restart_required"] is False + assert "restart_message" not in body + + def test_an_update_that_changed_nothing_does_not(self, web): + body = web.post("/api/v3/plugins/update", {"plugin_id": PLUGIN_ID}) + assert body["data"]["update_status"] == "up_to_date" + assert body["restart_required"] is False + + def test_installing_a_plugin_config_already_enables_asks_for_a_restart(self, web): + body = web.post("/api/v3/plugins/install", {"plugin_id": PLUGIN_ID}) + assert body["restart_required"] is True + + def test_installing_a_disabled_plugin_does_not(self, disabled_web): + # Enabling it later loads it live, from disk, in the display. + body = disabled_web.post("/api/v3/plugins/install", {"plugin_id": PLUGIN_ID}) + assert body["restart_required"] is False + + def test_the_queued_install_carries_the_flag_in_the_operation_result(self, web): + results = [] + + def enqueue(operation_type, plugin_id, operation_callback=None): + results.append(operation_callback(MagicMock())) + return "op-1" + + web.api.operation_queue = MagicMock() + web.api.operation_queue.enqueue_operation.side_effect = enqueue + web.post("/api/v3/plugins/install", {"plugin_id": PLUGIN_ID}) + assert results[0]["success"] is True + assert results[0]["restart_required"] is True + + def test_install_from_url_carries_the_flag(self, web): + web.api.plugin_store_manager.install_from_url.return_value = { + "success": True, "plugin_id": PLUGIN_ID} + body = web.post("/api/v3/plugins/install-from-url", {"repo_url": "https://example.invalid/r"}) + assert body["restart_required"] is True + + def test_uninstall_that_removes_the_config_needs_no_restart(self, web): + # The removed section reaches the display's config watcher, and its + # reconcile unloads the plugin. + body = web.post("/api/v3/plugins/uninstall", {"plugin_id": PLUGIN_ID}) + assert body["restart_required"] is False + assert PLUGIN_ID not in json.loads(web.config_file.read_text(encoding="utf-8")) + + def test_uninstall_that_keeps_an_enabled_config_asks_for_a_restart(self, web): + body = web.post("/api/v3/plugins/uninstall", + {"plugin_id": PLUGIN_ID, "preserve_config": True}) + assert body["restart_required"] is True + + @pytest.mark.parametrize("action,enabled,kwargs,expected", [ + ("install", False, {}, False), + ("install", True, {}, True), + ("update", True, {"changed": True}, True), + ("update", True, {"changed": False}, False), + ("update", False, {"changed": True}, False), + ("uninstall", True, {}, False), + ("uninstall", True, {"preserve_config": True}, True), + ("uninstall", False, {"preserve_config": True}, False), + ]) + def test_the_rule(self, action, enabled, kwargs, expected): + assert display_restart_required(action, enabled, **kwargs) is expected + + def test_an_unknown_action_is_a_programming_error(self): + with pytest.raises(ValueError): + display_restart_required("reinstall", True) + + +class TestCatalogReadsWhatIsInstalled: + """Real plugin directories: the repository's plugin-repos/ and fixtures.""" + + @staticmethod + def _manifests(root): + found = {} + for child in sorted(Path(root).iterdir()): + manifest = child / "manifest.json" + if child.is_dir() and manifest.exists(): + data = json.loads(manifest.read_text(encoding="utf-8")) + if isinstance(data, dict) and data.get("id"): + found[data["id"]] = (child, data) + return found + + @pytest.mark.parametrize("root", [ + PROJECT_ROOT / "plugin-repos", + PROJECT_ROOT / "test" / "fixtures" / "plugins", + ], ids=["plugin-repos", "fixtures"]) + def test_every_installed_plugin_is_read_from_its_files(self, root): + expected = self._manifests(root) + assert expected, f"no plugins under {root}" + before = set(sys.modules) + schema_manager = SchemaManager(plugins_dir=root, project_root=PROJECT_ROOT) + catalog = PluginCatalog(root, schema_manager=schema_manager) + + assert set(catalog.discover_plugins()) == set(expected) + for plugin_id, (plugin_dir, manifest) in expected.items(): + assert catalog.get_manifest(plugin_id) == manifest + assert catalog.get_plugin_directory(plugin_id) == str(plugin_dir) + assert catalog.get_installed_version(plugin_id) == manifest.get("version", "") + assert catalog.get_plugin_display_modes(plugin_id) == manifest.get("display_modes", []) + if (plugin_dir / "config_schema.json").exists(): + schema = catalog.get_schema(plugin_id, use_cache=False) + assert isinstance(schema, dict) and "properties" in schema, plugin_id + + imported = [name for name in set(sys.modules) - before + if str(root) in str(getattr(sys.modules[name], "__file__", "") or "")] + assert imported == [], "reading the catalog imported plugin code" + + def test_a_mode_finds_its_plugin(self): + catalog = PluginCatalog(PROJECT_ROOT / "test" / "fixtures" / "plugins") + catalog.discover_plugins() + assert catalog.find_plugin_for_mode("CI-Fixture") == "ci-fixture-plugin" + assert catalog.find_plugin_for_mode("nope") is None + + def test_discovery_follows_the_directory(self, tmp_path): + plugins = tmp_path / "plugin-repos" + root = PROJECT_ROOT / "test" / "fixtures" / "plugins" / "ci-fixture-plugin" + shutil.copytree(root, plugins / "ledmatrix-ci-fixture-plugin") + (plugins / "broken").mkdir() + (plugins / "broken" / "manifest.json").write_text("[]", encoding="utf-8") + catalog = PluginCatalog(plugins) + + assert catalog.discover_plugins() == ["ci-fixture-plugin"] + # Installed as ledmatrix-, found by its manifest id. + assert catalog.get_plugin_directory("ci-fixture-plugin").endswith("ledmatrix-ci-fixture-plugin") + # Not a plain name: refused, never joined onto the plugins directory. + assert catalog.get_plugin_directory("../plugin-repos") is None + + shutil.rmtree(plugins / "ledmatrix-ci-fixture-plugin") + assert catalog.discover_plugins() == [] + assert catalog.get_manifest("ci-fixture-plugin") is None + + def test_enabled_follows_the_display_rule(self, tmp_path): + config = MagicMock() + config.load_config.return_value = {"a": {"enabled": True}, "b": {}, "c": "junk"} + catalog = PluginCatalog(tmp_path, config_manager=config) + assert catalog.is_enabled("a") is True + # The display runs a plugin only when its section says so. + assert catalog.is_enabled("b") is False + assert catalog.is_enabled("c") is False + assert catalog.is_enabled("missing") is False + assert catalog.get_config("c") == {} + + def test_it_has_nothing_that_runs_a_plugin(self, tmp_path): + catalog = PluginCatalog(tmp_path) + for name in ("load_plugin", "unload_plugin", "reload_plugin", "get_plugin", + "plugins", "run_scheduled_updates"): + assert not hasattr(catalog, name), name diff --git a/test/web_interface/test_x_display_hidden.py b/test/web_interface/test_x_display_hidden.py index 914540bf..58756b23 100644 --- a/test/web_interface/test_x_display_hidden.py +++ b/test/web_interface/test_x_display_hidden.py @@ -189,7 +189,7 @@ def _make_post(tmp_path, schema, stored, plugin_id="demo"): from web_interface.blueprints import api_v3 as a originals = {k: getattr(a.api_v3, k, None) - for k in ("config_manager", "schema_manager", "plugin_manager")} + for k in ("config_manager", "schema_manager", "plugin_catalog")} pdir = tmp_path / "plugins" / plugin_id pdir.mkdir(parents=True) @@ -213,9 +213,7 @@ def _make_post(tmp_path, schema, stored, plugin_id="demo"): a.api_v3.schema_manager = SchemaManager(plugins_dir=tmp_path / "plugins", project_root=tmp_path) pm = MagicMock() - pm.plugins = {} - pm.get_plugin.return_value = None - a.api_v3.plugin_manager = pm + a.api_v3.plugin_catalog = pm app = Flask(__name__) app.config["TESTING"] = True diff --git a/web_interface/app.py b/web_interface/app.py index 9758cebb..267307bc 100644 --- a/web_interface/app.py +++ b/web_interface/app.py @@ -34,7 +34,7 @@ from src.common.path_safety import ( ) from werkzeug.exceptions import HTTPException from src.exceptions import ConfigError -from src.plugin_system.plugin_manager import PluginManager +from src.plugin_system.plugin_catalog import PluginCatalog from src.plugin_system.store_manager import PluginStoreManager from src.plugin_system.saved_repositories import SavedRepositoriesManager from src.plugin_system.schema_manager import SchemaManager @@ -117,12 +117,6 @@ else: # If relative, resolve relative to the project root plugins_dir = project_root / plugins_dir_name -plugin_manager = PluginManager( - plugins_dir=str(plugins_dir), - config_manager=config_manager, - display_manager=None, # Not needed for web interface - cache_manager=None # Not needed for web interface -) plugin_store_manager = PluginStoreManager(plugins_dir=str(plugins_dir)) # A core `git pull` update (or any checkout) restores built-in plugins # committed under plugin-repos/, even ones the user uninstalled. Re-remove any @@ -149,6 +143,18 @@ schema_manager = SchemaManager( config_manager=config_manager ) +# The web process reads plugins as files and never runs them: no plugin module +# is imported, no plugin class instantiated, no lifecycle hook called here. +# Only the display process (src/display_controller.py) does that. Config +# saves reach the running plugins through the display's config watcher; what +# the display knows at run time (health, metrics, errors, current mode) it +# publishes to the shared cache. See docs/ARCHITECTURE.md. +plugin_catalog = PluginCatalog( + plugins_dir=plugins_dir, + config_manager=config_manager, + schema_manager=schema_manager, +) + # Initialize operation queue for plugin operations operation_queue = PluginOperationQueue(max_history=500) @@ -169,7 +175,7 @@ operation_history = OperationHistory( ) # Plugin discovery is deferred until first API request that needs it -# This improves startup time - endpoints will call discover_plugins() when needed +# This improves startup time - endpoints call plugin_catalog.discover_plugins() when needed # Register blueprints from web_interface.blueprints.pages_v3 import pages_v3 @@ -177,13 +183,13 @@ from web_interface.blueprints.api_v3 import api_v3 # Initialize managers in blueprints pages_v3.config_manager = config_manager -pages_v3.plugin_manager = plugin_manager +pages_v3.plugin_catalog = plugin_catalog pages_v3.plugin_store_manager = plugin_store_manager pages_v3.saved_repositories_manager = saved_repositories_manager pages_v3.schema_manager = schema_manager api_v3.config_manager = config_manager -api_v3.plugin_manager = plugin_manager +api_v3.plugin_catalog = plugin_catalog api_v3.plugin_store_manager = plugin_store_manager api_v3.saved_repositories_manager = saved_repositories_manager api_v3.schema_manager = schema_manager @@ -194,17 +200,19 @@ api_v3.operation_history = operation_history from src.cache_manager import CacheManager api_v3.cache_manager = CacheManager() -# Wire plugin health/metrics for the web process. The display service records -# health and execution-time metrics to the shared on-disk cache; giving the web -# process its own tracker/monitor backed by that same cache lets the health API -# routes (/api/v3/plugins/health, /plugins/metrics) read that persisted data. +# Plugin health and metrics as the display publishes them. The display service +# records health and execution-time metrics to the shared on-disk cache; a +# tracker/monitor backed by that same cache lets the health API routes +# (/api/v3/plugins/health, /plugins/metrics) read what it wrote. # Guarded so any init failure degrades to "not available" rather than breaking # the web server. +api_v3.health_tracker = None +api_v3.resource_monitor = None try: from src.plugin_system.plugin_health import PluginHealthTracker from src.plugin_system.resource_monitor import PluginResourceMonitor - plugin_manager.health_tracker = PluginHealthTracker(api_v3.cache_manager) - plugin_manager.resource_monitor = PluginResourceMonitor(api_v3.cache_manager) + api_v3.health_tracker = PluginHealthTracker(api_v3.cache_manager) + api_v3.resource_monitor = PluginResourceMonitor(api_v3.cache_manager) except Exception as _hm_err: # pragma: no cover - defensive startup guard logging.getLogger(__name__).warning( "Could not enable plugin health/metrics for web UI: %s", _hm_err @@ -960,7 +968,7 @@ def _run_startup_reconciliation() -> None: reconciler = StateReconciliation( state_manager=plugin_state_manager, config_manager=config_manager, - plugin_manager=plugin_manager, + plugin_manager=plugin_catalog, plugins_dir=plugins_dir, store_manager=plugin_store_manager ) @@ -968,7 +976,7 @@ def _run_startup_reconciliation() -> None: if result.inconsistencies_found: _logger.info("[Reconciliation] %s", result.message) if result.inconsistencies_fixed: - plugin_manager.discover_plugins() + plugin_catalog.discover_plugins() if not result.reconciliation_successful: _logger.warning( "[Reconciliation] Finished with %d unresolved issue(s); " @@ -1043,7 +1051,7 @@ def start_auto_update_scheduler(): config_manager=config_manager, core_update=perform_core_update, store_manager=plugin_store_manager, - plugin_manager=plugin_manager, + plugin_catalog=plugin_catalog, schema_manager=schema_manager, operation_history=operation_history, ) diff --git a/web_interface/auto_update.py b/web_interface/auto_update.py index 1ac3085a..1a9012be 100644 --- a/web_interface/auto_update.py +++ b/web_interface/auto_update.py @@ -329,7 +329,7 @@ class AutoUpdater: """Decides when an automatic update is due and runs it.""" def __init__(self, config_manager, core_update, store_manager=None, - plugin_manager=None, schema_manager=None, operation_history=None, + plugin_catalog=None, schema_manager=None, operation_history=None, project_root=PROJECT_ROOT, state_file=None, clock=time.time, restart=restart_service, run=subprocess.run, service_active=_service_active, helper_ready=helper_ready, @@ -337,7 +337,10 @@ class AutoUpdater: self.config_manager = config_manager self.core_update = core_update self.store_manager = store_manager - self.plugin_manager = plugin_manager + # The web process's PluginCatalog: rescanned after plugin updates. + # The display picks the new code up when _run_deferred_plugins + # restarts it. + self.plugin_catalog = plugin_catalog self.schema_manager = schema_manager self.operation_history = operation_history self.project_root = Path(project_root) @@ -658,9 +661,9 @@ class AutoUpdater: for plugin_id in updated: if self.schema_manager: self.schema_manager.invalidate_cache(plugin_id) - if updated and self.plugin_manager: + if updated and self.plugin_catalog: try: - self.plugin_manager.discover_plugins() + self.plugin_catalog.discover_plugins() except Exception: logger.debug("discover_plugins after auto-update failed", exc_info=True) return updated, failed diff --git a/web_interface/blueprints/api_v3/__init__.py b/web_interface/blueprints/api_v3/__init__.py index c951bb39..6638fa7f 100644 --- a/web_interface/blueprints/api_v3/__init__.py +++ b/web_interface/blueprints/api_v3/__init__.py @@ -91,8 +91,9 @@ def _scrub_git_remote_url(url: str) -> str: pass return url # NOTE: the managers live on the blueprint object (app.py sets -# api_v3.config_manager, api_v3.plugin_manager, api_v3.cache_manager and -# the rest). Deliberately not mirrored as module globals: a bare +# api_v3.config_manager, api_v3.plugin_catalog, api_v3.cache_manager and +# the rest). There is no plugin manager: the web process reads plugins +# through a PluginCatalog and never runs them (docs/ARCHITECTURE.md). Deliberately not mirrored as module globals: a bare # `config_manager` used to resolve to a None that was never assigned, which # silently disabled the /health checks and made /display/current fall back # to a hardcoded 128x64. @@ -631,7 +632,7 @@ def _non_plugin_id_error(plugin_id): status_code=400) return None def _discovered_plugin_manifests(plugin_id=None, rescan=False): - """The plugin manager's manifests, discovering plugins first if needed. + """The plugin catalog's manifests, discovering plugins first if needed. The web process discovers plugins lazily (see app.py): nothing scans at startup, so plugin_manifests is empty until some endpoint calls @@ -646,18 +647,18 @@ def _discovered_plugin_manifests(plugin_id=None, rescan=False): Otherwise the existing map is reused, so a steady stream of requests for known plugins costs nothing. - Returns the manifest map, or {} when there is no plugin manager. + Returns the manifest map, or {} when there is no plugin catalog. """ - pm = api_v3.plugin_manager - if pm is None: + catalog = getattr(api_v3, 'plugin_catalog', None) + if catalog is None: return {} - manifests = getattr(pm, 'plugin_manifests', None) + manifests = getattr(catalog, 'plugin_manifests', None) if not manifests or rescan or (plugin_id is not None and plugin_id not in manifests): try: - pm.discover_plugins() + catalog.discover_plugins() except Exception: logger.warning('Plugin discovery failed', exc_info=True) - manifests = getattr(pm, 'plugin_manifests', None) + manifests = getattr(catalog, 'plugin_manifests', None) return manifests or {} def _do_transactional_uninstall(plugin_id, preserve_config): """Execute an uninstall with snapshot-based rollback. @@ -665,12 +666,13 @@ def _do_transactional_uninstall(plugin_id, preserve_config): Order of operations: 1. Snapshot main config + secrets (abort on unexpected errors, proceed on expected I/O errors). 2. Clean up plugin config (abort with 500 if this raises — avoids orphaned files). - 3. Unload plugin from runtime if loaded (rollback + 500 if this raises). - 4. Remove plugin files (rollback + 500 if this returns False or raises). - 5. Finish (remove state, invalidate caches). + 3. Remove plugin files (rollback + 500 if this returns False or raises). + 4. Finish (remove state, invalidate caches). - Rollback restores the config snapshot and, if the plugin had been - loaded before unload, calls load_plugin to restore runtime state. + Rollback restores the config snapshot. Nothing is unloaded here: the web + process never loaded the plugin. The display unloads it when the removed + config section reaches its config watcher; plugin_catalog's + display_restart_required() covers the case where that doesn't happen. Returns (True, None) on success or (False, error_message) on failure. """ @@ -692,13 +694,7 @@ def _do_transactional_uninstall(plugin_id, preserve_config): if not preserve_config: api_v3.config_manager.cleanup_plugin_config(plugin_id, remove_secrets=True) - # Record whether the plugin was running before we touch anything. - was_loaded = ( - api_v3.plugin_manager is not None - and plugin_id in api_v3.plugin_manager.plugins - ) - - def _rollback(reload_plugin): + def _rollback(): if main_snapshot is not None: try: api_v3.config_manager.save_raw_file_content('main', main_snapshot) @@ -709,32 +705,19 @@ def _do_transactional_uninstall(plugin_id, preserve_config): api_v3.config_manager.save_raw_file_content('secrets', secrets_snapshot) except Exception as restore_err: logger.error("Failed to restore secrets snapshot for %s: %s", plugin_id, restore_err) - if reload_plugin and api_v3.plugin_manager is not None: - try: - api_v3.plugin_manager.load_plugin(plugin_id) - except Exception as reload_err: - logger.error("Failed to reload plugin %s during rollback: %s", plugin_id, reload_err) - # --- Step 3: unload --- - if was_loaded: - try: - api_v3.plugin_manager.unload_plugin(plugin_id) - except Exception as unload_err: - _rollback(reload_plugin=False) # unload failed — runtime state unchanged - return False, f"Failed to unload plugin {plugin_id}: {unload_err}" - - # --- Step 4: remove files --- + # --- Step 3: remove files --- try: success = api_v3.plugin_store_manager.uninstall_plugin(plugin_id) except Exception as remove_err: - _rollback(reload_plugin=was_loaded) + _rollback() return False, f"Failed to remove plugin {plugin_id}: {remove_err}" if not success: - _rollback(reload_plugin=was_loaded) + _rollback() return False, f"Failed to uninstall plugin {plugin_id}" - # --- Step 5: finish --- + # --- Step 4: finish --- if api_v3.schema_manager: api_v3.schema_manager.invalidate_cache(plugin_id) if api_v3.plugin_state_manager: @@ -747,6 +730,43 @@ def _do_transactional_uninstall(plugin_id, preserve_config): except Exception as record_err: logger.warning("Could not record uninstall for %s: %s", plugin_id, record_err) return True, None +def _plugin_enabled_in_config(plugin_id: str) -> bool: + """Whether config.json enables ``plugin_id``, by the display's rule. + + Read this before an operation changes the config (uninstall removes the + section). A config that cannot be read counts as enabled, so the answer + errs towards asking for a restart. + """ + try: + section = (api_v3.config_manager.load_config() or {}).get(plugin_id) + except Exception: + logger.debug("Could not read config for %s", plugin_id, exc_info=True) + return True + return isinstance(section, dict) and bool(section.get('enabled', False)) + + +_RESTART_MESSAGES = { + 'install': 'Plugin installed — restart the display to start it', + 'update': 'Plugin updated — restart the display to run the new version', + 'uninstall': 'Plugin uninstalled — restart the display to stop it', +} + + +def _store_restart_fields(action: str, plugin_enabled: bool, **kwargs) -> Dict[str, Any]: + """``restart_required`` (and the banner's wording) for a store response. + + The rules are ``display_restart_required``'s: whether the display picks + the change up by itself or keeps running what it has until a restart. + The UI shows its restart banner when ``restart_required`` is true. + """ + from src.plugin_system.plugin_catalog import display_restart_required + required = display_restart_required(action, plugin_enabled, **kwargs) + fields: Dict[str, Any] = {'restart_required': required} + if required: + fields['restart_message'] = _RESTART_MESSAGES[action] + return fields + + def deep_merge(base_dict, update_dict): """ Deep merge update_dict into base_dict. @@ -1346,24 +1366,6 @@ def _enhance_schema_with_core_properties(schema): return with_core_plugin_properties(schema) -def _prepared_plugin_config(plugin_id, raw_config): - """A plugin's config section as the plugin runs with it, for on_config_change. - - Loading a plugin reads legacy booleans as objects and fills in schema - defaults (PluginManager.prepare_plugin_config); a save's notification must - hand over the same shape. Falls back to the raw section. - """ - prepare = getattr(api_v3.plugin_manager, 'prepare_plugin_config', None) - if callable(prepare): - try: - prepared = prepare(plugin_id, raw_config) - if isinstance(prepared, dict): - return prepared - except Exception: - logger.debug("Could not prepare config for %s", plugin_id, exc_info=True) - return raw_config - - def _filter_config_by_schema(config, schema, prefix=''): """ Filter config to only include fields defined in the schema. @@ -1423,15 +1425,15 @@ _CALENDAR_LIST_MAX_PAGES = 10 def _plugin_directory(plugin_id: str) -> Optional[Path]: """An installed plugin's directory, or None when it has none on disk. - Only the plugin manager is asked, so no plugin manager means None. There - is no fallback to the legacy plugins/ directory: the loader never scans - it, so a plugin found only there is one that never runs. + Only the plugin catalog is asked, so no catalog means None. There is no + fallback to the legacy plugins/ directory: the loader never scans it, so + a plugin found only there is one that never runs. """ - # getattr: the blueprint only has plugin_manager once the app has set it. - manager = getattr(api_v3, 'plugin_manager', None) - if not manager: + # getattr: the blueprint only has plugin_catalog once the app has set it. + catalog = getattr(api_v3, 'plugin_catalog', None) + if not catalog: return None - plugin_dir = manager.get_plugin_directory(plugin_id) + plugin_dir = catalog.get_plugin_directory(plugin_id) if not plugin_dir or not Path(plugin_dir).exists(): return None return Path(plugin_dir) @@ -1533,10 +1535,22 @@ _STARLARK_MANIFEST_FILE = _STARLARK_APPS_DIR / 'manifest.json' # A dedicated, never-replaced file to flock -- see _starlark_manifest_lock. _STARLARK_MANIFEST_LOCK_FILE = _STARLARK_APPS_DIR / 'manifest.json.lock' def _get_starlark_plugin() -> Optional[Any]: - """Get the starlark-apps plugin instance, or None.""" - if not api_v3.plugin_manager: - return None - return api_v3.plugin_manager.get_plugin('starlark-apps') + """The starlark-apps plugin instance in this process: always None. + + The web process runs no plugin code (see PluginCatalog), so every + Starlark route takes its standalone path -- starlark-apps/manifest.json + and each app's files on disk, rendered through Pixlet directly -- and the + display's own starlark-apps plugin reads what they write. Before, this + returned a web-side copy only in the rare session that had just + installed or updated starlark-apps from the store, and that copy's + frames and state never reached the panel. + + This is the one seam where a Starlark route would reach a plugin + instance. The instance branches behind it stay until the plugin + web-entry contract (docs/ARCHITECTURE.md) gives plugins an explicit way + to serve web requests; the tests drive them through this function. + """ + return None def _find_pixlet_binary(explicit_path: Optional[str] = None) -> Optional[str]: """Find pixlet binary: explicit path → bundled binary → system PATH.""" import platform @@ -1647,70 +1661,66 @@ def _starlark_github_token() -> Optional[str]: except Exception: logger.warning("[Starlark] Could not read config for a GitHub token", exc_info=True) return None -def _get_tronbyte_repository_class() -> Type[Any]: - """Import TronbyteRepository from plugin-repos directory.""" - import importlib.util - import importlib +def _import_plugin_code_in_web_process(module_name: str, module_path: Path, + reuse: bool = True) -> Any: + """Import a file of plugin code into the web process and return the module. + The only place the web process executes plugin code. Plugins run in the + display process; the web process reads them as files (PluginCatalog) and + runs a web-UI action's script as a subprocess. Two features still need a + plugin's Python in-process, and both come through here: + + - Starlark: the standalone routes use the starlark-apps plugin's + ``tronbyte_repository`` (browsing the app repository) and + ``pixlet_renderer`` (rendering an app) -- helper modules, never the + plugin class itself. + - A web-UI action with ``oauth_flow``: step 1 calls the action script's + ``get_auth_url()`` (or the Spotify credential helpers). + + Temporary: the plugin web-entry contract (docs/ARCHITECTURE.md, "Web and + display processes") replaces both with an explicit, declared entry point + for plugin web code. + + ``reuse`` returns the module already imported under ``module_name`` + instead of executing the file again. A module that fails to execute is + removed from sys.modules, so one transient failure cannot leave a + half-initialised module cached for the rest of the process (it used to + surface as AttributeError, not ImportError). + """ + import importlib.util + + if reuse and module_name in sys.modules: + return sys.modules[module_name] + + spec = importlib.util.spec_from_file_location(module_name, str(module_path)) + if spec is None or spec.loader is None: + raise ImportError(f"Failed to create module spec for {module_name} at {module_path}") + module = importlib.util.module_from_spec(spec) + sys.modules[module_name] = module + try: + spec.loader.exec_module(module) + except BaseException: + sys.modules.pop(module_name, None) + raise + return module + + +def _get_tronbyte_repository_class() -> Type[Any]: + """TronbyteRepository, from the installed starlark-apps plugin.""" module_path = PROJECT_ROOT / 'plugin-repos' / 'starlark-apps' / 'tronbyte_repository.py' if not module_path.exists(): raise ImportError(f"TronbyteRepository module not found at {module_path}") + return _import_plugin_code_in_web_process('tronbyte_repository', module_path).TronbyteRepository - # If already imported, return cached class - if "tronbyte_repository" in sys.modules: - return sys.modules["tronbyte_repository"].TronbyteRepository - spec = importlib.util.spec_from_file_location("tronbyte_repository", str(module_path)) - if spec is None: - raise ImportError(f"Failed to create module spec for tronbyte_repository at {module_path}") - - module = importlib.util.module_from_spec(spec) - if module is None: - raise ImportError("Failed to create module from spec for tronbyte_repository") - - sys.modules["tronbyte_repository"] = module - try: - spec.loader.exec_module(module) - except BaseException: - # A module that failed to execute must not stay in sys.modules: the - # cache branch above would hand back the half-initialised object for - # the rest of the process, so one transient failure would disable - # this path permanently and surface as AttributeError, not ImportError. - sys.modules.pop("tronbyte_repository", None) - raise - return module.TronbyteRepository def _get_pixlet_renderer_class() -> Type[Any]: - """Import PixletRenderer from plugin-repos directory.""" - import importlib.util - import importlib - + """PixletRenderer, from the installed starlark-apps plugin.""" module_path = PROJECT_ROOT / 'plugin-repos' / 'starlark-apps' / 'pixlet_renderer.py' if not module_path.exists(): raise ImportError(f"PixletRenderer module not found at {module_path}") + return _import_plugin_code_in_web_process('pixlet_renderer', module_path).PixletRenderer - # If already imported, return cached class - if "pixlet_renderer" in sys.modules: - return sys.modules["pixlet_renderer"].PixletRenderer - spec = importlib.util.spec_from_file_location("pixlet_renderer", str(module_path)) - if spec is None: - raise ImportError(f"Failed to create module spec for pixlet_renderer at {module_path}") - - module = importlib.util.module_from_spec(spec) - if module is None: - raise ImportError("Failed to create module from spec for pixlet_renderer") - - sys.modules["pixlet_renderer"] = module - try: - spec.loader.exec_module(module) - except BaseException: - # A module that failed to execute must not stay in sys.modules: the - # cache branch above would hand back the half-initialised object for - # the rest of the process, so one transient failure would disable - # this path permanently and surface as AttributeError, not ImportError. - sys.modules.pop("pixlet_renderer", None) - raise - return module.PixletRenderer def _validate_and_sanitize_app_id(app_id: Optional[str], fallback_source: Optional[str] = None) -> Tuple[Optional[str], Optional[str]]: """Validate and sanitize app_id to a safe slug.""" if not app_id and fallback_source: diff --git a/web_interface/blueprints/api_v3/config.py b/web_interface/blueprints/api_v3/config.py index 809503d9..6a774e8c 100644 --- a/web_interface/blueprints/api_v3/config.py +++ b/web_interface/blueprints/api_v3/config.py @@ -1014,7 +1014,7 @@ def save_main_config(): plugin_manifests = _pkg._discovered_plugin_manifests() for key in data: # Check if this key is a plugin ID - if api_v3.plugin_manager and key in plugin_manifests: + if api_v3.plugin_catalog and key in plugin_manifests: plugin_id = key submitted_config = data[key] if not isinstance(submitted_config, dict): @@ -1030,7 +1030,7 @@ def save_main_config(): # the schema load are far enough apart that a later edit could # separate them. Refuse rather than save without knowing which # fields are secrets. - schema_path = resolve_under(api_v3.plugin_manager.plugins_dir, + schema_path = resolve_under(api_v3.plugin_catalog.plugins_dir, plugin_id, 'config_schema.json') if schema_path is None: return error_response( @@ -1116,18 +1116,9 @@ def save_main_config(): invalidate_cache() - # Notify saved plugins of their new config (with secrets merged), now - # that it is on disk. - for plugin_id in plugin_keys_to_remove: - try: - plugin_instance = api_v3.plugin_manager.get_plugin(plugin_id) - if plugin_instance and hasattr(plugin_instance, 'on_config_change'): - merged_config = api_v3.config_manager.load_config() - plugin_instance.on_config_change(_pkg._prepared_plugin_config( - plugin_id, merged_config.get(plugin_id, {}))) - except Exception as hook_err: - # Don't fail the save if hook fails - logger.warning("on_config_change failed: %s", hook_err) + # Saved plugin sections reach the running plugins through the display + # process's config watcher (on_config_change there); nothing runs a + # plugin in this process. message = 'Configuration saved successfully' # Switching automatic updates on finishes their setup, which needs @@ -1139,7 +1130,10 @@ def save_main_config(): message = f'{message}. {note}' except Exception: logger.warning("Automatic update setup could not be started", exc_info=True) - return success_response(message=message) + # Display hardware, rotation/durations and general settings take + # effect after a display restart; the UI shows its restart banner on + # this flag. + return success_response(message=message, extra={'restart_required': True}) except Exception as e: logger.error("Error saving config", exc_info=True) return error_response( diff --git a/web_interface/blueprints/api_v3/display.py b/web_interface/blueprints/api_v3/display.py index 40bb83a3..1ff23750 100644 --- a/web_interface/blueprints/api_v3/display.py +++ b/web_interface/blueprints/api_v3/display.py @@ -77,19 +77,19 @@ def get_display_modes(): for the duration -- so they are reported with enabled: false rather than omitted. """ - if not api_v3.plugin_manager: - return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 + if not api_v3.plugin_catalog: + return jsonify({'status': 'error', 'message': 'Plugin catalog not initialized'}), 500 # Discovery is lazy and normally triggered by whichever endpoint runs # first, which is a person opening the dashboard. A caller that never # visits it would otherwise see an empty list. - api_v3.plugin_manager.discover_plugins() + api_v3.plugin_catalog.discover_plugins() include_disabled = request.args.get('include_disabled') in ('1', 'true', 'True') full_config = api_v3.config_manager.load_config() if api_v3.config_manager else {} modes = [] - for plugin_id, manifest in sorted(api_v3.plugin_manager.plugin_manifests.items()): + for plugin_id, manifest in sorted(api_v3.plugin_catalog.plugin_manifests.items()): # A hand-edited or migrated config.json can hold a non-dict under a # plugin id; DisplayController._reconcile guards the same shape, so # it happens in practice. Without this, .get() raises AttributeError, @@ -107,7 +107,7 @@ def get_display_modes(): if not enabled and not include_disabled: continue plugin_name = (manifest or {}).get('name') or plugin_id - plugin_modes = api_v3.plugin_manager.get_plugin_display_modes(plugin_id) or [plugin_id] + plugin_modes = api_v3.plugin_catalog.get_plugin_display_modes(plugin_id) or [plugin_id] for mode in plugin_modes: # A single-mode plugin's mode is the plugin, so its own name is # the readable label. Multi-mode plugins have no per-mode name @@ -162,21 +162,21 @@ def start_on_demand_display(): resolved_plugin = plugin_id resolved_mode = mode - if api_v3.plugin_manager: + if api_v3.plugin_catalog: if resolved_plugin and resolved_plugin not in _pkg._discovered_plugin_manifests(resolved_plugin): return jsonify({'status': 'error', 'message': f'Plugin {resolved_plugin} not found'}), 404 if resolved_plugin and not resolved_mode: - modes = api_v3.plugin_manager.get_plugin_display_modes(resolved_plugin) + modes = api_v3.plugin_catalog.get_plugin_display_modes(resolved_plugin) resolved_mode = modes[0] if modes else resolved_plugin elif resolved_mode and not resolved_plugin: _pkg._discovered_plugin_manifests() - resolved_plugin = api_v3.plugin_manager.find_plugin_for_mode(resolved_mode) + resolved_plugin = api_v3.plugin_catalog.find_plugin_for_mode(resolved_mode) if not resolved_plugin: # Not among what was discovered: the plugin that declares # it may have been installed since. Scan once more. _pkg._discovered_plugin_manifests(rescan=True) - resolved_plugin = api_v3.plugin_manager.find_plugin_for_mode(resolved_mode) + resolved_plugin = api_v3.plugin_catalog.find_plugin_for_mode(resolved_mode) if not resolved_plugin: return jsonify({'status': 'error', 'message': f'Mode {resolved_mode} not found'}), 404 diff --git a/web_interface/blueprints/api_v3/misc.py b/web_interface/blueprints/api_v3/misc.py index c708d7c0..87f9a697 100644 --- a/web_interface/blueprints/api_v3/misc.py +++ b/web_interface/blueprints/api_v3/misc.py @@ -76,7 +76,7 @@ def get_health(): # Check plugin system try: - if api_v3.plugin_manager: + if api_v3.plugin_catalog: plugin_count = len(_discovered_plugin_manifests()) health_status['checks']['plugin_system'] = { 'status': 'operational', diff --git a/web_interface/blueprints/api_v3/plugin_config.py b/web_interface/blueprints/api_v3/plugin_config.py index 7aed9110..8db8b168 100644 --- a/web_interface/blueprints/api_v3/plugin_config.py +++ b/web_interface/blueprints/api_v3/plugin_config.py @@ -452,40 +452,9 @@ def save_plugin_config(): status_code=500 ) - # If the plugin is loaded, notify it of the config change with merged config - try: - if api_v3.plugin_manager: - plugin_instance = api_v3.plugin_manager.get_plugin(plugin_id) - if plugin_instance: - # Reload merged config (includes secrets) and pass the plugin-specific section - merged_config = api_v3.config_manager.load_config() - plugin_full_config = _pkg._prepared_plugin_config( - plugin_id, merged_config.get(plugin_id, {})) - if hasattr(plugin_instance, 'on_config_change'): - plugin_instance.on_config_change(plugin_full_config) - - # Update plugin state manager and call lifecycle methods based on enabled state - # This ensures the plugin state is synchronized with the config - enabled = plugin_full_config.get('enabled', plugin_instance.enabled) - - # Update state manager if available - if api_v3.plugin_state_manager: - api_v3.plugin_state_manager.set_plugin_enabled(plugin_id, enabled) - - # Call lifecycle methods to ensure plugin state matches config - try: - if enabled: - if hasattr(plugin_instance, 'on_enable'): - plugin_instance.on_enable() - else: - if hasattr(plugin_instance, 'on_disable'): - plugin_instance.on_disable() - except Exception as lifecycle_error: - # Log the error but don't fail the save - config is already saved - logger.warning("Lifecycle method error for %s: %s", plugin_id, lifecycle_error, exc_info=True) - except Exception as hook_err: - # Do not fail the save if hook fails; just log - logger.warning("on_config_change failed: %s", hook_err) + # The running plugin hears about this from the display process: its + # config watcher calls on_config_change with the prepared section, and + # loads or unloads the plugin if "enabled" changed. secret_count = len(secrets_config) message = f'Plugin {plugin_id} configuration saved successfully' @@ -543,12 +512,7 @@ def _prepare_plugin_config_for_save(plugin_id, plugin_config, schema, schema_mgr current_config = api_v3.config_manager.load_config() if plugin_id in current_config and 'enabled' in current_config[plugin_id]: plugin_config['enabled'] = current_config[plugin_id]['enabled'] - elif api_v3.plugin_manager: - # Fallback to plugin instance if config doesn't have it - plugin_instance = api_v3.plugin_manager.get_plugin(plugin_id) - if plugin_instance: - plugin_config['enabled'] = plugin_instance.enabled - # Final fallback: default to True if plugin is loaded (matches BasePlugin default) + # Fallback: default to True (matches BasePlugin default) if 'enabled' not in plugin_config: plugin_config['enabled'] = True except Exception as e: @@ -977,18 +941,8 @@ def reset_plugin_config(): if default_secrets or not preserve_secrets: api_v3.config_manager.save_raw_file_content('secrets', current_secrets) - # Notify plugin of config change if loaded - try: - if api_v3.plugin_manager: - plugin_instance = api_v3.plugin_manager.get_plugin(plugin_id) - if plugin_instance: - merged_config = api_v3.config_manager.load_config() - plugin_full_config = _pkg._prepared_plugin_config( - plugin_id, merged_config.get(plugin_id, {})) - if hasattr(plugin_instance, 'on_config_change'): - plugin_instance.on_config_change(plugin_full_config) - except Exception as hook_err: - logger.warning("on_config_change failed: %s", hook_err) + # The display's config watcher passes the reset config to the running + # plugin (on_config_change); nothing to notify in this process. return jsonify({ 'status': 'success', diff --git a/web_interface/blueprints/api_v3/plugin_health.py b/web_interface/blueprints/api_v3/plugin_health.py index 093a2ede..f38e8795 100644 --- a/web_interface/blueprints/api_v3/plugin_health.py +++ b/web_interface/blueprints/api_v3/plugin_health.py @@ -1,7 +1,9 @@ """Plugin health, resource metrics and resource limits. -These read and reset the web process's own trackers; the display service -keeps its own (see the route docstrings). +The display process records health and metrics to the shared on-disk cache; +these routes read (and reset) that published state through a tracker and a +monitor backed by the same cache (app.py sets api_v3.health_tracker and +api_v3.resource_monitor). See the route docstrings. Routes decorate the shared `api_v3` Blueprint from the package `__init__`, so their endpoint names do not depend on which module they live in. @@ -11,20 +13,30 @@ from web_interface.blueprints.api_v3 import ( ) +def _health_tracker(): + """The reader of the display's published plugin health, or None.""" + return getattr(api_v3, 'health_tracker', None) + + +def _resource_monitor(): + """The reader of the display's published plugin metrics, or None.""" + return getattr(api_v3, 'resource_monitor', None) + + @api_v3.route('/plugins/health', methods=['GET']) def get_plugin_health(): """Get health metrics for all plugins""" - if not api_v3.plugin_manager: - return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 + if not api_v3.plugin_catalog: + return jsonify({'status': 'error', 'message': 'Plugin catalog not initialized'}), 500 - if not api_v3.plugin_manager.health_tracker: + if not _health_tracker(): return jsonify({ 'status': 'success', 'data': {}, 'message': 'Health tracking not available' }) - tracker = api_v3.plugin_manager.health_tracker + tracker = _health_tracker() # Build per-plugin summaries by ID so persisted (cross-process) health # is included, then fold in any in-memory-only entries. health_summaries = {} @@ -51,17 +63,17 @@ def get_plugin_health(): @api_v3.route('/plugins/health/', methods=['GET']) def get_plugin_health_single(plugin_id): """Get health metrics for a specific plugin""" - if not api_v3.plugin_manager: - return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 + if not api_v3.plugin_catalog: + return jsonify({'status': 'error', 'message': 'Plugin catalog not initialized'}), 500 - if not api_v3.plugin_manager.health_tracker: + if not _health_tracker(): return jsonify({ 'status': 'error', 'message': 'Health tracking not available' }), 503 # force_reload for the same reason as the list route above. - health_summary = api_v3.plugin_manager.health_tracker.get_health_summary( + health_summary = _health_tracker().get_health_summary( plugin_id, force_reload=True) return jsonify({ @@ -79,17 +91,17 @@ def reset_plugin_health(plugin_id): in-memory state, so its next recorded success or failure can write that state back; restart the display service for a reset it will honour. """ - if not api_v3.plugin_manager: - return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 + if not api_v3.plugin_catalog: + return jsonify({'status': 'error', 'message': 'Plugin catalog not initialized'}), 500 - if not api_v3.plugin_manager.health_tracker: + if not _health_tracker(): return jsonify({ 'status': 'error', 'message': 'Health tracking not available' }), 503 # Reset health state - api_v3.plugin_manager.health_tracker.reset_health(plugin_id) + _health_tracker().reset_health(plugin_id) return jsonify({ 'status': 'success', @@ -100,17 +112,17 @@ def reset_plugin_health(plugin_id): @api_v3.route('/plugins/metrics', methods=['GET']) def get_plugin_metrics(): """Get resource metrics for all plugins""" - if not api_v3.plugin_manager: - return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 + if not api_v3.plugin_catalog: + return jsonify({'status': 'error', 'message': 'Plugin catalog not initialized'}), 500 - if not api_v3.plugin_manager.resource_monitor: + if not _resource_monitor(): return jsonify({ 'status': 'success', 'data': {}, 'message': 'Resource monitoring not available' }) - monitor = api_v3.plugin_manager.resource_monitor + monitor = _resource_monitor() # Build per-plugin summaries by ID so persisted (cross-process) metrics # are included, then fold in any in-memory-only entries. metrics_summaries = {} @@ -136,17 +148,17 @@ def get_plugin_metrics(): @api_v3.route('/plugins/metrics/', methods=['GET']) def get_plugin_metrics_single(plugin_id): """Get resource metrics for a specific plugin""" - if not api_v3.plugin_manager: - return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 + if not api_v3.plugin_catalog: + return jsonify({'status': 'error', 'message': 'Plugin catalog not initialized'}), 500 - if not api_v3.plugin_manager.resource_monitor: + if not _resource_monitor(): return jsonify({ 'status': 'error', 'message': 'Resource monitoring not available' }), 503 # force_reload for the same reason as the list route above. - metrics_summary = api_v3.plugin_manager.resource_monitor.get_metrics_summary( + metrics_summary = _resource_monitor().get_metrics_summary( plugin_id, force_reload=True) return jsonify({ @@ -163,17 +175,17 @@ def reset_plugin_metrics(plugin_id): display service keeps accumulating in its own process and republishes its totals on its next persist, so the reset does not stick while it runs. """ - if not api_v3.plugin_manager: - return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 + if not api_v3.plugin_catalog: + return jsonify({'status': 'error', 'message': 'Plugin catalog not initialized'}), 500 - if not api_v3.plugin_manager.resource_monitor: + if not _resource_monitor(): return jsonify({ 'status': 'error', 'message': 'Resource monitoring not available' }), 503 # Reset metrics - api_v3.plugin_manager.resource_monitor.reset_metrics(plugin_id) + _resource_monitor().reset_metrics(plugin_id) return jsonify({ 'status': 'success', @@ -190,10 +202,10 @@ def manage_plugin_limits(plugin_id): plugin, so a change to existing limits takes effect there after the display service restarts. """ - if not api_v3.plugin_manager: - return jsonify({'status': 'error', 'message': 'Plugin manager not initialized'}), 500 + if not api_v3.plugin_catalog: + return jsonify({'status': 'error', 'message': 'Plugin catalog not initialized'}), 500 - if not api_v3.plugin_manager.resource_monitor: + if not _resource_monitor(): return jsonify({ 'status': 'error', 'message': 'Resource monitoring not available' @@ -201,7 +213,7 @@ def manage_plugin_limits(plugin_id): if request.method == 'GET': # Get limits - limits = api_v3.plugin_manager.resource_monitor.get_limits(plugin_id) + limits = _resource_monitor().get_limits(plugin_id) if limits: return jsonify({ 'status': 'success', @@ -234,7 +246,7 @@ def manage_plugin_limits(plugin_id): 'message': f'{bad} must be a non-negative number or null'}), 400 limits = limits_from_dict(data) - api_v3.plugin_manager.resource_monitor.set_limits(plugin_id, limits) + _resource_monitor().set_limits(plugin_id, limits) return jsonify({ 'status': 'success', diff --git a/web_interface/blueprints/api_v3/plugin_operations.py b/web_interface/blueprints/api_v3/plugin_operations.py index 05cda7e3..56f43071 100644 --- a/web_interface/blueprints/api_v3/plugin_operations.py +++ b/web_interface/blueprints/api_v3/plugin_operations.py @@ -120,10 +120,10 @@ def get_plugin_state(): def reconcile_plugin_state(): """Reconcile plugin state across all sources""" try: - if not api_v3.plugin_state_manager or not api_v3.plugin_manager: + if not api_v3.plugin_state_manager or not api_v3.plugin_catalog: return error_response( ErrorCode.SYSTEM_ERROR, - 'State manager or plugin manager not initialized', + 'State manager or plugin catalog not initialized', status_code=500 ) @@ -139,8 +139,8 @@ def reconcile_plugin_state(): reconciler = StateReconciliation( state_manager=api_v3.plugin_state_manager, config_manager=api_v3.config_manager, - plugin_manager=api_v3.plugin_manager, - plugins_dir=Path(api_v3.plugin_manager.plugins_dir) + plugin_manager=api_v3.plugin_catalog, + plugins_dir=Path(api_v3.plugin_catalog.plugins_dir) ) result = reconciler.reconcile_state(force=force) @@ -205,7 +205,7 @@ def _drop_stale_reconciliation_findings(unresolved): ) cm = api_v3.config_manager - plugins_dir = getattr(api_v3.plugin_manager, 'plugins_dir', None) + plugins_dir = getattr(api_v3.plugin_catalog, 'plugins_dir', None) installed = disk_plugin_ids(plugins_dir) if plugins_dir else set() config_keys = config_plugin_ids(cm.load_config() or {}, ignored_config_keys(cm, installed)) diff --git a/web_interface/blueprints/api_v3/plugin_store.py b/web_interface/blueprints/api_v3/plugin_store.py index 3580097f..016a3f75 100644 --- a/web_interface/blueprints/api_v3/plugin_store.py +++ b/web_interface/blueprints/api_v3/plugin_store.py @@ -6,7 +6,8 @@ so their endpoint names do not depend on which module they live in. """ from web_interface.blueprints.api_v3 import ( ErrorCode, OperationType, Path, _do_transactional_uninstall, - _non_plugin_id_error, _get_plugin_version, _plugin_directory, api_v3, + _non_plugin_id_error, _get_plugin_version, _plugin_directory, + _plugin_enabled_in_config, _store_restart_fields, api_v3, datetime, error_response, exception_error_response, json, jsonify, logger, request, success_response, validate_request_json, ) @@ -228,11 +229,12 @@ def update_plugin(): if api_v3.schema_manager: api_v3.schema_manager.invalidate_cache(plugin_id) - # Rediscover plugins - if api_v3.plugin_manager: - api_v3.plugin_manager.discover_plugins() - if plugin_id in api_v3.plugin_manager.plugins: - api_v3.plugin_manager.reload_plugin(plugin_id) + # Rediscover plugins. The web process runs no plugin code, so + # there is nothing here to reload: the display keeps running the + # version it loaded until it restarts, which restart_required + # below asks for. + if api_v3.plugin_catalog: + api_v3.plugin_catalog.discover_plugins() # Update state and history if api_v3.plugin_state_manager: @@ -261,7 +263,10 @@ def update_plugin(): 'commit': updated_commit, 'update_status': update_status }, - message=message + message=message, + extra=_store_restart_fields( + 'update', _plugin_enabled_in_config(plugin_id), + changed=update_status == 'updated'), ) else: refusal = _compatibility_refusal(plugin_id) @@ -341,6 +346,8 @@ def uninstall_plugin(): if api_v3.operation_queue: def uninstall_callback(operation): """Callback to execute plugin uninstallation via transactional helper.""" + # Read before the uninstall removes the config section. + was_enabled = _plugin_enabled_in_config(plugin_id) success, error_msg = _do_transactional_uninstall(plugin_id, preserve_config) if not success: if api_v3.operation_history: @@ -358,7 +365,9 @@ def uninstall_plugin(): status="success", details={"preserve_config": preserve_config} ) - return {'success': True, 'message': 'Plugin uninstalled successfully'} + return {'success': True, 'message': 'Plugin uninstalled successfully', + **_store_restart_fields('uninstall', was_enabled, + preserve_config=preserve_config)} # Enqueue operation operation_id = api_v3.operation_queue.enqueue_operation( @@ -373,6 +382,7 @@ def uninstall_plugin(): ) else: # Direct (non-queued) transactional uninstall + was_enabled = _plugin_enabled_in_config(plugin_id) success, error_msg = _do_transactional_uninstall(plugin_id, preserve_config) if success: @@ -383,7 +393,10 @@ def uninstall_plugin(): status="success", details={"preserve_config": preserve_config} ) - return success_response(message='Plugin uninstalled successfully') + return success_response( + message='Plugin uninstalled successfully', + extra=_store_restart_fields('uninstall', was_enabled, + preserve_config=preserve_config)) else: if api_v3.operation_history: api_v3.operation_history.record_operation( @@ -448,10 +461,11 @@ def install_plugin(): if api_v3.schema_manager: api_v3.schema_manager.invalidate_cache(plugin_id) - # Discover and load the new plugin - if api_v3.plugin_manager: - api_v3.plugin_manager.discover_plugins() - api_v3.plugin_manager.load_plugin(plugin_id) + # List the new plugin. The display loads it, from disk, when + # it is enabled; see restart_required below for one that + # already is. + if api_v3.plugin_catalog: + api_v3.plugin_catalog.discover_plugins() # Update state manager if api_v3.plugin_state_manager: @@ -468,7 +482,9 @@ def install_plugin(): ) branch_msg = f" (branch: {branch})" if branch else "" - return {'success': True, 'message': f'Plugin {plugin_id} installed successfully{branch_msg}'} + return {'success': True, + 'message': f'Plugin {plugin_id} installed successfully{branch_msg}', + **_store_restart_fields('install', _plugin_enabled_in_config(plugin_id))} else: error_msg = f'Failed to install plugin {plugin_id}' if branch: @@ -511,9 +527,8 @@ def install_plugin(): if success: if api_v3.schema_manager: api_v3.schema_manager.invalidate_cache(plugin_id) - if api_v3.plugin_manager: - api_v3.plugin_manager.discover_plugins() - api_v3.plugin_manager.load_plugin(plugin_id) + if api_v3.plugin_catalog: + api_v3.plugin_catalog.discover_plugins() if api_v3.plugin_state_manager: api_v3.plugin_state_manager.set_plugin_installed(plugin_id) if api_v3.operation_history: @@ -526,7 +541,9 @@ def install_plugin(): ) branch_msg = f" (branch: {branch})" if branch else "" - return success_response(message=f'Plugin installed successfully{branch_msg}') + return success_response( + message=f'Plugin installed successfully{branch_msg}', + extra=_store_restart_fields('install', _plugin_enabled_in_config(plugin_id))) else: error_msg = f'Failed to install plugin {plugin_id}' if branch: @@ -587,10 +604,9 @@ def install_plugin_from_url(): if api_v3.schema_manager and installed_plugin_id: api_v3.schema_manager.invalidate_cache(installed_plugin_id) - # Discover and load the new plugin - if api_v3.plugin_manager and installed_plugin_id: - api_v3.plugin_manager.discover_plugins() - api_v3.plugin_manager.load_plugin(installed_plugin_id) + # List the new plugin; the display loads it when it is enabled. + if api_v3.plugin_catalog and installed_plugin_id: + api_v3.plugin_catalog.discover_plugins() branch_msg = f" (branch: {result.get('branch', branch)})" if (result.get('branch') or branch) else "" response_data = { @@ -601,6 +617,9 @@ def install_plugin_from_url(): } if result.get('branch'): response_data['branch'] = result.get('branch') + if installed_plugin_id: + response_data.update(_store_restart_fields( + 'install', _plugin_enabled_in_config(installed_plugin_id))) return jsonify(response_data) else: return jsonify({ diff --git a/web_interface/blueprints/api_v3/plugins.py b/web_interface/blueprints/api_v3/plugins.py index 9ca4546e..639903ea 100644 --- a/web_interface/blueprints/api_v3/plugins.py +++ b/web_interface/blueprints/api_v3/plugins.py @@ -11,7 +11,7 @@ from web_interface.blueprints.api_v3 import ( ) from src.common.path_safety import safe_path_component from src.plugin_system.base_plugin import ( - configured_vegas_participation, resolve_vegas_participation, + configured_vegas_participation, vegas_participation_value, ) import web_interface.blueprints.api_v3 as _pkg # Read through the module rather than bound by value: tests patch these @@ -20,18 +20,48 @@ import web_interface.blueprints.api_v3 as _pkg # package is the only patch point that covers every caller. +def _vegas_participation(plugin_id, plugin_config, manifest): + """What Vegas does with a plugin, as far as its files say, and from where. + + The order the display resolves it in (resolve_vegas_participation), up + to where that needs the plugin's code: the user's ``vegas_participation`` setting + (``'config'``), then the manifest's declared ``vegas_participation`` + (``'manifest'``). Past those the display asks the plugin itself -- a + get_vegas_participation() override or the legacy Vegas hooks -- which the + web process never runs, so the answer is ``(None, 'runtime')``: decided + at run time, not guessed here. A plugin that overrides + get_vegas_participation() can still differ from its manifest. + """ + configured = configured_vegas_participation(plugin_id, plugin_config) + if configured is not None: + return configured, 'config' + declared = vegas_participation_value( + manifest.get('vegas_participation') if isinstance(manifest, dict) else None) + if declared is not None: + return declared, 'manifest' + return None, 'runtime' + + @api_v3.route('/plugins/installed', methods=['GET']) def get_installed_plugins(): - """Get installed plugins""" - if not api_v3.plugin_manager or not api_v3.plugin_store_manager: + """Get installed plugins. + + Metadata comes from the plugin catalog (manifests on disk), ``enabled`` + from config.json. ``loaded``, ``state`` and ``error_info`` are always + null: they would describe the display process's plugin instances, and + the display does not publish which plugins it has loaded. What it does + publish -- health, metrics, errors -- is served by /plugins/health, + /plugins/metrics and /errors. + """ + if not api_v3.plugin_catalog or not api_v3.plugin_store_manager: return jsonify({'status': 'error', 'message': 'Plugin managers not initialized'}), 500 # Re-discover plugins to ensure we have the latest list # This handles cases where plugins are added/removed after app startup - api_v3.plugin_manager.discover_plugins() + api_v3.plugin_catalog.discover_plugins() - # Get all installed plugin info from the plugin manager - all_plugin_info = api_v3.plugin_manager.get_all_plugin_info() + # Get all installed plugin info from the catalog + all_plugin_info = api_v3.plugin_catalog.get_all_plugin_info() # Load config once before the loop (not per-plugin) full_config = api_v3.config_manager.load_config() if api_v3.config_manager else {} @@ -45,18 +75,6 @@ def get_installed_plugins(): return None def _build_plugin_entry_inner(plugin_info, plugin_id): - # Capture runtime state (state machine + error context) before the - # manifest merge below can shadow the 'state' key. get_all_plugin_info - # attaches this via PluginStateManager.get_state_info(); surfacing it - # lets the UI show *why* a plugin isn't running instead of just - # 'loaded: false'. - state_info = plugin_info.get('state') - plugin_state = None - plugin_error_info = None - if isinstance(state_info, dict): - plugin_state = state_info.get('state') - plugin_error_info = state_info.get('error_info') - # Re-read manifest from disk to ensure we have the latest metadata. # Through the resolver, not plugins_dir/: a plugin installed as # ledmatrix- otherwise never had its manifest refreshed here. @@ -74,16 +92,13 @@ def get_installed_plugins(): except (FileNotFoundError, PermissionError, json.JSONDecodeError) as e: logger.debug("Could not read fresh manifest for %s: %s", plugin_id, e) - # Enabled status: config is source of truth, fall back to instance - enabled = None + # Enabled status: config.json, read by the display's rule -- it runs + # a plugin only when its section says "enabled": true, so a missing + # flag is disabled here too. plugin_config = full_config.get(plugin_id, {}) - if 'enabled' in plugin_config: - enabled = bool(plugin_config['enabled']) - - # Single get_plugin() call shared for both enabled fallback and Vegas mode - plugin_instance = api_v3.plugin_manager.get_plugin(plugin_id) - if enabled is None: - enabled = plugin_instance.enabled if plugin_instance else True + if not isinstance(plugin_config, dict): + plugin_config = {} + enabled = bool(plugin_config.get('enabled', False)) # Verified + latest published version from registry (no network call) store_info = api_v3.plugin_store_manager.get_registry_info(plugin_id) @@ -113,32 +128,16 @@ def get_installed_plugins(): if store_info and not last_commit_message: last_commit_message = store_info.get('last_commit_message') - # Vegas mode from instance, overridden by explicit config value - vegas_mode = None + # Vegas mode as configured. What a plugin's code would choose on its + # own is only known to the display, which runs it. + vegas_mode = plugin_config.get('vegas_mode') vegas_content_type = None - if plugin_instance: - try: - if hasattr(plugin_instance, 'get_vegas_display_mode'): - mode = plugin_instance.get_vegas_display_mode() - vegas_mode = mode.value if hasattr(mode, 'value') else str(mode) - except (AttributeError, TypeError, ValueError) as e: - logger.debug("[%s] Failed to get vegas_display_mode: %s", plugin_id, e) - try: - if hasattr(plugin_instance, 'get_vegas_content_type'): - vegas_content_type = plugin_instance.get_vegas_content_type() - except (AttributeError, TypeError, ValueError) as e: - logger.debug("[%s] Failed to get vegas_content_type: %s", plugin_id, e) - if 'vegas_mode' in plugin_config: - vegas_mode = plugin_config['vegas_mode'] - - # What Vegas actually does with the plugin: 'scroll', 'pause' or - # 'exclude'. The same resolution the ticker uses; without a loaded - # instance only the user's own setting is known. - if plugin_instance is not None: - vegas_participation = resolve_vegas_participation(plugin_instance, plugin_id) - else: - vegas_participation = configured_vegas_participation(plugin_id, plugin_config) + # What Vegas does with it: 'scroll', 'pause' or 'exclude', or None + # when only the plugin's code (run by the display) decides. The Vegas + # order list badges a None as its configured vegas_mode, else Scroll. + vegas_participation, vegas_participation_source = _vegas_participation( + plugin_id, plugin_config, plugin_info) return { 'id': plugin_id, @@ -155,9 +154,10 @@ def get_installed_plugins(): 'icon': plugin_info.get('icon') if isinstance(plugin_info.get('icon'), str) else None, 'enabled': enabled, 'verified': verified, - 'loaded': plugin_info.get('loaded', False), - 'state': plugin_state, - 'error_info': plugin_error_info, + # Not published by the display process; see the docstring. + 'loaded': None, + 'state': None, + 'error_info': None, 'last_updated': last_updated, 'last_commit': last_commit, 'last_commit_message': last_commit_message, @@ -166,6 +166,7 @@ def get_installed_plugins(): 'vegas_mode': vegas_mode, 'vegas_content_type': vegas_content_type, 'vegas_participation': vegas_participation, + 'vegas_participation_source': vegas_participation_source, } from concurrent.futures import ThreadPoolExecutor @@ -183,7 +184,7 @@ def toggle_plugin(): plugin_id = None enabled = None try: - if not api_v3.plugin_manager or not api_v3.config_manager: + if not api_v3.plugin_catalog or not api_v3.config_manager: return jsonify({'status': 'error', 'message': 'Plugin or config manager not initialized'}), 500 # Support both JSON and form data (for HTMX submissions) @@ -221,7 +222,7 @@ def toggle_plugin(): current_enabled = config.get(plugin_id, {}).get('enabled', False) enabled = not current_enabled - # A Starlark app is not a plugin in plugin_manager's sense -- it is an + # A Starlark app is not a plugin in the catalog's sense -- it is an # entry in starlark-apps' own manifest -- so its enable/disable is # handled here rather than falling through to the check below, which # would answer "Plugin not found". @@ -258,21 +259,9 @@ def toggle_plugin(): status="success" ) - # If plugin is loaded, also call its lifecycle methods - # Wrap in try/except to prevent lifecycle errors from failing the toggle - plugin = api_v3.plugin_manager.get_plugin(plugin_id) - if plugin: - try: - if enabled: - if hasattr(plugin, 'on_enable'): - plugin.on_enable() - else: - if hasattr(plugin, 'on_disable'): - plugin.on_disable() - except Exception as lifecycle_error: - # Log the error but don't fail the toggle - config is already saved - logger.warning("Lifecycle method error for %s: %s", plugin_id, lifecycle_error, exc_info=True) - + # No lifecycle hooks here: the display's config watcher sees the + # enabled flag change and loads or unloads the plugin itself + # (DisplayController._reconcile_enabled_plugins). return success_response( message=f"Plugin {plugin_id} {'enabled' if enabled else 'disabled'} successfully" ) @@ -511,16 +500,11 @@ sys.exit(proc.returncode) # Step 1: Get initial data (like auth URL) # For OAuth flows, we might need to import the script as a module if action_def.get('oauth_flow'): - # Import script as module to get auth URL - import sys - import importlib.util - - spec = importlib.util.spec_from_file_location("plugin_action", script_file) - action_module = importlib.util.module_from_spec(spec) - sys.modules["plugin_action"] = action_module - try: - spec.loader.exec_module(action_module) + # Plugin code in the web process: see the + # function for why, and what replaces it. + action_module = _pkg._import_plugin_code_in_web_process( + "plugin_action", script_file, reuse=False) # Try to get auth URL using common patterns auth_url = None diff --git a/web_interface/blueprints/api_v3/system.py b/web_interface/blueprints/api_v3/system.py index b9cd93f1..51720ed3 100644 --- a/web_interface/blueprints/api_v3/system.py +++ b/web_interface/blueprints/api_v3/system.py @@ -506,7 +506,7 @@ def execute_system_action(): 'output': "\n".join(outputs) }) elif action == 'install_plugin_requirements': - active_pm = getattr(api_v3, 'plugin_manager', None) + active_pm = getattr(api_v3, 'plugin_catalog', None) if active_pm: plugins_dir = Path(active_pm.plugins_dir) else: diff --git a/web_interface/blueprints/pages_v3.py b/web_interface/blueprints/pages_v3.py index 8b01fc43..4a15220d 100644 --- a/web_interface/blueprints/pages_v3.py +++ b/web_interface/blueprints/pages_v3.py @@ -20,7 +20,8 @@ from web_interface import widget_bundle logger = logging.getLogger(__name__) # The managers live on the blueprint object: app.py sets -# pages_v3.config_manager, pages_v3.plugin_manager and the rest. +# pages_v3.config_manager, pages_v3.plugin_catalog and the rest. The catalog +# reads plugins as files; this process never runs plugin code. pages_v3 = Blueprint('pages_v3', __name__) @@ -201,11 +202,11 @@ def settings_search_index(): ] try: plugin_ids = [] - if pages_v3.plugin_manager: + if pages_v3.plugin_catalog: try: - pages_v3.plugin_manager.discover_plugins() + pages_v3.plugin_catalog.discover_plugins() plugin_ids = sorted( - pi.get('id') for pi in pages_v3.plugin_manager.get_all_plugin_info() + pi.get('id') for pi in pages_v3.plugin_catalog.get_all_plugin_info() if pi.get('id') ) except Exception: @@ -220,7 +221,7 @@ def settings_search_index(): fields.extend(_extract_settings_fields(_partial_html(loader), tab, label)) for pid in plugin_ids: - info = pages_v3.plugin_manager.get_plugin_info(pid) or {} + info = pages_v3.plugin_catalog.get_plugin_info(pid) or {} label = info.get('name', pid) html = _partial_html(lambda pid=pid: _load_plugin_config_partial(pid)) fields.extend(_extract_settings_fields(html, pid, label)) @@ -264,8 +265,8 @@ def serve_plugin_web_ui(plugin_id, filename): if not safe_id or not safe_fn: return 'Invalid path component', 400, {'Content-Type': 'text/plain'} - if not pages_v3.plugin_manager: - return 'Plugin manager not available', 503, {'Content-Type': 'text/plain'} + if not pages_v3.plugin_catalog: + return 'Plugin catalog not available', 503, {'Content-Type': 'text/plain'} try: web_ui_path = resolve_under(_plugin_dir_for(safe_id) / 'web_ui', safe_fn) @@ -329,7 +330,7 @@ def _resolved_plugin_dir(plugin_id): its id is found there), and it refuses anything that is not one plain path segment. See src/plugin_system/plugin_dirs.py. """ - found = pages_v3.plugin_manager.get_plugin_directory(plugin_id) + found = pages_v3.plugin_catalog.get_plugin_directory(plugin_id) if isinstance(found, (str, Path)) and Path(found).exists(): return Path(found) return None @@ -347,7 +348,7 @@ def _plugin_dir_for(safe_id): if resolved is not None: return resolved - plugins_base = Path(pages_v3.plugin_manager.plugins_dir).resolve() + plugins_base = Path(pages_v3.plugin_catalog.plugins_dir).resolve() plugin_dir = resolve_under(plugins_base, safe_id) if plugin_dir is None: raise ValueError('plugin id escapes the plugins directory') @@ -416,8 +417,8 @@ def serve_plugin_widget(plugin_id, widget_name): if not safe_id or not safe_widget: return 'Invalid path component', 400, {'Content-Type': 'text/plain'} - if not pages_v3.plugin_manager: - return 'Plugin manager not available', 503, {'Content-Type': 'text/plain'} + if not pages_v3.plugin_catalog: + return 'Plugin catalog not available', 503, {'Content-Type': 'text/plain'} try: plugin_dir = _plugin_dir_for(safe_id) @@ -543,17 +544,17 @@ def _load_durations_partial(): main_config = pages_v3.config_manager.load_config() duration_groups = [] covered_keys = set() - if pages_v3.plugin_manager: + if pages_v3.plugin_catalog: try: - pages_v3.plugin_manager.discover_plugins() + pages_v3.plugin_catalog.discover_plugins() saved = (main_config.get('display', {}) or {}).get('display_durations', {}) or {} - infos = sorted(pages_v3.plugin_manager.get_all_plugin_info(), + infos = sorted(pages_v3.plugin_catalog.get_all_plugin_info(), key=lambda i: (i.get('name') or i.get('id') or '').lower()) for info in infos: pid = info.get('id') if not pid or not (main_config.get(pid, {}) or {}).get('enabled', False): continue - modes = pages_v3.plugin_manager.get_plugin_display_modes(pid) or [pid] + modes = pages_v3.plugin_catalog.get_plugin_display_modes(pid) or [pid] covered_keys.update(modes) default = _plugin_default_duration(pid, main_config.get(pid, {}) or {}) duration_groups.append({ @@ -691,7 +692,7 @@ def _load_plugin_config_partial(plugin_id): return '
Invalid plugin ID
', 400 try: - if not pages_v3.plugin_manager: + if not pages_v3.plugin_catalog: return '
Plugin manager not available
', 500 # Handle starlark app config (starlark:) @@ -699,18 +700,18 @@ def _load_plugin_config_partial(plugin_id): return _load_starlark_config_partial(plugin_id[len('starlark:'):]) # Resolve and validate all plugin paths against the plugins base directory - _plugins_base = Path(pages_v3.plugin_manager.plugins_dir).resolve() + _plugins_base = Path(pages_v3.plugin_catalog.plugins_dir).resolve() _plugin_dir = resolve_under(_plugins_base, plugin_id) if _plugin_dir is None: return '
Invalid plugin ID
', 400 # Try to get plugin info first - plugin_info = pages_v3.plugin_manager.get_plugin_info(plugin_id) + plugin_info = pages_v3.plugin_catalog.get_plugin_info(plugin_id) # If not found, re-discover plugins (handles plugins added after startup) if not plugin_info: - pages_v3.plugin_manager.discover_plugins() - plugin_info = pages_v3.plugin_manager.get_plugin_info(plugin_id) + pages_v3.plugin_catalog.discover_plugins() + plugin_info = pages_v3.plugin_catalog.get_plugin_info(plugin_id) if not plugin_info: return '
Plugin not found
', 404 @@ -720,9 +721,6 @@ def _load_plugin_config_partial(plugin_id): # for one installed as ledmatrix- is not plugins_dir/. _plugin_dir = _resolved_plugin_dir(plugin_id) or _plugin_dir - # Get plugin instance (may be None if not loaded) - plugin_instance = pages_v3.plugin_manager.get_plugin(plugin_id) - # Get plugin configuration from config file config = {} if pages_v3.config_manager: @@ -829,8 +827,6 @@ def _load_plugin_config_partial(plugin_id): # Determine enabled status enabled = config.get('enabled', True) - if plugin_instance: - enabled = plugin_instance.enabled # Build plugin data for template plugin_data = { @@ -868,7 +864,10 @@ def _load_starlark_config_partial(app_id): return '
Invalid app ID
', 400 try: - starlark_plugin = pages_v3.plugin_manager.get_plugin('starlark-apps') if pages_v3.plugin_manager else None + # Always None: the web process runs no plugin code. See + # api_v3._get_starlark_plugin, the one seam for this. + from web_interface.blueprints.api_v3 import _get_starlark_plugin + starlark_plugin = _get_starlark_plugin() if starlark_plugin and hasattr(starlark_plugin, 'apps'): app = starlark_plugin.apps.get(app_id) diff --git a/web_interface/static/v3/app.js b/web_interface/static/v3/app.js index 77a607b3..f10a036e 100644 --- a/web_interface/static/v3/app.js +++ b/web_interface/static/v3/app.js @@ -27,8 +27,9 @@ * the display" banner; the floating live preview; aria-current on the nav; * the mobile nav drawer's keyboard handling; header widget placement. * - * Globals: showSaveResult, showRestartPending, dismissRestartPending, - * restartPendingNow, toggleFloatingPreview, applyFloatingPreviewSize, + * Globals: showSaveResult, showRestartPending, noteRestartRequired, + * dismissRestartPending, restartPendingNow, toggleFloatingPreview, + * applyFloatingPreviewSize, * cycleFloatingPreviewSize, updateFloatingPreviewVisibility, * previewPluginNow, updateNavAriaCurrent, placeHeaderWidgets. */ @@ -72,19 +73,31 @@ document.body.addEventListener('htmx:afterRequest', function(event) { } } - // Main-config saves (display hardware, rotation/durations, general) only - // take effect after a display-service restart — surface the reminder - // banner. Plugin config saves apply live and are deliberately excluded. + // A response that needs a display restart to take effect says so with + // restart_required (main-config saves, store operations the display + // cannot pick up live); surface the reminder banner for it. try { - const cfg = event.detail.requestConfig; - if (cfg && cfg.verb === 'post' && - (cfg.path || '').includes('/api/v3/config/main') && - response && response.status >= 200 && response.status < 300) { - window.showRestartPending(); + if (response && response.status >= 200 && response.status < 300 && response.responseText) { + window.noteRestartRequired(JSON.parse(response.responseText)); } - } catch { /* banner is best-effort */ } + } catch { /* not JSON; the banner is best-effort */ } }); +/** + * Shows the restart-pending banner when an API response says the change + * needs a display restart (`restart_required: true`), with the response's + * `restart_message` as its wording when there is one. Every caller of an + * endpoint that can answer this way passes the parsed body here, so the + * server alone decides when the banner appears. + * @param {Object} data - a parsed JSON response body (or an operation result) + * @returns {boolean} whether the banner was shown + */ +window.noteRestartRequired = function(data) { + if (!data || data.restart_required !== true) return false; + window.showRestartPending(typeof data.restart_message === 'string' ? data.restart_message : undefined); + return true; +}; + /** * Shows the outcome of a settings form save as one notification. Used by the * hx-on:htmx:after-request of the Display, Rotation & Durations and General diff --git a/web_interface/static/v3/js/app-shell.js b/web_interface/static/v3/js/app-shell.js index 6bf6c3cd..2ab63bea 100644 --- a/web_interface/static/v3/js/app-shell.js +++ b/web_interface/static/v3/js/app-shell.js @@ -1366,6 +1366,8 @@ function markPanelLoadFailed(event) { const data = await response.json(); showNotification(data.message, data.status); + // The display keeps running the old code until it restarts. + window.noteRestartRequired(data); if (data.status === 'success') { // Refresh the plugin list diff --git a/web_interface/static/v3/js/plugins/install_manager.js b/web_interface/static/v3/js/plugins/install_manager.js index d399eb39..8ed36398 100644 --- a/web_interface/static/v3/js/plugins/install_manager.js +++ b/web_interface/static/v3/js/plugins/install_manager.js @@ -69,10 +69,20 @@ const PluginInstallManager = { if (onProgress) onProgress(i + 1, plugins.length, plugin.id); // Each plugin gets its own pass over the backoff schedule. const pendingDelays = retryDelays.slice(); + let lostAnswer = false; for (;;) { try { const result = await window.PluginAPI.updatePlugin(plugin.id); - results.push({ pluginId: plugin.id, success: true, result }); + const entry = { pluginId: plugin.id, success: true, result }; + if (lostAnswer) { + // An earlier attempt got no answer, so it may have + // updated the plugin before the connection dropped, + // and this answer then says up_to_date. restartRequest() + // reads these two. + entry.afterLostAnswer = true; + entry.enabled = plugin.enabled === true; + } + results.push(entry); break; } catch (error) { // No HTTP answer at all (connection refused/reset, e.g. the @@ -81,6 +91,7 @@ const PluginInstallManager = { // back rather than skipping it. An HTTP error response is // the server's answer and is not retried. if (error && error.error_code === 'NETWORK_ERROR' && pendingDelays.length > 0) { + lostAnswer = true; await sleep(pendingDelays.shift()); continue; } @@ -90,9 +101,14 @@ const PluginInstallManager = { } } - // Reload plugin list once at the end + // Reload plugin list once at the end. A failed refresh must not + // lose the results: they carry the restart flags. if (window.PluginStateManager) { - await window.PluginStateManager.loadInstalledPlugins(); + try { + await window.PluginStateManager.loadInstalledPlugins(); + } catch (error) { + console.warn('Could not refresh the installed plugin list after updating:', error); + } } return results; @@ -148,6 +164,42 @@ const PluginInstallManager = { text: parts.join(', '), type }; + }, + + /** + * The first update answer that says the display needs a restart, or null. + * + * The display keeps running the code it loaded until it restarts, so an + * update of a plugin it runs answers `restart_required: true` (with the + * banner's wording in `restart_message`). One restart covers every + * plugin in the run, so one answer is enough; pass it to + * window.noteRestartRequired. + * + * Failing that, an enabled plugin whose first request got no answer and + * whose re-sent one says up_to_date may have been updated by the lost + * request, which nothing reported: that asks for a restart too, since a + * needless restart is cheaper than the display running old code. + * + * @param {Array} results - updateAll()'s results + * @returns {Object|null} + */ + restartRequest(results) { + const entries = Array.isArray(results) ? results : []; + for (const entry of entries) { + const body = entry && entry.success ? entry.result : null; + if (body && body.restart_required === true) return body; + } + for (const entry of entries) { + if (entry && entry.success && entry.afterLostAnswer && entry.enabled + && this.updateOutcome(entry) === 'up_to_date') { + return { + restart_required: true, + restart_message: `Plugin ${entry.pluginId} may have been updated before the ` + + 'connection dropped — restart the display to be sure it runs the new version', + }; + } + } + return null; } }; diff --git a/web_interface/static/v3/plugins_manager.js b/web_interface/static/v3/plugins_manager.js index d2cd4745..f1e4d296 100644 --- a/web_interface/static/v3/plugins_manager.js +++ b/web_interface/static/v3/plugins_manager.js @@ -453,6 +453,7 @@ window.handleGitHubPluginInstall = function() { urlInput.value = ''; showNotification(`Plugin ${data.plugin_id} installed successfully`, 'success'); + window.noteRestartRequired(data); setTimeout(() => window.pluginManager.loadInstalledPlugins(true).catch(() => {}), 1000); } else { @@ -1548,6 +1549,9 @@ function runUpdateAllPlugins() { // a no-op update is "already up to date", not "updated". const summary = window.PluginInstallManager.summarizeUpdateResults(results); showNotification(summary.text, summary.type); + // An updated plugin the display is running keeps its old code + // until the display restarts. + window.noteRestartRequired(window.PluginInstallManager.restartRequest(results)); }) .catch(error => { console.error('Error updating all plugins:', error); @@ -2062,6 +2066,7 @@ window.uninstallPlugin = function(pluginId) { pollOperationStatus(operationId, pluginId, pluginName); } else if (data.status === 'success') { // Direct uninstall completed immediately + window.noteRestartRequired(data); handleUninstallSuccess(pluginId); } else { // Error response @@ -2104,6 +2109,9 @@ function pollOperationStatus(operationId, pluginId, pluginName, options = {}) { const status = operation.status; if (status === 'completed') { + // The operation's result says whether the display picks + // the change up by itself or needs a restart. + window.noteRestartRequired(operation.result); onComplete(); } else if (status === 'failed') { onFailed(operation.error || operation.message); @@ -2549,6 +2557,7 @@ window.installPlugin = function(pluginId, branch = null) { }); } else { // No operation queue configured - install already completed synchronously. + window.noteRestartRequired(data); enableAfterInstall(); } }) @@ -2579,6 +2588,7 @@ window.installFromCustomRegistry = function(pluginId, registryUrl, pluginPath, b .then(data => { if (data.status === 'success') { showNotification(`Plugin ${data.plugin_id} installed successfully`, 'success'); + window.noteRestartRequired(data); // Refresh installed plugins and re-render custom registry loadInstalledPlugins().catch(() => {}); // Re-render custom registry to update install buttons @@ -2771,6 +2781,7 @@ function attachInstallButtonHandler() { pluginStatusDiv.innerHTML = `Successfully installed: ${escapeHtml(data.plugin_id)}`; } pluginUrlInput.value = ''; + window.noteRestartRequired(data); // Refresh installed plugins list setTimeout(() => { diff --git a/web_interface/templates/v3/base.html b/web_interface/templates/v3/base.html index a7a4c1f6..87c743f1 100644 --- a/web_interface/templates/v3/base.html +++ b/web_interface/templates/v3/base.html @@ -456,10 +456,11 @@ - +