diff --git a/CHANGELOG.md b/CHANGELOG.md index d70034fe..892b83c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -175,6 +175,41 @@ read any of them: 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. +- The display publishes its plugin runtime state, and the web interface + reads it (web plugin catalog, stage 2). A new snapshot in the shared cache + (`plugin_runtime_snapshot`, `src/plugin_system/plugin_runtime.py`) lists, + per plugin, whether the display has it loaded, its lifecycle state, a + short redacted summary of its last error, the version it loaded and when. + It is written when something changes (at most every 10 s; an ordinary + plugin update is not a change) and otherwise once a minute, carries its + publish time, and says `running: false` when the display stops. + - `/api/v3/plugins/installed` fills `loaded`, `state` and `error_info` + again, from that snapshot, and adds `loaded_version` and `loaded_at`. + Only a live snapshot counts: when the display is stopped, has not + published, or has not refreshed for 3 minutes, those fields are `null` + and the new `data.runtime.status` says `stopped`, `unknown` or `stale`. + - `data/plugin_state.json` is retired: nothing reads or writes it. It held + copies of config.json's enabled flags and the manifests' versions, plus + install timestamps only `GET /api/v3/plugins/state` returned, so nothing + in it is migrated; an existing file is left in place and can be deleted. + The web-side `PluginStateManager` (`src/plugin_system/state_manager.py`) + that wrote it is removed; the display's state machine in + `plugin_state.py` is now the only `PluginStateManager`. + - `GET /api/v3/plugins/state` is built per request from config.json, the + plugins on disk and the display's snapshot (`installed`, `in_config`, + `enabled`, `version`, `status`, the runtime fields, and `installed_at` / + `last_updated` from the operation history), with a top-level `runtime`. + It no longer returns `config_version` or `metadata`. + - State reconciliation compares desired state (config.json plus disk) with + the display's snapshot. New findings -- enabled but not loaded (with the + load error), and loaded at an older version than is installed -- are + reported with `fix_action: no_action`; the unresolved-issues banner is + unchanged. `StateReconciliation` takes `config_manager`, `plugins_dir`, + `store_manager` and `runtime_source` as keywords. + - Backups list the installed plugins from disk, with `enabled` from + config.json, instead of merging in `plugin_state.json`. A plugin that + only that file still named (not installed, not configured) is no longer + listed. Restores are unchanged. ### Fixes diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 82ae99e7..be73bab3 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -48,6 +48,7 @@ each other. They share three things: | Error clear | cache `plugin_error_clear_request` | web | display | | Font usage | cache `font_usage_snapshot` | display: `FontUsagePublisher` ([`src/font_usage.py`](../src/font_usage.py)) | web: Fonts tab | | Plugin health | cache `plugin_health:` | display (web writes on reset) | web: `/api/v3/plugins/health` | +| Plugin runtime (loaded, state, last error, version) | cache `plugin_runtime_snapshot` | display: `PluginRuntimePublisher` ([`src/plugin_system/plugin_runtime.py`](../src/plugin_system/plugin_runtime.py)) | web: `read_plugin_runtime()` for `/api/v3/plugins/installed`, `/plugins/state`, reconciliation | | Preview frame | `/tmp/led_matrix_preview.png` | display: `DisplayManager`, gated by [`snapshot_policy`](../src/common/snapshot_policy.py) | web: display SSE stream, `/api/v3/health` (file age) | | Preview viewer marker | `/tmp/led_matrix_preview_viewer` | web, while a preview is open | display: writes full-rate snapshots only while it is fresh | | Hardware init status | `/tmp/led_matrix_hw_status.json` | display | web: `/api/v3/hardware/status` | @@ -85,12 +86,10 @@ routes return it as `restart_required` (with the banner's wording 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 +Runtime state shown in the UI comes from what the display publishes to the +shared cache: health and metrics (`/api/v3/plugins/health`, +`/plugins/metrics`), errors (`/api/v3/errors/*`), the current mode, and the +plugin runtime snapshot described below. `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, @@ -103,14 +102,68 @@ 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`. +Next stages: a **control socket** from the web process to the display +(reload one plugin, ask for its state) in place of `restart_required` and +the cache-key mailboxes, and the plugin web-entry contract above. + +### Plugin state: desired, observed, and who owns it + +There is one plugin state machine, and the display owns it: +`PluginStateManager` in +[`plugin_state.py`](../src/plugin_system/plugin_state.py) (unloaded → +loaded → enabled ⇄ running, error, disabled), held by the display's +`PluginManager`. It also records, per loaded plugin, the manifest version it +loaded and when. Nothing else keeps plugin state: + +| Question | Answered by | +|---|---| +| Is it installed, at which version? | the plugins directory (`manifest.json`) | +| Should it run? | `config.json` (`.enabled`, missing = disabled) | +| Has the user uninstalled it for good? | the store's uninstalled-plugins record | +| Is the display running it, at which version, and why not? | the display's runtime snapshot | + +**The runtime snapshot.** `PluginRuntimePublisher` +([`plugin_runtime.py`](../src/plugin_system/plugin_runtime.py)), started by +`DisplayController` right after it creates the `PluginManager`, writes the +cache key `plugin_runtime_snapshot`: per plugin `loaded`, `state`, `error` +(type, a redacted message of at most 200 characters, when, recoverable), +`version` and `loaded_at`, plus `published_at`, `stale_after` and `running`. +The cache is on disk, usually the SD card, so it writes when something a +reader sees changes -- throttled to once per 10 s -- and otherwise once a +minute as a heartbeat. RUNNING, which every `update()` passes through, is +published as ENABLED, so plugin updates alone never cause a write. +`cleanup()` publishes `running: false`. + +**Reading it.** `read_plugin_runtime()` judges the snapshot before anyone +uses it: `live` (fresh, from a running display), `stale` (older than +`stale_after`, 3 minutes: a hung or crashed display), `stopped` or +`unknown` (none, unreadable, or another schema). Only a live view reports +per-plugin facts; every other status answers `null` for them, so stale +truth cannot leak into a response. `/api/v3/plugins/installed` returns +`loaded`, `state`, `error_info`, `loaded_version` and `loaded_at` per +plugin and `data.runtime` (`status`, `published_at`, `age_seconds`); +`/api/v3/plugins/state` returns the same beside the desired state. + +**Reconciliation** +([`state_reconciliation.py`](../src/plugin_system/state_reconciliation.py)) +compares desired state (config + disk) with observed state (the snapshot). +It fixes desired-state gaps -- a plugin on disk with no config section gets +`{"enabled": false}`, a configured plugin missing from disk is reinstalled +unless the user uninstalled it -- and only reports observed-state gaps +(enabled but not loaded, loaded at an older version): the display loads and +unloads by config on its own, and a version gap needs a restart. + +**`data/plugin_state.json` is retired.** The web process used to keep a +second `PluginStateManager` (`state_manager.py`) persisted to that file: +per plugin an enabled flag copied from config, a version copied from the +manifest (when set at all), a status derived from those, and install/update +timestamps. Reconciliation mostly synced it back to config and backups +merged it into their plugin list. Every field is derivable (the timestamps +from the operation history), so nothing is migrated: no code reads or +writes the file, and a copy left on a device is inert and safe to delete. +The two classes shared a name but not a concern -- a persisted install +record versus the live lifecycle -- so they were not merged; the persisted +one had nothing left to hold and was removed. ## Display loop diff --git a/docs/REST_API_REFERENCE.md b/docs/REST_API_REFERENCE.md index 541dde50..5db4cf55 100644 --- a/docs/REST_API_REFERENCE.md +++ b/docs/REST_API_REFERENCE.md @@ -509,9 +509,11 @@ List all installed plugins with their status and metadata. "tags": ["sports", "football", "nfl"], "enabled": true, "verified": true, - "loaded": null, - "state": null, + "loaded": true, + "state": "enabled", "error_info": null, + "loaded_version": "1.2.3", + "loaded_at": 1790000000.0, "last_updated": "2025-01-15T10:30:00Z", "last_commit": "abc1234", "last_commit_message": "feat: Add live game updates", @@ -522,17 +524,37 @@ List all installed plugins with their status and metadata. "vegas_participation": "scroll", "vegas_participation_source": "manifest" } - ] + ], + "runtime": { + "status": "live", + "published_at": 1790000030.0, + "age_seconds": 12.4, + "stale_after": 180.0 + } } } ``` 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/*`. +reads it). `vegas_mode` is the plugin's configured `vegas_mode`, or `null`. + +`loaded`, `state`, `error_info`, `loaded_version` and `loaded_at` come from +the runtime snapshot the display publishes (the web process runs no plugin +code). `state` is the display's lifecycle state (`loaded` while loading, +`enabled`, `disabled`, `error`, `unloaded`); `error_info` is `null` or +`{"type", "message", "at", "recoverable"}`, with the message redacted and at +most 200 characters (the full error is at `/errors/*`). `loaded_version` is +the version the display loaded, which differs from `version` after an update +until the display restarts. A plugin a live snapshot does not list is +`loaded: false`, `state: "unloaded"`. + +`runtime.status` says whether to believe them: `live` (fresh snapshot from +a running display), `stale` (not refreshed within `stale_after` seconds: the +display is hung or died), `stopped` (the display shut down) or `unknown` +(nothing published yet). Unless it is `live`, every one of those fields is +`null`. Health and metrics are at [`/plugins/health`](#get-plugin-health) +and `/plugins/metrics`. `vegas_participation` is what Vegas mode does with the plugin: `"scroll"`, `"pause"` or `"exclude"` (see @@ -545,8 +567,7 @@ 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`. +manifest's declaration. `vegas_content_type` is always `null`. ### Get Plugin Configuration @@ -990,8 +1011,11 @@ copy, not the display service's in-memory state. **GET** `/api/v3/plugins/state` -Get the state manager's record for every plugin, keyed by plugin id. Pass -`?plugin_id=` for one plugin (`data` is then that record). +Every plugin that is installed or configured, keyed by plugin id: desired +state from `config.json` and the plugins directory, observed state from the +display's runtime snapshot. Built per request; there is no state file. +Pass `?plugin_id=` for one plugin (`data` is then that record; 404 if +it is neither installed nor configured). **Response**: ```json @@ -1000,23 +1024,41 @@ Get the state manager's record for every plugin, keyed by plugin id. Pass "data": { "football-scoreboard": { "plugin_id": "football-scoreboard", - "status": "loaded", + "status": "enabled", + "installed": true, + "in_config": true, "enabled": true, "version": "1.2.3", + "loaded": true, + "state": "enabled", + "error_info": null, + "loaded_version": "1.2.3", + "loaded_at": 1790000000.0, "installed_at": "2025-01-15T10:30:00", - "last_updated": "2025-01-15T10:30:00", - "config_version": 1, - "metadata": {} + "last_updated": "2025-01-15T10:30:00" } - } + }, + "runtime": {"status": "live", "published_at": 1790000030.0, "age_seconds": 12.4, "stale_after": 180.0} } ``` +`status` is `enabled` / `disabled` for an installed plugin, `unknown` for +one that is configured but not installed, and `error` when the display +reports its state as `error`. `installed_at` and `last_updated` are the +newest successful install, and install or update, in the operation history +(`null` when it has none). The runtime fields follow the same rule as +[`/plugins/installed`](#get-installed-plugins): `null` unless +`runtime.status` is `live`. + ### Reconcile Plugin State **POST** `/api/v3/plugins/state/reconcile` -Reconcile plugin state across config, disk and the state manager. +Reconcile desired state (`config.json` plus the plugins on disk) with the +display's runtime snapshot. Desired-state gaps are fixed (a plugin on disk +with no config section is added disabled); observed-state gaps -- enabled +but not loaded, loaded at an older version than is installed -- are +reported with `fix_action: "no_action"`. **Request Body** (optional): ```json diff --git a/mypy-clean.txt b/mypy-clean.txt index 2e969bae..e1381349 100644 --- a/mypy-clean.txt +++ b/mypy-clean.txt @@ -59,6 +59,7 @@ src/plugin_system/plugin_dirs.py src/plugin_system/plugin_executor.py src/plugin_system/plugin_health.py src/plugin_system/plugin_loader.py +src/plugin_system/plugin_runtime.py src/plugin_system/plugin_state.py src/plugin_system/repo_urls.py src/plugin_system/resource_monitor.py diff --git a/src/backup_manager.py b/src/backup_manager.py index 5f61e3f9..bcd7b342 100644 --- a/src/backup_manager.py +++ b/src/backup_manager.py @@ -88,7 +88,6 @@ _WIFI_REL = Path("config/wifi_config.json") _YTM_REL = Path("config/ytm_auth.json") _FONTS_REL = Path("assets/fonts") _PLUGIN_UPLOADS_REL = Path("assets/plugins") -_STATE_REL = Path("data/plugin_state.json") #: The sections that are one file each: (section name, path, the #: RestoreOptions flag that restores it). create, preview, validate and @@ -179,20 +178,27 @@ def _build_manifest(contents: List[str]) -> Dict[str, Any]: # --------------------------------------------------------------------------- -def _plugins_directory(project_root: Path) -> Path: - """The plugin install directory: ``plugin_system.plugins_directory`` from - config/config.json (relative to ``project_root`` unless absolute), or - ``plugin-repos`` when the config does not say or cannot be read.""" - configured: Any = None +def _read_config(project_root: Path) -> Dict[str, Any]: + """config/config.json as a dict; empty when missing or unreadable.""" try: with (project_root / _CONFIG_REL).open("r", encoding="utf-8") as f: config = json.load(f) - if isinstance(config, dict): - plugin_system = config.get("plugin_system") - if isinstance(plugin_system, dict): - configured = plugin_system.get("plugins_directory") except (OSError, json.JSONDecodeError): - pass + return {} + return config if isinstance(config, dict) else {} + + +def _plugins_directory(project_root: Path, + config: Optional[Dict[str, Any]] = None) -> Path: + """The plugin install directory: ``plugin_system.plugins_directory`` from + config/config.json (relative to ``project_root`` unless absolute), or + ``plugin-repos`` when the config does not say or cannot be read.""" + if config is None: + config = _read_config(project_root) + configured: Any = None + plugin_system = config.get("plugin_system") + if isinstance(plugin_system, dict): + configured = plugin_system.get("plugins_directory") if not isinstance(configured, str) or not configured.strip(): configured = "plugin-repos" path = Path(configured) @@ -202,33 +208,23 @@ def _plugins_directory(project_root: Path) -> Path: def list_installed_plugins(project_root: Path) -> List[Dict[str, Any]]: """ Return a list of currently-installed plugins suitable for the backup - manifest. Each entry has ``plugin_id`` and ``version``. + manifest. Each entry has ``plugin_id``, ``version`` and ``enabled``. - Reads ``data/plugin_state.json`` if present, then adds any plugin it - does not list from the ``manifest.json`` files in the configured plugin - directory (see :func:`_plugins_directory`). + The plugins are the ``manifest.json`` files in the configured plugin + directory (see :func:`_plugins_directory`), with the manifest's version; + ``enabled`` is config.json's flag by the display's rule (a missing flag + is disabled). A restore reinstalls every listed plugin and takes enabled + state from the restored config.json, so ``enabled`` is informational. + + ``data/plugin_state.json`` is not read: it only ever repeated config's + enabled flags and the manifests' versions, and is retired (nothing + writes it any more). An old backup that listed a plugin only from that + file still restores it, since restore reads ``plugins.json`` as written. """ plugins: Dict[str, Dict[str, Any]] = {} + config = _read_config(project_root) - state_file = project_root / _STATE_REL - if state_file.exists(): - try: - with state_file.open("r", encoding="utf-8") as f: - state = json.load(f) - raw_plugins = state.get("states", {}) if isinstance(state, dict) else {} - if isinstance(raw_plugins, dict): - for plugin_id, info in raw_plugins.items(): - if not isinstance(info, dict): - continue - plugins[plugin_id] = { - "plugin_id": plugin_id, - "version": info.get("version") or "", - "enabled": bool(info.get("enabled", True)), - } - except (OSError, json.JSONDecodeError) as e: - logger.warning("Could not read plugin_state.json: %s", e) - - plugins_root = _plugins_directory(project_root) + plugins_root = _plugins_directory(project_root, config) if plugins_root.exists(): for entry in sorted(plugins_root.iterdir()): if not entry.is_dir(): @@ -247,10 +243,11 @@ def list_installed_plugins(project_root: Path) -> List[Dict[str, Any]]: continue plugin_id = data.get("id") or entry.name if plugin_id not in plugins: + section = config.get(plugin_id) plugins[plugin_id] = { "plugin_id": plugin_id, "version": data.get("version", ""), - "enabled": True, + "enabled": isinstance(section, dict) and bool(section.get("enabled", False)), } return sorted(plugins.values(), key=lambda p: p["plugin_id"]) diff --git a/src/display_controller.py b/src/display_controller.py index 9d61a333..df148f10 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -218,6 +218,7 @@ class DisplayController: # Initialize Plugin System plugin_time = time.time() self.plugin_manager = None + self._plugin_runtime_publisher = None self.plugin_modes = {} # mode -> plugin_instance mapping for plugin-first dispatch self.mode_to_plugin_id: Dict[str, str] = {} self.plugin_display_modes: Dict[str, List[str]] = {} @@ -322,6 +323,13 @@ class DisplayController: font_manager=self.font_manager ) + # The web UI's loaded / state / error_info for each plugin read + # what this publishes. Started before loading, so the loads that + # follow are published as they land. + from src.plugin_system.plugin_runtime import start_plugin_runtime_publisher + self._plugin_runtime_publisher = start_plugin_runtime_publisher( + self.cache_manager, self.plugin_manager.state_manager) + # Activate the plugin health/metrics subsystem. PluginManager leaves # health_tracker/resource_monitor as None by default; wiring real # instances here turns on the circuit breaker (a repeatedly-failing @@ -452,6 +460,12 @@ class DisplayController: except Exception: # pylint: disable=broad-except logger.exception("Plugin system initialization failed") self.plugin_manager = None + # Its state machine no longer describes what runs; let the last + # snapshot go stale (readers then say unknown) rather than keep + # refreshing it. + if self._plugin_runtime_publisher is not None: + self._plugin_runtime_publisher.stop(publish_stopped=False) + self._plugin_runtime_publisher = None # The web UI's Fonts tab ("Used by") reads what this publishes. from src.font_usage import start_font_usage_publisher @@ -3641,6 +3655,12 @@ class DisplayController: logger.warning("Error shutting down config service: %s", e) if getattr(self, '_font_usage_publisher', None) is not None: self._font_usage_publisher.stop() + # Publishes "stopped", so the web UI stops reporting what was loaded. + if getattr(self, '_plugin_runtime_publisher', None) is not None: + try: + self._plugin_runtime_publisher.stop() + except Exception as e: + logger.warning("Error stopping the plugin runtime publisher: %s", e) logger.info("Cleaning up display controller...") if hasattr(self, 'display_manager'): self.display_manager.cleanup() diff --git a/src/plugin_system/plugin_manager.py b/src/plugin_system/plugin_manager.py index 3d73f3c3..7b02ad22 100644 --- a/src/plugin_system/plugin_manager.py +++ b/src/plugin_system/plugin_manager.py @@ -522,7 +522,13 @@ class PluginManager: raise else: self.state_manager.set_state(plugin_id, PluginState.DISABLED) - + + # The version this instance runs, for the runtime snapshot the + # web UI reads: the manifest on disk can move on after an update. + version = manifest.get('version') + self.state_manager.record_loaded( + plugin_id, version if isinstance(version, str) else None) + self.logger.info("Loaded plugin: %s", plugin_id) return True @@ -774,6 +780,9 @@ class PluginManager: except Exception as e: self.logger.error("Error unloading plugin %s: %s", plugin_id, e, exc_info=True) self.state_manager.set_state(plugin_id, PluginState.ERROR, error=e) + if plugin_id not in self.plugins: + # Failed after the instance was dropped: it is not loaded. + self.state_manager.record_unloaded(plugin_id) return False def reload_plugin(self, plugin_id: str) -> bool: diff --git a/src/plugin_system/plugin_runtime.py b/src/plugin_system/plugin_runtime.py new file mode 100644 index 00000000..8b74fc38 --- /dev/null +++ b/src/plugin_system/plugin_runtime.py @@ -0,0 +1,377 @@ +"""The display's plugin runtime snapshot, shared with the web interface. + +Only the display process runs plugins, so only it knows which ones it has +loaded, where each is in its lifecycle (``plugin_state.PluginStateManager``), +why one failed and which version it is running. It publishes that to the +shared cache directory -- the channel, and the file permissions, that the +error snapshot, plugin health and ``display_current_state`` already use -- +and the web interface reads it back for ``/api/v3/plugins/installed``, +``/api/v3/plugins/state`` and state reconciliation. + + PLUGIN_RUNTIME_KEY written by the display service only + +Writes. The cache lives on disk, usually the SD card, so the snapshot is +written when something a reader would see changes, at most once every +``MIN_INTERVAL`` seconds, and otherwise once every ``REFRESH_INTERVAL`` +seconds as a heartbeat. An ordinary plugin update is not a change: the +RUNNING state it passes through is published as ENABLED +(``plugin_state.published_state``). A display with nothing changing writes +this one small file once a minute. + +Staleness. Every snapshot carries ``published_at`` (wall clock) and +``stale_after``. A reader treats a snapshot older than that as unknown, not +as the truth: a display that died without cleaning up leaves its last +snapshot behind. A display that stops cleanly publishes ``running: false`` +on the way out, so readers see "stopped" at once rather than after the +stale window. Nothing on the reading side reports a runtime fact from a +snapshot that is not live. +""" + +import math +import os +import threading +import time +from dataclasses import dataclass, field +from typing import Any, Callable, Dict, Optional + +from src.logging_config import get_logger +from src.redaction import redact_credentials + +logger = get_logger(__name__) + +PLUGIN_RUNTIME_KEY = "plugin_runtime_snapshot" +SNAPSHOT_SCHEMA = 1 + +#: Shortest gap, in seconds, between two change-driven writes. Startup loads +#: every plugin in a burst, and a plugin failing each update cycle changes its +#: error info each time; either is written at most this often. +MIN_INTERVAL = 10.0 + +#: An unchanged snapshot is rewritten this often so readers can tell a quiet +#: display from a dead one. +REFRESH_INTERVAL = 60.0 + +#: How often the publisher thread looks for changes: an in-memory comparison. +TICK_INTERVAL = 5.0 + +#: A snapshot older than this is stale: three missed refreshes. +STALE_AFTER = 3 * REFRESH_INTERVAL + +#: Bounds on a published ``stale_after``, so a corrupt value can make a +#: reader neither trust a dead display for hours nor distrust a live one. +_STALE_AFTER_MIN = 30.0 +_STALE_AFTER_MAX = 3600.0 + +_ERROR_MESSAGE_CHARS = 200 +_ERROR_TYPE_CHARS = 80 +_ID_CHARS = 100 +_VERSION_CHARS = 40 + +#: Reader statuses. Only LIVE carries runtime facts. +LIVE = "live" +STALE = "stale" +STOPPED = "stopped" +UNKNOWN = "unknown" + + +def _clip(value: Any, limit: int) -> str: + text = value if isinstance(value, str) else str(value) + return text if len(text) <= limit else text[:limit - 3] + "..." + + +def _epoch(value: Any) -> Optional[float]: + """Seconds since the epoch for a float or a datetime; None otherwise.""" + if isinstance(value, bool): + return None + if isinstance(value, (int, float)): + number = float(value) + return number if math.isfinite(number) else None + timestamp = getattr(value, "timestamp", None) + if callable(timestamp): + try: + number = float(timestamp()) + except (TypeError, ValueError, OverflowError, OSError): + return None + return number if math.isfinite(number) else None + return None + + +def summarize_error(error_info: Optional[Dict[str, Any]]) -> Optional[Dict[str, Any]]: + """A short, redacted summary of the state machine's error info. + + ``message`` is redacted before it is clipped: clipping first could cut a + ``token=`` marker off and keep the secret after it. No stack trace: the + full error, with its trace, is in the error snapshot (/api/v3/errors). + """ + if not isinstance(error_info, dict): + return None + message = error_info.get("error") + error_type = error_info.get("error_type") + return { + "type": _clip(error_type, _ERROR_TYPE_CHARS) if error_type else None, + "message": _clip(redact_credentials(message if isinstance(message, str) + else str(message or "")), + _ERROR_MESSAGE_CHARS), + "at": _epoch(error_info.get("timestamp")), + "recoverable": bool(error_info.get("recoverable", False)), + } + + +def build_runtime_snapshot(state_manager: Any, *, started_at: float, + now: Optional[float] = None, + running: bool = True) -> Dict[str, Any]: + """The snapshot for ``state_manager`` (a plugin_state.PluginStateManager). + + A stopped snapshot (``running=False``) lists no plugins: nothing is + loaded once the display has gone. + """ + plugins: Dict[str, Dict[str, Any]] = {} + if running: + for plugin_id, record in state_manager.runtime_records().items(): + version = record.get("version") + plugins[_clip(plugin_id, _ID_CHARS)] = { + "loaded": bool(record.get("loaded")), + "state": record.get("state"), + "error": summarize_error(record.get("error_info")), + "version": _clip(version, _VERSION_CHARS) if version else None, + "loaded_at": _epoch(record.get("loaded_at")), + } + return { + "schema": SNAPSHOT_SCHEMA, + "running": running, + "published_at": time.time() if now is None else now, + "started_at": started_at, + "refresh_interval": REFRESH_INTERVAL, + "stale_after": STALE_AFTER, + "pid": os.getpid(), + "plugins": plugins, + } + + +class PluginRuntimePublisher: + """Publishes the display's plugin state machine to the shared cache. + + Runs in the display service only. tick() is the whole job; start() calls + it from a daemon thread every TICK_INTERVAL seconds. Nothing here raises: + a failed write is logged at debug and retried on a later tick, at the + throttled rate. + """ + + def __init__(self, cache_manager: Any, state_manager: Any, + min_interval: float = MIN_INTERVAL, + refresh_interval: float = REFRESH_INTERVAL, + clock: Callable[[], float] = time.monotonic, + wall_clock: Callable[[], float] = time.time) -> None: + self.cache_manager = cache_manager + self.state_manager = state_manager + self.min_interval = min_interval + self.refresh_interval = refresh_interval + self._clock = clock + self._wall_clock = wall_clock + self.started_at = wall_clock() + # None forces a first publish, which replaces whatever a previous run + # of the service left behind. + self._published_change: Optional[int] = None + self._last_attempt: Optional[float] = None + self._tick_lock = threading.Lock() + self._stop = threading.Event() + self._thread: Optional[threading.Thread] = None + + def _write(self, running: bool) -> None: + snapshot = build_runtime_snapshot(self.state_manager, started_at=self.started_at, + now=self._wall_clock(), running=running) + self.cache_manager.set(PLUGIN_RUNTIME_KEY, snapshot) + + def tick(self) -> bool: + """Publish if something changed (throttled) or the refresh is due. + True if a snapshot was written.""" + with self._tick_lock: + try: + change = self.state_manager.change_count + now = self._clock() + since = None if self._last_attempt is None else now - self._last_attempt + if since is not None: + if change == self._published_change: + if since < self.refresh_interval: + return False + elif since < self.min_interval: + return False + # Stamp the attempt before writing: a cache that keeps failing + # is retried at the throttled rate, not on every tick. + self._last_attempt = now + self._write(running=True) + self._published_change = change + return True + except Exception as err: # never let reporting break the display + logger.debug("Could not publish the plugin runtime snapshot: %s", + err, exc_info=True) + return False + + def start(self, interval: float = TICK_INTERVAL) -> None: + """Tick from a daemon thread until stop(). A no-op while running.""" + if self._thread is not None and self._thread.is_alive(): + return + self._stop.clear() + + def run() -> None: + self.tick() + while not self._stop.wait(interval): + self.tick() + + self._thread = threading.Thread(target=run, name="plugin-runtime-publisher", + daemon=True) + self._thread.start() + + def stop(self, publish_stopped: bool = True) -> None: + """Stop ticking and, by default, publish ``running: false`` so readers + see the display as stopped now rather than after the stale window.""" + self._stop.set() + if self._thread is not None: + self._thread.join(timeout=2) + self._thread = None + if publish_stopped: + with self._tick_lock: + try: + self._write(running=False) + except Exception as err: + logger.debug("Could not publish the stopped plugin runtime snapshot: %s", + err, exc_info=True) + + +def start_plugin_runtime_publisher(cache_manager: Any, + state_manager: Any) -> Optional[PluginRuntimePublisher]: + """Start publishing the display's plugin runtime state. Display service + only: whichever process calls it becomes the source readers trust. + Never raises.""" + try: + publisher = PluginRuntimePublisher(cache_manager, state_manager) + publisher.start() + return publisher + except Exception as err: + logger.warning("Plugin runtime reporting to the web interface is unavailable: %s", err) + return None + + +# --- Reading side (web interface) ------------------------------------------- + +#: What a reader reports for a plugin when it does not know. +_UNKNOWN_PLUGIN: Dict[str, Any] = { + "loaded": None, + "state": None, + "error_info": None, + "loaded_version": None, + "loaded_at": None, +} + +#: A plugin a live snapshot does not list: the display has not loaded it +#: (never enabled, or unloaded since), which is what its state machine +#: reports for an id it has no record of. +_NOT_LOADED_PLUGIN: Dict[str, Any] = { + "loaded": False, + "state": "unloaded", + "error_info": None, + "loaded_version": None, + "loaded_at": None, +} + + +@dataclass(frozen=True) +class PluginRuntimeView: + """What a reader may say about the display's plugins right now. + + ``status``: ``live`` (a fresh snapshot from a running display), + ``stale`` (the last snapshot is older than its ``stale_after``: the + display is hung or died without cleaning up), ``stopped`` (the display + said so on its way out) or ``unknown`` (no readable snapshot). Only a + live view reports per-plugin facts; every other status answers None for + them, so a caller cannot pass stale truth on by accident. + """ + + status: str + published_at: Optional[float] = None + age_seconds: Optional[float] = None + stale_after: float = STALE_AFTER + plugins: Dict[str, Dict[str, Any]] = field(default_factory=dict) + + @property + def live(self) -> bool: + return self.status == LIVE + + def plugin(self, plugin_id: str) -> Dict[str, Any]: + """``loaded``, ``state``, ``error_info``, ``loaded_version`` and + ``loaded_at`` for one plugin; all None unless the view is live.""" + if not self.live: + return dict(_UNKNOWN_PLUGIN) + record = self.plugins.get(plugin_id) + if not isinstance(record, dict): + return dict(_NOT_LOADED_PLUGIN) + error = record.get("error") + return { + "loaded": bool(record.get("loaded")), + "state": record.get("state") if isinstance(record.get("state"), str) else None, + "error_info": dict(error) if isinstance(error, dict) else None, + "loaded_version": record.get("version"), + "loaded_at": record.get("loaded_at"), + } + + def describe(self) -> Dict[str, Any]: + """The view's own status, for a response to carry beside the facts.""" + return { + "status": self.status, + "published_at": self.published_at, + "age_seconds": None if self.age_seconds is None else round(self.age_seconds, 1), + "stale_after": self.stale_after, + } + + +def _stale_after_of(snapshot: Dict[str, Any]) -> float: + value = snapshot.get("stale_after") + if isinstance(value, bool) or not isinstance(value, (int, float)): + return STALE_AFTER + number = float(value) + if not math.isfinite(number): + return STALE_AFTER + return min(max(number, _STALE_AFTER_MIN), _STALE_AFTER_MAX) + + +def view_from_snapshot(snapshot: Any, now: Optional[float] = None) -> PluginRuntimeView: + """Judge a snapshot read from the cache; never raises.""" + if not isinstance(snapshot, dict) or snapshot.get("schema") != SNAPSHOT_SCHEMA: + return PluginRuntimeView(status=UNKNOWN) + published_at = _epoch(snapshot.get("published_at")) + if published_at is None: + return PluginRuntimeView(status=UNKNOWN) + stale_after = _stale_after_of(snapshot) + age = (time.time() if now is None else now) - published_at + if snapshot.get("running") is not True: + return PluginRuntimeView(status=STOPPED, published_at=published_at, + age_seconds=max(age, 0.0), stale_after=stale_after) + # A snapshot from the future is trusted a little: the Pi has no RTC and + # its clock steps when NTP syncs. Far in the future, it cannot be dated. + if age > stale_after or age < -stale_after: + return PluginRuntimeView(status=STALE, published_at=published_at, + age_seconds=age, stale_after=stale_after) + plugins = snapshot.get("plugins") + return PluginRuntimeView( + status=LIVE, published_at=published_at, age_seconds=max(age, 0.0), + stale_after=stale_after, + plugins={k: v for k, v in plugins.items() if isinstance(v, dict)} + if isinstance(plugins, dict) else {}, + ) + + +def read_plugin_runtime(cache_manager: Any, now: Optional[float] = None) -> PluginRuntimeView: + """The display's latest snapshot, judged for staleness. Never raises; a + missing cache manager or an unreadable snapshot is ``unknown``. + + memory_ttl=0: the key is written by the other process, so only the file + is current. + """ + if cache_manager is None: + return PluginRuntimeView(status=UNKNOWN) + try: + snapshot = cache_manager.get(PLUGIN_RUNTIME_KEY, max_age=None, memory_ttl=0) + except Exception as err: + logger.debug("Could not read the plugin runtime snapshot: %s", err, exc_info=True) + return PluginRuntimeView(status=UNKNOWN) + return view_from_snapshot(snapshot, now=now) diff --git a/src/plugin_system/plugin_state.py b/src/plugin_system/plugin_state.py index 2f4b9641..17755890 100644 --- a/src/plugin_system/plugin_state.py +++ b/src/plugin_system/plugin_state.py @@ -1,11 +1,14 @@ """ Plugin State Management -Manages plugin state machine (loaded → enabled → running → error) -with state transitions and queries. +The display process's plugin state machine (loaded → enabled → running → +error), with state transitions and queries. It is the only record of plugin +lifecycle state: the web process runs no plugins, and reads this state as the +snapshot ``plugin_runtime.PluginRuntimePublisher`` publishes from it. """ import threading +import time from enum import Enum from typing import Optional, Dict, Any from datetime import datetime @@ -24,8 +27,27 @@ class PluginState(Enum): DISABLED = "disabled" # Plugin is disabled in config +def published_state(state: PluginState) -> PluginState: + """The state as readers outside the scheduler see it. + + RUNNING is the scheduler's claim on a plugin for one update() call: every + update flips ENABLED -> RUNNING -> ENABLED. Published as is, that would be + a change -- and a cache write to the SD card -- on every plugin update, and + a reader would see a plugin blink between two states that mean the same + thing to it (loaded and taking part). Readers get ENABLED for both. + """ + return PluginState.ENABLED if state == PluginState.RUNNING else state + + class PluginStateManager: - """Manages plugin state transitions and queries.""" + """Manages plugin state transitions and queries. + + Owned by the display process's PluginManager. ``change_count`` moves + whenever something a reader of the published snapshot would see changes + (published state, error info, the loaded record) and stays put across the + RUNNING/ENABLED flip of an ordinary update, so a publisher can tell + "nothing new" without diffing. + """ def __init__(self, logger: Optional[logging.Logger] = None) -> None: """ @@ -41,6 +63,20 @@ class PluginStateManager: self._state_transition_counts: Dict[str, int] = {} self._error_info: Dict[str, Dict[str, Any]] = {} self._last_update: Dict[str, datetime] = {} + # What load_plugin() registered: {'version', 'loaded_at'} per plugin + # whose instance is live. Cleared with the rest of its state on unload. + self._loaded: Dict[str, Dict[str, Any]] = {} + self._change_count = 0 + + @property + def change_count(self) -> int: + """Moves on every change a published snapshot would show.""" + with self._lock: + return self._change_count + + def _note_change(self) -> None: + """Count a reader-visible change. Callers must already hold ``_lock``.""" + self._change_count += 1 def _record_transition(self, plugin_id: str) -> None: """Count a state transition. Callers must already hold ``_lock``.""" @@ -63,9 +99,12 @@ class PluginStateManager: error: Optional error if transitioning to ERROR state """ with self._lock: + known = plugin_id in self._states old_state = self._states.get(plugin_id, PluginState.UNLOADED) self._states[plugin_id] = state self._record_transition(plugin_id) + if not known or published_state(old_state) != published_state(state): + self._note_change() # Store error info if transitioning to ERROR state if state == PluginState.ERROR and error: @@ -74,9 +113,11 @@ class PluginStateManager: 'error_type': type(error).__name__, 'timestamp': datetime.now() } + self._note_change() elif state != PluginState.ERROR: # Clear error info when leaving ERROR state - self._error_info.pop(plugin_id, None) + if self._error_info.pop(plugin_id, None) is not None: + self._note_change() self.logger.debug( "Plugin %s state transition: %s → %s", @@ -147,6 +188,7 @@ class PluginStateManager: self._states[plugin_id] = state self._record_transition(plugin_id) self._error_info[plugin_id] = dict(error_info) + self._note_change() self.logger.debug( "Plugin %s state transition: %s → %s (recoverable error stored)", @@ -173,6 +215,52 @@ class PluginStateManager: info = self._error_info.get(plugin_id) return dict(info) if info is not None else None + def record_loaded(self, plugin_id: str, version: Optional[str], + loaded_at: Optional[float] = None) -> None: + """Record that ``plugin_id``'s instance is live, and which version. + + Called by PluginManager.load_plugin() once the instance is registered; + clear_state() (unload) forgets it. ``version`` is the manifest's at + load time, which is what the display keeps running until it reloads + the plugin -- the version on disk can move on after a store update. + """ + with self._lock: + self._loaded[plugin_id] = { + 'version': version, + 'loaded_at': time.time() if loaded_at is None else loaded_at, + } + self._note_change() + + def record_unloaded(self, plugin_id: str) -> None: + """Forget the loaded record alone, keeping state and error info: for + an unload that failed after the instance was already dropped.""" + with self._lock: + if self._loaded.pop(plugin_id, None) is not None: + self._note_change() + + def runtime_records(self) -> Dict[str, Dict[str, Any]]: + """Every known plugin's reader-visible state, taken in one critical + section so a concurrent load or unload is seen whole or not at all. + + Per plugin: ``state`` (published_state()'s value), ``loaded``, + ``version`` and ``loaded_at`` (None unless loaded) and ``error_info`` + (a copy, or None). + """ + with self._lock: + records: Dict[str, Dict[str, Any]] = {} + for plugin_id in set(self._states) | set(self._loaded): + loaded = self._loaded.get(plugin_id) + info = self._error_info.get(plugin_id) + records[plugin_id] = { + 'state': published_state( + self._states.get(plugin_id, PluginState.UNLOADED)).value, + 'loaded': loaded is not None, + 'version': loaded['version'] if loaded else None, + 'loaded_at': loaded['loaded_at'] if loaded else None, + 'error_info': dict(info) if info is not None else None, + } + return records + def record_update(self, plugin_id: str) -> None: """Record that plugin update() was called.""" self._last_update[plugin_id] = datetime.now() @@ -221,8 +309,13 @@ class PluginStateManager: state. """ with self._lock: + had = (plugin_id in self._states or plugin_id in self._loaded + or plugin_id in self._error_info) self._states.pop(plugin_id, None) self._state_transition_counts.pop(plugin_id, None) self._error_info.pop(plugin_id, None) self._last_update.pop(plugin_id, None) + self._loaded.pop(plugin_id, None) + if had: + self._note_change() diff --git a/src/plugin_system/state_manager.py b/src/plugin_system/state_manager.py deleted file mode 100644 index 8e8243a8..00000000 --- a/src/plugin_system/state_manager.py +++ /dev/null @@ -1,343 +0,0 @@ -""" -Centralized plugin state management. - -Provides a single source of truth for plugin state (installed, enabled, version, etc.) -with persistence. -""" - -import json -import threading -from typing import Dict, Any, Optional -from pathlib import Path -from datetime import datetime -from dataclasses import dataclass, asdict -from enum import Enum - -from src.config_manager_atomic import atomic_write_text -from src.logging_config import get_logger - - -class PluginStateStatus(Enum): - """Status of a plugin.""" - INSTALLED = "installed" - ENABLED = "enabled" - DISABLED = "disabled" - ERROR = "error" - UNKNOWN = "unknown" - - -@dataclass -class PluginState: - """Represents the state of a plugin.""" - plugin_id: str - status: PluginStateStatus - enabled: bool - version: Optional[str] = None - installed_at: Optional[datetime] = None - last_updated: Optional[datetime] = None - # Bumped on every update_plugin_state(). Nothing reads it; it stays so - # plugin_state.json keeps the shape older releases load with cls(**data). - config_version: int = 1 - metadata: Dict[str, Any] = None - - def __post_init__(self): - if self.metadata is None: - self.metadata = {} - - def to_dict(self) -> Dict[str, Any]: - """Convert state to dictionary for serialization.""" - result = asdict(self) - # Convert enum to string - result['status'] = self.status.value - # Convert datetime to ISO string - if result.get('installed_at'): - result['installed_at'] = self.installed_at.isoformat() - if result.get('last_updated'): - result['last_updated'] = self.last_updated.isoformat() - return result - - @classmethod - def from_dict(cls, data: Dict[str, Any]) -> 'PluginState': - """Create state from dictionary.""" - # Parse enum - if isinstance(data.get('status'), str): - data['status'] = PluginStateStatus(data['status']) - - # Parse datetime - if data.get('installed_at') and isinstance(data['installed_at'], str): - data['installed_at'] = datetime.fromisoformat(data['installed_at']) - if data.get('last_updated') and isinstance(data['last_updated'], str): - data['last_updated'] = datetime.fromisoformat(data['last_updated']) - - return cls(**data) - - -class PluginStateManager: - """ - Centralized plugin state manager. - - Provides: - - Single source of truth for plugin state - - State persistence - """ - - def __init__( - self, - state_file: Optional[str] = None, - auto_save: bool = True, - lazy_load: bool = False - ): - """ - Initialize state manager. - - Args: - state_file: Path to file for persisting state - auto_save: Whether to automatically save state on changes - lazy_load: If True, defer loading state file until first access - """ - self.logger = get_logger(__name__) - self.state_file = Path(state_file) if state_file else None - self.auto_save = auto_save - self._lazy_load = lazy_load - self._state_loaded = False - - # State storage - self._states: Dict[str, PluginState] = {} - # The file's top-level "version", written back as read. Nothing - # checks it yet; it is there for a future format change to branch on. - self._state_version = 1 - - # Threading - self._lock = threading.RLock() - - # Load state from file if it exists (unless lazy loading) - if not self._lazy_load and self.state_file and self.state_file.exists(): - self._load_state() - self._state_loaded = True - - def _ensure_loaded(self) -> None: - """Ensure state is loaded (for lazy loading).""" - if not self._state_loaded and self.state_file and self.state_file.exists(): - self._load_state() - self._state_loaded = True - - def get_plugin_state(self, plugin_id: str) -> Optional[PluginState]: - """ - Get state for a plugin. - - Args: - plugin_id: Plugin identifier - - Returns: - PluginState if found, None otherwise - """ - self._ensure_loaded() - with self._lock: - return self._states.get(plugin_id) - - def get_all_states(self) -> Dict[str, PluginState]: - """ - Get all plugin states. - - Returns: - Dictionary mapping plugin_id to PluginState - """ - self._ensure_loaded() - with self._lock: - return self._states.copy() - - def update_plugin_state( - self, - plugin_id: str, - updates: Dict[str, Any] - ) -> bool: - """ - Update plugin state. - - Args: - plugin_id: Plugin identifier - updates: Dictionary of state updates - - Returns: - True if update successful - """ - self._ensure_loaded() - with self._lock: - # Get current state or create new - current_state = self._states.get(plugin_id) - if not current_state: - current_state = PluginState( - plugin_id=plugin_id, - status=PluginStateStatus.UNKNOWN, - enabled=False - ) - - # Apply updates - if 'status' in updates: - if isinstance(updates['status'], str): - current_state.status = PluginStateStatus(updates['status']) - else: - current_state.status = updates['status'] - - if 'enabled' in updates: - current_state.enabled = bool(updates['enabled']) - - if 'version' in updates: - current_state.version = updates['version'] - - if 'installed_at' in updates: - current_state.installed_at = updates['installed_at'] - - if 'last_updated' in updates: - current_state.last_updated = updates['last_updated'] - else: - current_state.last_updated = datetime.now() - - if 'metadata' in updates: - if current_state.metadata is None: - current_state.metadata = {} - current_state.metadata.update(updates['metadata']) - - current_state.config_version += 1 - - # Store updated state - self._states[plugin_id] = current_state - - # Auto-save if enabled - if self.auto_save: - self._save_state() - - return True - - def set_plugin_enabled(self, plugin_id: str, enabled: bool) -> bool: - """ - Set plugin enabled/disabled state. - - Args: - plugin_id: Plugin identifier - enabled: Whether plugin is enabled - - Returns: - True if update successful - """ - status = PluginStateStatus.ENABLED if enabled else PluginStateStatus.DISABLED - return self.update_plugin_state( - plugin_id, - { - 'enabled': enabled, - 'status': status - } - ) - - def set_plugin_installed( - self, - plugin_id: str, - version: Optional[str] = None, - installed_at: Optional[datetime] = None - ) -> bool: - """ - Mark plugin as installed. - - Args: - plugin_id: Plugin identifier - version: Plugin version - installed_at: Installation timestamp - - Returns: - True if update successful - """ - return self.update_plugin_state( - plugin_id, - { - 'status': PluginStateStatus.INSTALLED, - 'version': version, - 'installed_at': installed_at or datetime.now() - } - ) - - def remove_plugin_state(self, plugin_id: str) -> bool: - """ - Remove plugin state (e.g., after uninstall). - - Args: - plugin_id: Plugin identifier - - Returns: - True if removal successful - """ - self._ensure_loaded() - with self._lock: - if plugin_id in self._states: - del self._states[plugin_id] - - # Auto-save if enabled - if self.auto_save: - self._save_state() - - return True - - return False - - def _save_state(self) -> None: - """Save state to file.""" - if not self.state_file: - return - - try: - # The write stays under the lock and goes through a temp file: - # Flask serves requests on threads, and two saves racing on a - # plain open('w') could interleave or leave a truncated file - # that _load_state then drops wholesale. - with self._lock: - # Convert states to dicts - states_data = { - plugin_id: state.to_dict() - for plugin_id, state in self._states.items() - } - - state_data = { - 'version': self._state_version, - 'states': states_data, - 'last_updated': datetime.now().isoformat() - } - - # Ensure directory exists with proper permissions - from src.common.permission_utils import ( - ensure_directory_permissions, - get_config_dir_mode - ) - ensure_directory_permissions(self.state_file.parent, get_config_dir_mode()) - - # Write to file - atomic_write_text(self.state_file, json.dumps(state_data, indent=2)) - - except Exception as e: - self.logger.error(f"Error saving plugin state: {e}", exc_info=True) - - def _load_state(self) -> None: - """Load state from file.""" - if not self.state_file or not self.state_file.exists(): - return - - try: - with open(self.state_file, 'r', encoding='utf-8') as f: - state_data = json.load(f) - - with self._lock: - # Load state version - self._state_version = state_data.get('version', 1) - - # Load states - states_data = state_data.get('states', {}) - for plugin_id, state_dict in states_data.items(): - try: - self._states[plugin_id] = PluginState.from_dict(state_dict) - except Exception as e: - self.logger.warning( - f"Error loading state for plugin {plugin_id}: {e}" - ) - - self.logger.info(f"Loaded {len(self._states)} plugin states from file") - - except Exception as e: - self.logger.error(f"Error loading plugin state: {e}", exc_info=True) diff --git a/src/plugin_system/state_reconciliation.py b/src/plugin_system/state_reconciliation.py index 6f63c362..1477984e 100644 --- a/src/plugin_system/state_reconciliation.py +++ b/src/plugin_system/state_reconciliation.py @@ -1,22 +1,34 @@ """ State reconciliation system. -Detects and fixes inconsistencies between: -- Config file state -- Plugin manager state -- Disk state (installed plugins) -- State manager state +Compares what the user wants with what is there and what runs: + +- desired: config.json (which plugins are configured, and enabled) plus the + plugins directory on disk (which are installed, at which version); +- observed: the runtime snapshot the display publishes + (src/plugin_system/plugin_runtime.py) -- which plugins it has loaded, at + which version, and why one failed. Only a live snapshot is compared; a + stale, stopped or missing one is unknown and yields no findings. + +Desired-state gaps (on disk but not in config, in config but not on disk) +are fixed here. Observed-state gaps (enabled but not loaded, loaded at an +older version) are reported, never "fixed": the display reconciles its own +loaded set against config, and a version gap needs a display restart. + +There is no third, persisted record any more. ``data/plugin_state.json`` +held a copy of config's enabled flags and the disk's versions, and this +module mostly synced it back to config; it is no longer read or written. """ import json -from typing import Dict, Any, List, Set, cast +from typing import Any, Callable, Dict, List, Optional, Set from dataclasses import dataclass from enum import Enum from pathlib import Path from src.core_config_keys import CORE_CONFIG_KEYS, CORE_SECRETS_KEYS from src.plugin_system.plugin_dirs import PluginDirectoryIndex -from src.plugin_system.state_manager import PluginStateManager +from src.plugin_system.plugin_runtime import PluginRuntimeView, UNKNOWN from src.logging_config import get_logger @@ -160,36 +172,40 @@ def still_unresolved(entries: List[Dict[str, Any]], return live +RuntimeSource = Callable[[], PluginRuntimeView] + + class StateReconciliation: """ State reconciliation system. - - Compares state from multiple sources and detects/fixes inconsistencies. + + Compares desired state (config + disk) with observed state (the + display's runtime snapshot) and fixes what can safely be fixed. """ - + def __init__( self, - state_manager: PluginStateManager, + *, config_manager, - plugin_manager, plugins_dir: Path, - store_manager=None + store_manager=None, + runtime_source: Optional[RuntimeSource] = None, ): """ Initialize reconciliation system. Args: - state_manager: PluginStateManager instance config_manager: ConfigManager instance - plugin_manager: PluginManager instance plugins_dir: Path to plugins directory store_manager: Optional PluginStoreManager for auto-repair + runtime_source: Returns the display's runtime snapshot as a + PluginRuntimeView (plugin_runtime.read_plugin_runtime bound to + a cache manager). None: observed state is unknown. """ - self.state_manager = state_manager self.config_manager = config_manager - self.plugin_manager = plugin_manager self.plugins_dir = Path(plugins_dir) self.store_manager = store_manager + self.runtime_source = runtime_source self.logger = get_logger(__name__) # Plugin IDs that failed auto-repair and should NOT be retried this @@ -230,30 +246,28 @@ class StateReconciliation: manual_fix_required = [] try: - # Get state from all sources + # Desired: config + disk. Observed: the display's snapshot. config_state = self._get_config_state() disk_state = self._get_disk_state() - manager_state = self._get_manager_state() - state_manager_state = self._get_state_manager_state() - - # Find all unique plugin IDs + observed = self._get_observed_state() + + # Plugins the display reports but neither config nor disk knows + # (removed while it still runs them) are not a finding of their + # own: the display unloads them when their section goes. all_plugin_ids: Set[str] = set() all_plugin_ids.update(config_state.keys()) all_plugin_ids.update(disk_state.keys()) - all_plugin_ids.update(manager_state.keys()) - all_plugin_ids.update(state_manager_state.keys()) - + # Check each plugin for inconsistencies for plugin_id in all_plugin_ids: plugin_inconsistencies = self._check_plugin_consistency( plugin_id, config_state, disk_state, - manager_state, - state_manager_state + observed, ) inconsistencies.extend(plugin_inconsistencies) - + # Attempt to fix auto-fixable inconsistencies for inconsistency in inconsistencies: if inconsistency.can_auto_fix and inconsistency.fix_action == FixAction.AUTO_FIX: @@ -309,8 +323,9 @@ class StateReconciliation: for plugin_id in config_plugin_ids(config, ignored): plugin_config = config[plugin_id] state[plugin_id] = { - 'enabled': plugin_config.get('enabled', True), - 'version': plugin_config.get('version'), + # The display's rule: it runs a plugin only when its + # section says "enabled": true. + 'enabled': bool(plugin_config.get('enabled', False)), 'exists_in_config': True } except Exception as e: @@ -339,45 +354,51 @@ class StateReconciliation: self.logger.warning(f"Error reading disk state: {e}") return state - def _get_manager_state(self) -> Dict[str, Dict[str, Any]]: - """Get plugin state from plugin manager.""" - state = {} + def _get_observed_state(self) -> PluginRuntimeView: + """The display's runtime snapshot; unknown when there is no source or + it cannot be read. Only a live view is compared.""" + if self.runtime_source is None: + return PluginRuntimeView(status=UNKNOWN) try: - if self.plugin_manager: - # Get discovered plugins - if hasattr(self.plugin_manager, 'plugin_manifests'): - for plugin_id in self.plugin_manager.plugin_manifests.keys(): - state[plugin_id] = { - 'exists_in_manager': True, - 'loaded': plugin_id in getattr(self.plugin_manager, 'plugins', {}) - } + return self.runtime_source() except Exception as e: - self.logger.warning(f"Error reading manager state: {e}") - return state - - def _get_state_manager_state(self) -> Dict[str, Dict[str, Any]]: - """Get plugin state from state manager.""" - state = {} - try: - all_states = self.state_manager.get_all_states() - for plugin_id, plugin_state in all_states.items(): - state[plugin_id] = { - 'enabled': plugin_state.enabled, - 'status': plugin_state.status.value, - 'version': plugin_state.version, - 'exists_in_state_manager': True - } - except Exception as e: - self.logger.warning(f"Error reading state manager state: {e}") - return state - + self.logger.warning(f"Error reading the display's runtime state: {e}") + return PluginRuntimeView(status=UNKNOWN) + + def plugin_states(self) -> Dict[str, Dict[str, Any]]: + """Desired and observed state for every plugin config or disk knows. + + Per plugin: ``installed`` and ``version`` (disk), ``in_config`` and + ``enabled`` (config, by the display's rule), and the display's + ``loaded`` / ``state`` / ``error_info`` / ``loaded_version`` / + ``loaded_at`` (None unless its snapshot is live). What + /api/v3/plugins/state serves, in place of plugin_state.json. + """ + config_state = self._get_config_state() + disk_state = self._get_disk_state() + observed = self._get_observed_state() + states: Dict[str, Dict[str, Any]] = {} + for plugin_id in sorted(set(config_state) | set(disk_state)): + if plugin_id in CORE_CONFIG_KEYS: + continue + config = config_state.get(plugin_id, {}) + disk = disk_state.get(plugin_id, {}) + states[plugin_id] = { + 'plugin_id': plugin_id, + 'installed': bool(disk.get('exists_on_disk')), + 'version': disk.get('version'), + 'in_config': bool(config.get('exists_in_config')), + 'enabled': bool(config.get('enabled', False)), + **observed.plugin(plugin_id), + } + return states + def _check_plugin_consistency( self, plugin_id: str, config_state: Dict[str, Dict[str, Any]], disk_state: Dict[str, Dict[str, Any]], - manager_state: Dict[str, Dict[str, Any]], - state_manager_state: Dict[str, Dict[str, Any]] + observed: PluginRuntimeView, ) -> List[Inconsistency]: """Check consistency for a single plugin.""" inconsistencies: List[Inconsistency] = [] @@ -397,7 +418,6 @@ class StateReconciliation: config = config_state.get(plugin_id, {}) disk = disk_state.get(plugin_id, {}) - state_mgr = state_manager_state.get(plugin_id, {}) # Check: Plugin exists on disk but not in config if disk.get('exists_on_disk') and not config.get('exists_in_config'): @@ -442,21 +462,47 @@ class StateReconciliation: can_auto_fix=can_repair )) - # Check: Enabled state mismatch - config_enabled = config.get('enabled', False) - state_mgr_enabled = state_mgr.get('enabled') + # Observed checks: only against a live snapshot, and only for a plugin + # that is both configured and installed (the checks above cover the + # rest). Reported, never fixed here: the display loads and unloads by + # config on its own, so a gap is either transient (it is catching up) + # or something only the user can act on (a failed load, a restart). + if (observed.live and config.get('exists_in_config') + and disk.get('exists_on_disk')): + runtime = observed.plugin(plugin_id) + config_enabled = bool(config.get('enabled', False)) + loaded = bool(runtime.get('loaded')) + if config_enabled != loaded: + error = runtime.get('error_info') or {} + why = f" ({error.get('message')})" if error.get('message') else "" + inconsistencies.append(Inconsistency( + plugin_id=plugin_id, + inconsistency_type=InconsistencyType.PLUGIN_ENABLED_MISMATCH, + description=( + f"Plugin {plugin_id} is {'enabled' if config_enabled else 'disabled'} " + f"in config but the display has it " + f"{'loaded' if loaded else 'not loaded'} " + f"(state {runtime.get('state')}){why}"), + fix_action=FixAction.NO_ACTION, + current_state={'loaded': loaded, 'state': runtime.get('state')}, + expected_state={'loaded': config_enabled}, + can_auto_fix=False + )) + loaded_version = runtime.get('loaded_version') + disk_version = disk.get('version') + if loaded and loaded_version and disk_version and loaded_version != disk_version: + inconsistencies.append(Inconsistency( + plugin_id=plugin_id, + inconsistency_type=InconsistencyType.PLUGIN_VERSION_MISMATCH, + description=( + f"Plugin {plugin_id} {disk_version} is installed but the display " + f"is running {loaded_version}; restart the display to run it"), + fix_action=FixAction.NO_ACTION, + current_state={'version': loaded_version}, + expected_state={'version': disk_version}, + can_auto_fix=False + )) - if state_mgr_enabled is not None and config_enabled != state_mgr_enabled: - inconsistencies.append(Inconsistency( - plugin_id=plugin_id, - inconsistency_type=InconsistencyType.PLUGIN_ENABLED_MISMATCH, - description=f"Plugin {plugin_id} enabled state mismatch: config={config_enabled}, state_manager={state_mgr_enabled}", - fix_action=FixAction.AUTO_FIX, - current_state={'enabled': state_mgr_enabled}, - expected_state={'enabled': config_enabled}, - can_auto_fix=True - )) - return inconsistencies def _fix_inconsistency(self, inconsistency: Inconsistency) -> bool: @@ -490,26 +536,6 @@ class StateReconciliation: elif inconsistency.inconsistency_type == InconsistencyType.PLUGIN_MISSING_ON_DISK: return self._auto_repair_missing_plugin(inconsistency.plugin_id) - - elif inconsistency.inconsistency_type == InconsistencyType.PLUGIN_ENABLED_MISMATCH: - # config.json is the user-editable source of truth for enabled state. - # Bring the state manager in sync with config rather than the reverse, - # so that manual config edits (or the state left behind after an - # uninstall+reinstall cycle) don't silently override the user's intent. - # Always set for this type (see _check_plugin_consistency). - config_enabled = cast(bool, inconsistency.expected_state.get('enabled')) - success = self.state_manager.set_plugin_enabled(inconsistency.plugin_id, config_enabled) - if success: - self.logger.info( - f"Fixed: Synced state manager enabled={config_enabled} for " - f"{inconsistency.plugin_id} to match config" - ) - else: - self.logger.warning( - f"Failed to sync state manager enabled={config_enabled} for " - f"{inconsistency.plugin_id}" - ) - return success except Exception as e: self.logger.error(f"Error fixing inconsistency: {e}", exc_info=True) diff --git a/test/_api_v3_test_helpers.py b/test/_api_v3_test_helpers.py index 02c1c563..4e1439d4 100644 --- a/test/_api_v3_test_helpers.py +++ b/test/_api_v3_test_helpers.py @@ -22,7 +22,7 @@ from flask import Flask # whatever a previously-run test left on the singleton. API_V3_MANAGER_ATTRS = ( 'config_manager', 'plugin_catalog', 'plugin_store_manager', - 'plugin_state_manager', 'saved_repositories_manager', 'schema_manager', + 'saved_repositories_manager', 'schema_manager', 'operation_queue', 'operation_history', 'cache_manager', 'health_tracker', 'resource_monitor', ) diff --git a/test/test_api_v3_plugin_install_endpoints.py b/test/test_api_v3_plugin_install_endpoints.py index 07e384d7..a956e3c2 100644 --- a/test/test_api_v3_plugin_install_endpoints.py +++ b/test/test_api_v3_plugin_install_endpoints.py @@ -50,7 +50,6 @@ def side_effects(module): return { "schema_invalidated": api.schema_manager.invalidate_cache.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, } @@ -87,7 +86,6 @@ class TestInstallDirectPath: effects = side_effects(api_v3_module) assert effects["schema_invalidated"] == [(("clock",), {})] assert effects["discovered"] == 1 - assert effects["state_set"] == [(("clock",), {})] assert effects["history"][0].kwargs["status"] == "success" def test_branch_forwarded_to_the_manager(self, api_v3_client, api_v3_module): @@ -134,7 +132,6 @@ class TestInstallDirectPath: effects = side_effects(api_v3_module) assert effects["schema_invalidated"] == [] assert effects["discovered"] == 0 - assert effects["state_set"] == [] class TestInstallQueuedPath: @@ -157,7 +154,6 @@ class TestInstallQueuedPath: effects = side_effects(api_v3_module) assert effects["schema_invalidated"] == [(("clock",), {})] assert effects["discovered"] == 1 - assert effects["state_set"] == [(("clock",), {})] assert effects["history"][0].kwargs["status"] == "success" def test_callback_reports_success(self, api_v3_client, api_v3_module, queued): @@ -199,7 +195,6 @@ 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_catalog, - api_v3_module.api_v3.plugin_state_manager, api_v3_module.api_v3.operation_history): mock.reset_mock() queue = MagicMock() @@ -210,7 +205,6 @@ class TestInstallPathsAgree: assert direct["schema_invalidated"] == queued["schema_invalidated"] assert direct["discovered"] == queued["discovered"] - assert direct["state_set"] == queued["state_set"] assert (direct["history"][0].kwargs["status"] == queued["history"][0].kwargs["status"]) assert (direct["history"][0].kwargs["details"] diff --git a/test/test_backup_manager.py b/test/test_backup_manager.py index e468990e..34d7695a 100644 --- a/test/test_backup_manager.py +++ b/test/test_backup_manager.py @@ -72,7 +72,8 @@ def _make_project(root: Path) -> Path: encoding="utf-8", ) - # plugin_state.json + # A plugin_state.json left behind by an older release. Retired: the + # listing must ignore it (see test_list_installed_plugins). (root / "data").mkdir() (root / "data" / "plugin_state.json").write_text( json.dumps( @@ -130,12 +131,82 @@ def test_bundled_fonts_matches_repo() -> None: def test_list_installed_plugins(project: Path) -> None: + """Installed = a manifest on disk; enabled = config.json. The retired + plugin_state.json is not read: its "other-plugin" is not installed and + not configured, so a restore must not install it.""" plugins = list_installed_plugins(project) - ids = [p["plugin_id"] for p in plugins] - assert "my-plugin" in ids - assert "other-plugin" in ids - my = next(p for p in plugins if p["plugin_id"] == "my-plugin") - assert my["version"] == "1.2.3" + assert plugins == [{"plugin_id": "my-plugin", "version": "1.2.3", "enabled": True}] + + +def test_list_installed_plugins_reads_enabled_from_config(project: Path) -> None: + """The display's rule: only "enabled": true is enabled; a plugin with no + config section, or no flag, is disabled.""" + for pid in ("quiet-plugin", "unconfigured-plugin"): + d = project / "plugin-repos" / pid + d.mkdir() + (d / "manifest.json").write_text(json.dumps({"id": pid, "version": "2.0.0"}), + encoding="utf-8") + config_path = project / "config" / "config.json" + config = json.loads(config_path.read_text(encoding="utf-8")) + config["quiet-plugin"] = {"favorites": []} + config_path.write_text(json.dumps(config), encoding="utf-8") + + by_id = {p["plugin_id"]: p for p in list_installed_plugins(project)} + + assert by_id["my-plugin"]["enabled"] is True + assert by_id["quiet-plugin"]["enabled"] is False + assert by_id["unconfigured-plugin"]["enabled"] is False + assert by_id["quiet-plugin"]["version"] == "2.0.0" + + +def test_list_installed_plugins_without_a_state_file(project: Path) -> None: + """Nothing depends on plugin_state.json being there.""" + (project / "data" / "plugin_state.json").unlink() + assert [p["plugin_id"] for p in list_installed_plugins(project)] == ["my-plugin"] + + +def test_backup_restore_round_trip_ignores_the_retired_state_file( + project: Path, empty_project: Path, tmp_path: Path) -> None: + """A backup made on a device that still has plugin_state.json restores + the installed plugins and their enabled state (from config.json), and + carries no state file of its own.""" + zip_path = create_backup(project, output_dir=tmp_path / "exports") + with zipfile.ZipFile(zip_path) as zf: + names = set(zf.namelist()) + listed = json.loads(zf.read("plugins.json")) + assert not any("plugin_state" in n for n in names) + assert listed == [{"plugin_id": "my-plugin", "version": "1.2.3", "enabled": True}] + + result = restore_backup(zip_path, empty_project, RestoreOptions()) + + assert result.success, result.errors + assert result.plugins_to_install == [{"plugin_id": "my-plugin", "version": "1.2.3"}] + restored = json.loads((empty_project / "config" / "config.json").read_text()) + assert restored["my-plugin"]["enabled"] is True + assert not (empty_project / "data" / "plugin_state.json").exists() + + +def test_restore_of_a_backup_listing_a_state_file_only_plugin( + project: Path, empty_project: Path, tmp_path: Path) -> None: + """A backup written by an older release could list a plugin known only + to plugin_state.json. Restore reads plugins.json as written, so such a + backup still restores everything it lists.""" + zip_path = tmp_path / "old.zip" + with zipfile.ZipFile(zip_path, "w") as zf: + zf.writestr("manifest.json", json.dumps({ + "schema_version": 1, "created_at": "2026-01-01T00:00:00Z", + "ledmatrix_version": "3.6.0", "hostname": "old", + "contents": ["config", "plugins"]})) + zf.writestr("config/config.json", json.dumps({"my-plugin": {"enabled": True}})) + zf.writestr("plugins.json", json.dumps([ + {"plugin_id": "my-plugin", "version": "1.2.3", "enabled": True}, + {"plugin_id": "other-plugin", "version": "0.1.0", "enabled": False}, + ])) + + result = restore_backup(zip_path, empty_project, RestoreOptions()) + + assert result.success, result.errors + assert {p["plugin_id"] for p in result.plugins_to_install} == {"my-plugin", "other-plugin"} def test_preview_backup_contents(project: Path) -> None: diff --git a/test/test_core_config_keys.py b/test/test_core_config_keys.py index e6436184..2d01031a 100644 --- a/test/test_core_config_keys.py +++ b/test/test_core_config_keys.py @@ -97,14 +97,8 @@ def _reconcile(tmp_path, config, installed=(), secrets=None): _install(plugins_dir, pid) secrets_path = tmp_path / "config_secrets.json" secrets_path.write_text(json.dumps(secrets or {}), encoding="utf-8") - state_manager = Mock() - state_manager.get_all_states.return_value = {} - plugin_manager = Mock() - plugin_manager.plugin_manifests = {} reconciler = StateReconciliation( - state_manager=state_manager, config_manager=_ConfigManager(config, str(secrets_path)), - plugin_manager=plugin_manager, plugins_dir=plugins_dir, ) return reconciler, reconciler.reconcile_state() diff --git a/test/test_plugin_runtime_snapshot.py b/test/test_plugin_runtime_snapshot.py new file mode 100644 index 00000000..61edcb83 --- /dev/null +++ b/test/test_plugin_runtime_snapshot.py @@ -0,0 +1,442 @@ +"""The display publishes its plugin runtime state; the web reads it back. + +Only the display runs plugins, so only it knows which ones are loaded, where +each is in its lifecycle, why one failed and which version it runs. It +publishes that as one snapshot in the shared cache (plugin_runtime), and the +web interface fills /api/v3/plugins/installed's ``loaded`` / ``state`` / +``error_info`` from it -- but only while the snapshot is live. A stale, +stopped or missing snapshot is reported as such, with those fields null. + +The cross-process tests use two CacheManagers over one temporary directory, +the arrangement of the real services (which share /var/cache/ledmatrix). +""" +import json +import sys +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent)) + +from src.cache_manager import CacheManager # noqa: E402 +from src.plugin_system import plugin_runtime as rt # noqa: E402 +from src.plugin_system.plugin_manager import PluginManager # noqa: E402 +from src.plugin_system.plugin_runtime import ( # noqa: E402 + PLUGIN_RUNTIME_KEY, PluginRuntimePublisher, read_plugin_runtime, + view_from_snapshot, +) +from src.plugin_system.plugin_state import PluginState, PluginStateManager # noqa: E402 +from test._api_v3_test_helpers import api_v3_client, api_v3_module # noqa: F401,E402 + + +class FakeClock: + def __init__(self, now=1000.0): + self.now = now + + def __call__(self): + return self.now + + +@pytest.fixture +def shared_cache(tmp_path, monkeypatch): + """Two cache managers over one directory: the display's and the web's.""" + monkeypatch.setattr(CacheManager, "_get_writable_cache_dir", lambda self: str(tmp_path)) + display_cache, web_cache = CacheManager(), CacheManager() + yield display_cache, web_cache + display_cache.stop_cleanup_thread() + web_cache.stop_cleanup_thread() + + +def _publisher(cache, states, wall=None, mono=None): + return PluginRuntimePublisher(cache, states, clock=mono or FakeClock(), + wall_clock=wall or FakeClock(1_800_000_000.0)) + + +# --- The state machine: what counts as a change ----------------------------- + +class TestChangeCount: + def test_an_ordinary_update_is_not_a_change(self): + """ENABLED -> RUNNING -> ENABLED is every update() call. Publishing it + would be an SD-card write per plugin update.""" + states = PluginStateManager() + states.set_state("clock", PluginState.ENABLED) + before = states.change_count + for _ in range(50): + states.set_state("clock", PluginState.RUNNING) + states.set_state("clock", PluginState.ENABLED) + assert states.change_count == before + assert states.runtime_records()["clock"]["state"] == "enabled" + + def test_running_is_published_as_enabled(self): + states = PluginStateManager() + states.set_state("clock", PluginState.RUNNING) + assert states.runtime_records()["clock"]["state"] == "enabled" + + @pytest.mark.parametrize("change", [ + lambda s: s.set_state("clock", PluginState.ERROR, error=ValueError("x")), + lambda s: s.set_state_with_error("clock", PluginState.ENABLED, {"error": "x"}), + lambda s: s.record_loaded("clock", "1.0.0"), + lambda s: s.set_state("clock", PluginState.DISABLED), + lambda s: s.clear_state("clock"), + lambda s: s.set_state("weather", PluginState.LOADED), + ]) + def test_reader_visible_changes_move_it(self, change): + states = PluginStateManager() + states.set_state("clock", PluginState.ENABLED) + before = states.change_count + change(states) + assert states.change_count > before + + def test_recovering_from_an_error_is_a_change(self): + states = PluginStateManager() + states.set_state_with_error("clock", PluginState.ENABLED, {"error": "timeout"}) + before = states.change_count + states.set_state("clock", PluginState.ENABLED) # next update succeeded + assert states.change_count > before + assert states.runtime_records()["clock"]["error_info"] is None + + def test_clear_state_forgets_the_loaded_record(self): + states = PluginStateManager() + states.set_state("clock", PluginState.ENABLED) + states.record_loaded("clock", "1.0.0", loaded_at=5.0) + assert states.runtime_records()["clock"]["loaded"] is True + states.clear_state("clock") + assert states.runtime_records() == {} + + +class TestPluginManagerRecordsWhatItLoaded: + @pytest.fixture + def pm(self, tmp_path): + plugins_dir = tmp_path / "plugins" + (plugins_dir / "demo").mkdir(parents=True) + manager = PluginManager(plugins_dir=str(plugins_dir)) + manager.plugin_manifests["demo"] = {"id": "demo", "name": "Demo", "version": "2.1.0"} + manager.schema_manager = MagicMock() + manager.schema_manager.get_schema_path.return_value = None + manager.plugin_loader = MagicMock() + manager.plugin_loader.find_plugin_directory.return_value = plugins_dir / "demo" + manager.plugin_loader.load_plugin.return_value = (MagicMock(spec=["on_enable"]), None) + return manager + + def test_load_records_the_manifest_version(self, pm): + assert pm.load_plugin("demo") is True + record = pm.state_manager.runtime_records()["demo"] + assert record["loaded"] is True + assert record["state"] == "enabled" + assert record["version"] == "2.1.0" + assert isinstance(record["loaded_at"], float) + + def test_unload_forgets_it(self, pm): + pm.load_plugin("demo") + pm.unload_plugin("demo") + assert "demo" not in pm.state_manager.runtime_records() + + def test_a_failed_load_is_an_error_and_not_loaded(self, pm): + pm.plugin_loader.load_plugin.side_effect = ImportError("No module named 'requests'") + assert pm.load_plugin("demo") is False + record = pm.state_manager.runtime_records()["demo"] + assert record["loaded"] is False + assert record["state"] == "error" + assert "requests" in record["error_info"]["error"] + + +# --- Publishing ------------------------------------------------------------- + +class TestPublisher: + def test_first_tick_publishes(self): + cache = MagicMock() + states = PluginStateManager() + states.set_state("clock", PluginState.ENABLED) + states.record_loaded("clock", "1.0.0", loaded_at=10.0) + + assert _publisher(cache, states).tick() is True + + key, snapshot = cache.set.call_args.args + assert key == PLUGIN_RUNTIME_KEY + assert snapshot["running"] is True + assert snapshot["published_at"] == 1_800_000_000.0 + assert snapshot["stale_after"] == rt.STALE_AFTER + assert snapshot["plugins"] == {"clock": { + "loaded": True, "state": "enabled", "error": None, + "version": "1.0.0", "loaded_at": 10.0}} + + def test_changes_are_throttled_and_quiet_displays_refresh(self): + cache = MagicMock() + states = PluginStateManager() + mono = FakeClock() + publisher = _publisher(cache, states, mono=mono) + publisher.tick() + assert cache.set.call_count == 1 + + # Nothing changed: no write until the refresh is due. + mono.now += rt.REFRESH_INTERVAL - 1 + assert publisher.tick() is False + mono.now += 1 + assert publisher.tick() is True + assert cache.set.call_count == 2 + + # A burst of changes is written at most once per MIN_INTERVAL. + states.set_state("a", PluginState.LOADED) + mono.now += 1 + assert publisher.tick() is False + mono.now += rt.MIN_INTERVAL + assert publisher.tick() is True + assert cache.set.call_count == 3 + + def test_updates_alone_cause_no_writes(self): + """A display running plugins but changing nothing writes once a + minute, however many update() calls it makes.""" + cache = MagicMock() + states = PluginStateManager() + states.set_state("clock", PluginState.ENABLED) + mono = FakeClock() + publisher = _publisher(cache, states, mono=mono) + publisher.tick() + for _ in range(11): # 55 s of 5 s ticks + states.set_state("clock", PluginState.RUNNING) + states.set_state("clock", PluginState.ENABLED) + mono.now += rt.TICK_INTERVAL + publisher.tick() + assert cache.set.call_count == 1 + + def test_errors_are_redacted_and_short(self): + cache = MagicMock() + states = PluginStateManager() + secret = "https://api.example.com/v1?api_key=SUPERSECRET123&q=" + "x" * 500 + states.set_state("weather", PluginState.ERROR, error=ConnectionError(secret)) + + _publisher(cache, states).tick() + + error = cache.set.call_args.args[1]["plugins"]["weather"]["error"] + assert "SUPERSECRET123" not in json.dumps(error) + assert len(error["message"]) <= 200 + assert error["type"] == "ConnectionError" + assert isinstance(error["at"], float) + assert error["recoverable"] is False + + def test_stop_publishes_stopped(self): + cache = MagicMock() + states = PluginStateManager() + states.record_loaded("clock", "1.0.0") + publisher = _publisher(cache, states) + publisher.tick() + + publisher.stop() + + snapshot = cache.set.call_args.args[1] + assert snapshot["running"] is False + assert snapshot["plugins"] == {} + + def test_a_failing_cache_never_raises(self): + cache = MagicMock() + cache.set.side_effect = OSError("read-only file system") + publisher = _publisher(cache, PluginStateManager()) + assert publisher.tick() is False + publisher.stop() # also swallowed + + def test_the_thread_starts_and_stops(self): + cache = MagicMock() + publisher = rt.start_plugin_runtime_publisher(cache, PluginStateManager()) + try: + assert publisher is not None + finally: + publisher.stop() + assert cache.set.call_args.args[1]["running"] is False + + +# --- Reading: live, stale, stopped, unknown --------------------------------- + +def _snapshot(published_at, running=True, plugins=None, **extra): + base = {"schema": rt.SNAPSHOT_SCHEMA, "running": running, + "published_at": published_at, "stale_after": 180.0, + "plugins": plugins or {}} + base.update(extra) + return base + + +class TestReader: + NOW = 1_800_000_000.0 + CLOCK = {"loaded": True, "state": "enabled", "error": None, + "version": "1.0.0", "loaded_at": 5.0} + + def test_live(self): + view = view_from_snapshot(_snapshot(self.NOW - 30, plugins={"clock": self.CLOCK}), + now=self.NOW) + assert view.status == "live" + assert view.plugin("clock") == {"loaded": True, "state": "enabled", + "error_info": None, "loaded_version": "1.0.0", + "loaded_at": 5.0} + # Listed nowhere in a live snapshot: not loaded. + assert view.plugin("weather")["loaded"] is False + assert view.plugin("weather")["state"] == "unloaded" + assert view.describe()["age_seconds"] == 30.0 + + def test_stale_reports_nothing(self): + view = view_from_snapshot(_snapshot(self.NOW - 181, plugins={"clock": self.CLOCK}), + now=self.NOW) + assert view.status == "stale" + assert view.plugin("clock") == {"loaded": None, "state": None, "error_info": None, + "loaded_version": None, "loaded_at": None} + assert view.describe()["status"] == "stale" + + def test_stopped_reports_nothing(self): + view = view_from_snapshot(_snapshot(self.NOW - 1, running=False), now=self.NOW) + assert view.status == "stopped" + assert view.plugin("clock")["loaded"] is None + + @pytest.mark.parametrize("snapshot", [ + None, "junk", [], {}, {"schema": 99, "running": True, "published_at": NOW}, + _snapshot(None), _snapshot("yesterday"), _snapshot(float("nan")), + ]) + def test_unknown(self, snapshot): + view = view_from_snapshot(snapshot, now=self.NOW) + assert view.status == "unknown" + assert view.plugin("clock")["state"] is None + + def test_a_little_in_the_future_is_live_far_is_stale(self): + """The Pi has no RTC: its clock steps at NTP sync.""" + assert view_from_snapshot(_snapshot(self.NOW + 60), now=self.NOW).status == "live" + assert view_from_snapshot(_snapshot(self.NOW + 3600), now=self.NOW).status == "stale" + + def test_a_corrupt_stale_after_is_bounded(self): + forever = _snapshot(self.NOW - 7200, stale_after=1e12) + assert view_from_snapshot(forever, now=self.NOW).status == "stale" + eager = _snapshot(self.NOW - 20, stale_after=0) + assert view_from_snapshot(eager, now=self.NOW).status == "live" + + def test_no_cache_manager_or_a_failing_one_is_unknown(self): + assert read_plugin_runtime(None).status == "unknown" + cache = MagicMock() + cache.get.side_effect = OSError("gone") + assert read_plugin_runtime(cache).status == "unknown" + + +# --- Across processes, through the real cache and the real routes ----------- + +class TestAcrossProcesses: + def test_what_the_display_publishes_the_web_reads(self, shared_cache): + display_cache, web_cache = shared_cache + states = PluginStateManager() + states.set_state("clock", PluginState.ENABLED) + states.record_loaded("clock", "1.0.0") + publisher = PluginRuntimePublisher(display_cache, states) + + assert read_plugin_runtime(web_cache).status == "unknown" + publisher.tick() + view = read_plugin_runtime(web_cache) + assert view.status == "live" + assert view.plugin("clock")["loaded"] is True + + # memory_ttl=0 on the reading side: a later write is seen at once. + publisher.stop() + assert read_plugin_runtime(web_cache).status == "stopped" + + def test_a_snapshot_left_by_a_dead_display_goes_stale(self, shared_cache): + display_cache, web_cache = shared_cache + states = PluginStateManager() + states.record_loaded("clock", "1.0.0") + old = FakeClock(1_000_000.0) # long ago + PluginRuntimePublisher(display_cache, states, wall_clock=old).tick() + assert read_plugin_runtime(web_cache).status == "stale" + + +@pytest.fixture +def web_listing(api_v3_module, api_v3_client, shared_cache, tmp_path): # noqa: F811 + """GET /api/v3/plugins/installed over one real plugin and the shared cache.""" + display_cache, web_cache = shared_cache + api = api_v3_module.api_v3 + api.cache_manager = web_cache + api.plugin_catalog.plugins_dir = str(tmp_path / "plugins") + api.plugin_catalog.get_all_plugin_info = MagicMock(return_value=[ + {"id": "clock", "name": "Clock", "version": "1.1.0"}, + {"id": "weather", "name": "Weather", "version": "3.0.0"}, + ]) + 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={ + "clock": {"enabled": True}, "weather": {"enabled": True}}) + + def get(): + response = api_v3_client.get("/api/v3/plugins/installed") + assert response.status_code == 200, response.get_data(as_text=True) + data = response.get_json()["data"] + return {p["id"]: p for p in data["plugins"]}, data["runtime"] + + return display_cache, get + + +class TestInstalledPluginsRoute: + def _display(self, display_cache): + states = PluginStateManager() + states.set_state("clock", PluginState.ENABLED) + states.record_loaded("clock", "1.0.0", loaded_at=123.0) + states.set_state("weather", PluginState.ERROR, + error=RuntimeError("token=abc123 rejected")) + return PluginRuntimePublisher(display_cache, states) + + def test_restores_loaded_state_and_error_info(self, web_listing): + display_cache, get = web_listing + self._display(display_cache).tick() + + plugins, runtime = get() + + assert runtime["status"] == "live" + clock, weather = plugins["clock"], plugins["weather"] + assert (clock["loaded"], clock["state"], clock["error_info"]) == (True, "enabled", None) + # The display runs 1.0.0; 1.1.0 is on disk (an update awaiting restart). + assert clock["version"] == "1.1.0" and clock["loaded_version"] == "1.0.0" + assert clock["loaded_at"] == 123.0 + assert weather["loaded"] is False and weather["state"] == "error" + assert weather["error_info"]["type"] == "RuntimeError" + assert "abc123" not in weather["error_info"]["message"] + + def test_unknown_before_the_display_has_published(self, web_listing): + _, get = web_listing + plugins, runtime = get() + assert runtime["status"] == "unknown" + assert all(p["loaded"] is None and p["state"] is None and p["error_info"] is None + for p in plugins.values()) + # enabled still comes from config.json. + assert plugins["clock"]["enabled"] is True + + def test_stopped_display_is_not_reported_as_running(self, web_listing): + display_cache, get = web_listing + publisher = self._display(display_cache) + publisher.tick() + publisher.stop() + + plugins, runtime = get() + + assert runtime["status"] == "stopped" + assert plugins["clock"]["loaded"] is None + + def test_stale_snapshot_is_not_reported_as_truth(self, web_listing): + display_cache, get = web_listing + states = PluginStateManager() + states.record_loaded("clock", "1.0.0") + PluginRuntimePublisher(display_cache, states, + wall_clock=FakeClock(1_000_000.0)).tick() + + plugins, runtime = get() + + assert runtime["status"] == "stale" + assert runtime["age_seconds"] > rt.STALE_AFTER + assert plugins["clock"]["loaded"] is None + + +class TestDisplayControllerStopsThePublisher: + def test_cleanup_publishes_stopped(self, monkeypatch): + # display_manager binds the hardware rgbmatrix module unless + # EMULATOR=true is set before import (test_initial_update_budget.py). + monkeypatch.setenv("EMULATOR", "true") + from src.display_controller import DisplayController + controller = DisplayController.__new__(DisplayController) + publisher = MagicMock() + controller._plugin_runtime_publisher = publisher + controller.plugin_manager = None + controller.vegas_coordinator = None + controller.sync_manager = None + controller.display_manager = MagicMock() + controller.cleanup() + publisher.stop.assert_called_once_with() diff --git a/test/test_plugin_state_files_atomic.py b/test/test_plugin_state_files_atomic.py index 1405da36..3b94b1b7 100644 --- a/test/test_plugin_state_files_atomic.py +++ b/test/test_plugin_state_files_atomic.py @@ -1,48 +1,16 @@ -"""plugin_state.json and the operation history file are replaced atomically. +"""The operation history file is replaced atomically. -Both were written with a plain ``open(path, 'w')`` + ``json.dump`` outside -their lock. The open truncates first, so a value json can't encode (or a -crash, or a second Flask thread saving at the same moment) left a partial -file, and the next load dropped every saved state. They now serialise first -and go through a temp file + rename while holding the lock. +It was written with a plain ``open(path, 'w')`` + ``json.dump`` outside its +lock. The open truncates first, so a value json can't encode (or a crash, or +a second Flask thread saving at the same moment) left a partial file, and the +next load dropped every saved record. It now serialises first and goes +through a temp file + rename while holding the lock. (plugin_state.json had +the same fix; it is retired now -- nothing writes it.) """ import json -import threading from src.plugin_system.operation_history import OperationHistory -from src.plugin_system.state_manager import PluginStateManager - - -def test_state_file_survives_a_failed_save(tmp_path): - state_file = tmp_path / "plugin_state.json" - mgr = PluginStateManager(state_file=str(state_file)) - mgr.set_plugin_enabled("clock", True) - before = json.loads(state_file.read_text()) - - # Not JSON-serialisable: the save fails (and is logged, not raised). - mgr.update_plugin_state("clock", {"metadata": {"bad": object()}}) - - assert json.loads(state_file.read_text()) == before - assert [p.name for p in tmp_path.iterdir()] == ["plugin_state.json"] - - -def test_state_file_is_valid_after_concurrent_saves(tmp_path): - state_file = tmp_path / "plugin_state.json" - mgr = PluginStateManager(state_file=str(state_file)) - - def worker(n): - for i in range(15): - mgr.set_plugin_enabled(f"plugin-{n}", i % 2 == 0) - - threads = [threading.Thread(target=worker, args=(n,)) for n in range(6)] - for t in threads: - t.start() - for t in threads: - t.join() - - data = json.loads(state_file.read_text()) - assert set(data["states"]) == {f"plugin-{n}" for n in range(6)} def test_history_file_survives_a_failed_save(tmp_path): diff --git a/test/test_plugin_system_utf8_reads.py b/test/test_plugin_system_utf8_reads.py index 88094728..6aa7003d 100644 --- a/test/test_plugin_system_utf8_reads.py +++ b/test/test_plugin_system_utf8_reads.py @@ -1,8 +1,8 @@ """Plugin-system JSON files are read as UTF-8, whatever the locale says. -store_manager (install_from_url's manifest, the secrets file) and -state_manager (plugin_state.json) opened text files without an encoding, so -the platform default applied. A manifest written in UTF-8 with a non-ASCII +store_manager (install_from_url's manifest, the secrets file) opened text +files without an encoding, so the platform default applied. (So did the +retired plugin_state.json reader.) A manifest written in UTF-8 with a non-ASCII name then read as mojibake -- or raised UnicodeDecodeError -- on a host whose locale encoding is not UTF-8 (Windows' cp1252; a Pi with LANG=C). On a UTF-8 host these pass either way; they fail on old code where the @@ -13,7 +13,6 @@ import json import pytest -from src.plugin_system.state_manager import PluginStateManager from src.plugin_system.store_manager import PluginStoreManager NAME = "Météo Á" # "Á" is C3 81 in UTF-8; 0x81 is undefined in cp1252 @@ -48,15 +47,3 @@ def test_install_from_url_reads_a_utf8_manifest(store): written = json.loads( (store.plugins_dir / "meteo" / "manifest.json").read_bytes().decode("utf-8")) assert written["name"] == NAME - - -def test_state_manager_loads_a_utf8_state_file(tmp_path): - state_file = tmp_path / "plugin_state.json" - state_file.write_bytes(json.dumps({ - "version": 1, - "states": {"meteo": {"plugin_id": "meteo", "status": "installed", - "enabled": True, "metadata": {"label": NAME}}}, - }, ensure_ascii=False).encode("utf-8")) - - mgr = PluginStateManager(state_file=str(state_file)) - assert mgr.get_plugin_state("meteo").metadata["label"] == NAME diff --git a/test/test_reconciliation_phantom_plugins.py b/test/test_reconciliation_phantom_plugins.py index 7d9d8f6d..3b86f499 100644 --- a/test/test_reconciliation_phantom_plugins.py +++ b/test/test_reconciliation_phantom_plugins.py @@ -54,9 +54,7 @@ class _ConfigManager: def _reconciler(config, plugins_dir, secrets_path=None): return StateReconciliation( - state_manager=Mock(), config_manager=_ConfigManager(config, secrets_path), - plugin_manager=Mock(), plugins_dir=plugins_dir, ) @@ -92,9 +90,8 @@ class TestSecretsKeysAreNotPlugins: assert "odds-ticker" in r._get_config_state() def test_config_manager_without_secrets_path_is_survivable(self, tmp_path): - r = StateReconciliation(state_manager=Mock(), - config_manager=Mock(spec=["load_config", "save_config"]), - plugin_manager=Mock(), plugins_dir=tmp_path) + r = StateReconciliation(config_manager=Mock(spec=["load_config", "save_config"]), + plugins_dir=tmp_path) r.config_manager.load_config.return_value = {"odds-ticker": {"enabled": True}} assert "odds-ticker" in r._get_config_state() diff --git a/test/test_uninstall_and_reconcile_endpoint.py b/test/test_uninstall_and_reconcile_endpoint.py index 99534acd..20156841 100644 --- a/test/test_uninstall_and_reconcile_endpoint.py +++ b/test/test_uninstall_and_reconcile_endpoint.py @@ -33,7 +33,7 @@ from test._api_v3_test_helpers import mock_plugin_catalog _API_V3_MOCKED_ATTRS = ( 'config_manager', 'plugin_catalog', 'plugin_store_manager', - 'plugin_state_manager', 'saved_repositories_manager', 'schema_manager', + 'saved_repositories_manager', 'schema_manager', 'operation_queue', 'operation_history', 'cache_manager', 'health_tracker', 'resource_monitor', ) @@ -64,8 +64,6 @@ def _make_client(): 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 = {} api_v3.saved_repositories_manager = MagicMock() api_v3.schema_manager = MagicMock() api_v3.operation_queue = None # force the direct (non-queue) path diff --git a/test/test_web_api.py b/test/test_web_api.py index eb94d25d..df881bd4 100644 --- a/test/test_web_api.py +++ b/test/test_web_api.py @@ -68,7 +68,6 @@ def client(mock_config_manager, mock_plugin_catalog): api_v3.saved_repositories_manager = MagicMock() api_v3.schema_manager = MagicMock() api_v3.operation_queue = MagicMock() - 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). @@ -88,9 +87,6 @@ def client(mock_config_manager, mock_plugin_catalog): 'properties': {'enabled': {'type': 'boolean'}} } - # Setup state manager mocks - api_v3.plugin_state_manager.get_all_states.return_value = {} - test_app.register_blueprint(api_v3, url_prefix='/api/v3') with test_app.test_client() as client: @@ -664,7 +660,6 @@ class TestPluginsAPI: from web_interface.blueprints.api_v3 import api_v3 api_v3.config_manager = mock_config_manager api_v3.plugin_catalog = mock_plugin_catalog - api_v3.plugin_state_manager = MagicMock() api_v3.operation_history = MagicMock() # Setup plugin manifests diff --git a/test/test_web_plugin_dir_resolution.py b/test/test_web_plugin_dir_resolution.py index aea37af3..27b2188e 100644 --- a/test/test_web_plugin_dir_resolution.py +++ b/test/test_web_plugin_dir_resolution.py @@ -87,7 +87,6 @@ class TestUpdateRoute: api.plugin_store_manager.update_plugin = MagicMock(return_value=True) api.plugin_store_manager._get_local_git_info = MagicMock(return_value=None) api.schema_manager = None - api.plugin_state_manager = None api.operation_history = None return client.post("/api/v3/plugins/update", json={"plugin_id": "demo"}) diff --git a/test/web_interface/integration/test_plugin_operations.py b/test/web_interface/integration/test_plugin_operations.py index 128f14ed..dc9d08e5 100644 --- a/test/web_interface/integration/test_plugin_operations.py +++ b/test/web_interface/integration/test_plugin_operations.py @@ -9,7 +9,6 @@ from pathlib import Path from src.plugin_system.operation_queue import PluginOperationQueue from src.plugin_system.operation_types import OperationType -from src.plugin_system.state_manager import PluginStateManager from src.plugin_system.operation_history import OperationHistory @@ -22,12 +21,10 @@ class TestPluginOperationsIntegration(unittest.TestCase): # Initialize components self.operation_queue = PluginOperationQueue(max_history=100) - - self.state_manager = PluginStateManager( - state_file=str(self.temp_dir / "state.json"), - auto_save=True - ) - + + # No state manager: installed / enabled / version are read from disk + # and config.json, not from a plugin_state.json record. + self.operation_history = OperationHistory( history_file=str(self.temp_dir / "history.json"), max_records=100 @@ -64,22 +61,15 @@ class TestPluginOperationsIntegration(unittest.TestCase): operation_id=operation_id ) self.assertIsNotNone(history_id) - - # Update state manager - self.state_manager.set_plugin_installed(plugin_id, "1.0.0") - - # Verify state - state = self.state_manager.get_plugin_state(plugin_id) - self.assertIsNotNone(state) - self.assertEqual(state.version, "1.0.0") - + + # Verify history + history = self.operation_history.get_history(plugin_id=plugin_id) + self.assertEqual([r.operation_type for r in history], ["install"]) + def test_update_operation_flow(self): """Test complete update operation flow.""" plugin_id = "test-plugin" - # First, mark as installed - self.state_manager.set_plugin_installed(plugin_id, "1.0.0") - # Enqueue update operation operation_id = self.operation_queue.enqueue_operation( OperationType.UPDATE, @@ -97,20 +87,14 @@ class TestPluginOperationsIntegration(unittest.TestCase): operation_id=operation_id ) - # Update state - self.state_manager.update_plugin_state(plugin_id, {"version": "2.0.0"}) - - # Verify state - state = self.state_manager.get_plugin_state(plugin_id) - self.assertEqual(state.version, "2.0.0") - + # Verify history + history = self.operation_history.get_history(plugin_id=plugin_id) + self.assertEqual([r.operation_type for r in history], ["update"]) + def test_uninstall_operation_flow(self): """Test complete uninstall operation flow.""" plugin_id = "test-plugin" - # First, mark as installed - self.state_manager.set_plugin_installed(plugin_id, "1.0.0") - # Enqueue uninstall operation operation_id = self.operation_queue.enqueue_operation( OperationType.UNINSTALL, @@ -127,13 +111,10 @@ class TestPluginOperationsIntegration(unittest.TestCase): operation_id=operation_id ) - # Update state - remove plugin state - self.state_manager.remove_plugin_state(plugin_id) - - # Verify state - state = self.state_manager.get_plugin_state(plugin_id) - self.assertIsNone(state) - + # Verify history + history = self.operation_history.get_history(plugin_id=plugin_id) + self.assertEqual([r.operation_type for r in history], ["uninstall"]) + def test_operation_history_tracking(self): """Test that operations are tracked in history.""" plugin_id = "test-plugin" diff --git a/test/web_interface/test_api_v3_backup_paths.py b/test/web_interface/test_api_v3_backup_paths.py index c929e20c..b7c83449 100644 --- a/test/web_interface/test_api_v3_backup_paths.py +++ b/test/web_interface/test_api_v3_backup_paths.py @@ -30,7 +30,7 @@ from web_interface.blueprints.api_v3 import api_v3 # noqa: E402 _MANAGER_ATTRS = ( 'config_manager', 'plugin_catalog', 'plugin_store_manager', - 'plugin_state_manager', 'saved_repositories_manager', 'schema_manager', + 'saved_repositories_manager', 'schema_manager', 'operation_queue', 'operation_history', 'cache_manager', ) _SENTINEL = object() diff --git a/test/web_interface/test_api_v3_backup_restore.py b/test/web_interface/test_api_v3_backup_restore.py index db6fe38f..a7f0f123 100644 --- a/test/web_interface/test_api_v3_backup_restore.py +++ b/test/web_interface/test_api_v3_backup_restore.py @@ -33,7 +33,7 @@ URL = "/api/v3/backup/restore" _MANAGER_ATTRS = ( 'config_manager', 'plugin_catalog', 'plugin_store_manager', - 'plugin_state_manager', 'saved_repositories_manager', 'schema_manager', + 'saved_repositories_manager', 'schema_manager', 'operation_queue', 'operation_history', 'cache_manager', ) _SENTINEL = object() diff --git a/test/web_interface/test_api_v3_config_raw.py b/test/web_interface/test_api_v3_config_raw.py index 6a711d4a..fe30749b 100644 --- a/test/web_interface/test_api_v3_config_raw.py +++ b/test/web_interface/test_api_v3_config_raw.py @@ -44,7 +44,7 @@ def env(tmp_path): _SENTINEL = object() attrs = ('config_manager', 'plugin_catalog', 'plugin_store_manager', - 'plugin_state_manager', 'saved_repositories_manager', + 'saved_repositories_manager', 'schema_manager', 'operation_queue', 'operation_history', 'cache_manager') originals = {name: getattr(api_v3, name, _SENTINEL) for name in attrs} diff --git a/test/web_interface/test_api_v3_secret_roundtrip.py b/test/web_interface/test_api_v3_secret_roundtrip.py index 3c218775..db13d26a 100644 --- a/test/web_interface/test_api_v3_secret_roundtrip.py +++ b/test/web_interface/test_api_v3_secret_roundtrip.py @@ -86,7 +86,6 @@ def env(tmp_path): api_v3.plugin_store_manager = MagicMock() api_v3.saved_repositories_manager = MagicMock() api_v3.operation_queue = MagicMock() - api_v3.plugin_state_manager = MagicMock() api_v3.operation_history = MagicMock() api_v3.cache_manager = MagicMock() diff --git a/test/web_interface/test_api_v3_unhandled_errors.py b/test/web_interface/test_api_v3_unhandled_errors.py index 067cd476..49ab0dbc 100644 --- a/test/web_interface/test_api_v3_unhandled_errors.py +++ b/test/web_interface/test_api_v3_unhandled_errors.py @@ -41,7 +41,7 @@ EXPECTED = { MANAGERS = ("config_manager", "plugin_catalog", "plugin_store_manager", "saved_repositories_manager", "schema_manager", "operation_queue", - "plugin_state_manager", "operation_history", "cache_manager") + "operation_history", "cache_manager") class Boom: diff --git a/test/web_interface/test_plugin_config_json_saves.py b/test/web_interface/test_plugin_config_json_saves.py index 2be4989e..3187f4dd 100644 --- a/test/web_interface/test_plugin_config_json_saves.py +++ b/test/web_interface/test_plugin_config_json_saves.py @@ -73,7 +73,7 @@ STORED = { } _ATTRS = ('config_manager', 'plugin_catalog', 'plugin_store_manager', - 'plugin_state_manager', 'saved_repositories_manager', 'schema_manager', + 'saved_repositories_manager', 'schema_manager', 'operation_queue', 'operation_history', 'cache_manager') diff --git a/test/web_interface/test_plugin_state_file_retired.py b/test/web_interface/test_plugin_state_file_retired.py new file mode 100644 index 00000000..5fcc82a7 --- /dev/null +++ b/test/web_interface/test_plugin_state_file_retired.py @@ -0,0 +1,166 @@ +"""data/plugin_state.json is retired; /plugins/state and reconciliation read +config + disk (desired) and the display's runtime snapshot (observed). + +The file held, per plugin, an enabled flag (a copy of config.json's), a +version (a copy of the manifest's, when anything set it at all), a status +derived from those two, and install/update timestamps that only +GET /api/v3/plugins/state ever returned -- the operation history records the +same events. Nothing in it was needed that cannot be derived, so it is not +migrated: nothing reads or writes it any more, and an existing file is left +in place, unread. These tests hold that line and check the replacements. +""" +import ast +import json +import time +from datetime import datetime +from pathlib import Path +from unittest.mock import MagicMock + +import pytest + +from src.plugin_system.operation_history import OperationRecord +from src.plugin_system.plugin_runtime import SNAPSHOT_SCHEMA, view_from_snapshot +from src.plugin_system.state_reconciliation import StateReconciliation +from test._api_v3_test_helpers import api_v3_client, api_v3_module # noqa: F401 + +REPO = Path(__file__).resolve().parents[2] + + +def _code_strings(path): + """String constants in a module, docstrings excluded.""" + tree = ast.parse(path.read_text(encoding="utf-8")) + docstrings = {id(node.value) for node in ast.walk(tree) + if isinstance(node, ast.Expr) and isinstance(node.value, ast.Constant)} + return [node.value for node in ast.walk(tree) + if isinstance(node, ast.Constant) and isinstance(node.value, str) + and id(node) not in docstrings] + + +def test_no_code_reads_or_writes_the_state_file(): + offenders = [] + for root in ("src", "web_interface", "scripts"): + for path in (REPO / root).rglob("*.py"): + if any("plugin_state.json" in s for s in _code_strings(path)): + offenders.append(str(path.relative_to(REPO))) + assert offenders == [] + + +def test_the_web_side_state_manager_is_gone(): + import importlib.util + assert importlib.util.find_spec("src.plugin_system.state_manager") is None + from web_interface.blueprints.api_v3 import api_v3 + import web_interface.app as app_module + assert not hasattr(app_module, "plugin_state_manager") + assert getattr(api_v3, "plugin_state_manager", None) is None + + +def _install(plugins_dir, plugin_id, version): + d = plugins_dir / plugin_id + d.mkdir(parents=True) + (d / "manifest.json").write_text(json.dumps({"id": plugin_id, "version": version}), + encoding="utf-8") + + +def _live(plugins): + now = time.time() + return view_from_snapshot({"schema": SNAPSHOT_SCHEMA, "running": True, + "published_at": now, "plugins": plugins}, now=now) + + +def test_an_existing_state_file_changes_nothing(tmp_path): + """A device upgraded with a plugin_state.json that disagrees with config + and disk: reconciliation answers exactly as on a device without one.""" + plugins_dir = tmp_path / "plugin-repos" + _install(plugins_dir, "clock", "1.0.0") + config = MagicMock() + config.load_config.return_value = {"clock": {"enabled": True}} + config.get_secrets_path.return_value = str(tmp_path / "none.json") + + def run(): + r = StateReconciliation(config_manager=config, plugins_dir=plugins_dir) + result = r.reconcile_state() + return ([i.inconsistency_type for i in result.inconsistencies_found], + r.plugin_states()) + + without = run() + (tmp_path / "data").mkdir() + (tmp_path / "data" / "plugin_state.json").write_text(json.dumps({ + "version": 1, "states": { + "clock": {"plugin_id": "clock", "status": "disabled", "enabled": False, + "version": "0.0.1"}, + "gone": {"plugin_id": "gone", "status": "installed", "enabled": True}}}), + encoding="utf-8") + + assert run() == without + assert without[1]["clock"]["enabled"] is True + assert "gone" not in without[1] + + +@pytest.fixture +def state_api(api_v3_module, api_v3_client, tmp_path, monkeypatch): # noqa: F811 + api = api_v3_module.api_v3 + plugins_dir = tmp_path / "plugin-repos" + _install(plugins_dir, "clock", "1.1.0") + _install(plugins_dir, "weather", "2.0.0") + api.plugin_catalog.plugins_dir = str(plugins_dir) + api.config_manager.load_config = MagicMock(return_value={ + "clock": {"enabled": True}, "weather": {"enabled": False}, + "ghost": {"enabled": True}}) + api.config_manager.get_secrets_path = MagicMock(return_value=str(tmp_path / "s.json")) + api.operation_history.get_history = MagicMock(return_value=[ + OperationRecord("3", "update", "clock", datetime(2026, 9, 3), "success"), + OperationRecord("2", "update", "clock", datetime(2026, 9, 2), "failed"), + OperationRecord("1", "install", "clock", datetime(2026, 9, 1), "success"), + ]) + observed = {"view": _live({"clock": {"loaded": True, "state": "enabled", "error": None, + "version": "1.0.0", "loaded_at": 50.0}})} + import web_interface.blueprints.api_v3 as pkg + monkeypatch.setattr(pkg, "_plugin_runtime_view", lambda: observed["view"]) + return api_v3_client, observed + + +class TestPluginStateRoute: + def test_every_plugin_desired_and_observed(self, state_api): + client, _ = state_api + body = client.get("/api/v3/plugins/state").get_json() + + assert body["status"] == "success" + assert body["runtime"]["status"] == "live" + data = body["data"] + assert set(data) == {"clock", "weather", "ghost"} + clock = data["clock"] + assert clock["status"] == "enabled" + assert (clock["installed"], clock["enabled"], clock["version"]) == (True, True, "1.1.0") + assert (clock["loaded"], clock["loaded_version"]) == (True, "1.0.0") + assert clock["installed_at"] == "2026-09-01T00:00:00" + assert clock["last_updated"] == "2026-09-03T00:00:00" + assert data["weather"]["status"] == "disabled" + assert data["weather"]["loaded"] is False + assert data["weather"]["installed_at"] is None + assert data["ghost"]["status"] == "unknown" # configured, not installed + + def test_one_plugin(self, state_api): + client, _ = state_api + body = client.get("/api/v3/plugins/state?plugin_id=clock").get_json() + assert body["data"]["plugin_id"] == "clock" + response = client.get("/api/v3/plugins/state?plugin_id=nope") + assert response.status_code == 404 + + def test_display_not_running_leaves_observed_fields_null(self, state_api): + client, observed = state_api + observed["view"] = view_from_snapshot({"schema": SNAPSHOT_SCHEMA, "running": False, + "published_at": time.time()}) + body = client.get("/api/v3/plugins/state").get_json() + assert body["runtime"]["status"] == "stopped" + assert body["data"]["clock"]["loaded"] is None + assert body["data"]["clock"]["enabled"] is True + + def test_reconcile_reports_what_the_display_runs(self, state_api): + client, _ = state_api + body = client.post("/api/v3/plugins/state/reconcile", json={}).get_json() + found = {(i["plugin_id"], i["type"], i["fix_action"]) + for i in body["data"]["inconsistencies"]} + assert ("clock", "plugin_version_mismatch", "no_action") in found + assert ("ghost", "plugin_missing_on_disk", "manual_fix_required") in found + # Reported only; nothing the user must fix beyond the missing plugin. + assert [i["plugin_id"] for i in body["data"]["manual_fix_required"]] == ["ghost"] diff --git a/test/web_interface/test_state_reconciliation.py b/test/web_interface/test_state_reconciliation.py index b8e92613..fcab4012 100644 --- a/test/web_interface/test_state_reconciliation.py +++ b/test/web_interface/test_state_reconciliation.py @@ -1,338 +1,352 @@ """ Tests for state reconciliation system. + +Desired state is config.json plus the plugins on disk; observed state is +the runtime snapshot the display publishes (plugin_runtime). There is no +third, persisted record: data/plugin_state.json is retired. """ import unittest import tempfile import shutil import json +import time from pathlib import Path from unittest.mock import Mock, patch +from src.plugin_system.plugin_runtime import ( + PluginRuntimeView, SNAPSHOT_SCHEMA, view_from_snapshot, +) from src.plugin_system.state_reconciliation import ( StateReconciliation, InconsistencyType, FixAction, ReconciliationResult ) -from src.plugin_system.state_manager import PluginStateManager, PluginStateStatus + + +def live_view(plugins): + """A live runtime view, as read from a fresh snapshot of a running display.""" + now = time.time() + return view_from_snapshot({ + "schema": SNAPSHOT_SCHEMA, "running": True, "published_at": now, + "stale_after": 180, "plugins": plugins, + }, now=now) + + +def stale_view(plugins): + """The same snapshot, read long after the display stopped refreshing it.""" + now = time.time() + return view_from_snapshot({ + "schema": SNAPSHOT_SCHEMA, "running": True, "published_at": now - 3600, + "stale_after": 180, "plugins": plugins, + }, now=now) class TestStateReconciliation(unittest.TestCase): """Test state reconciliation system.""" - + def setUp(self): """Set up test fixtures.""" self.temp_dir = Path(tempfile.mkdtemp()) self.plugins_dir = self.temp_dir / "plugins" self.plugins_dir.mkdir() - - # Create mock managers - self.state_manager = Mock(spec=PluginStateManager) + self.config_manager = Mock() - self.plugin_manager = Mock() - + # What the display reports; tests replace it. + self.observed = PluginRuntimeView(status="unknown") + # Initialize reconciliation system self.reconciler = StateReconciliation( - state_manager=self.state_manager, config_manager=self.config_manager, - plugin_manager=self.plugin_manager, - plugins_dir=self.plugins_dir + plugins_dir=self.plugins_dir, + runtime_source=lambda: self.observed, ) - + def tearDown(self): """Clean up test fixtures.""" shutil.rmtree(self.temp_dir) - + + def _install(self, plugin_id, version="1.0.0"): + plugin_dir = self.plugins_dir / plugin_id + plugin_dir.mkdir() + with open(plugin_dir / "manifest.json", 'w') as f: + json.dump({"id": plugin_id, "version": version, "name": plugin_id}, f) + def test_reconcile_no_inconsistencies(self): - """Test reconciliation with no inconsistencies.""" - # Setup: All states are consistent + """Config, disk and the display agree.""" self.config_manager.load_config.return_value = { "plugin1": {"enabled": True} } - - self.state_manager.get_all_states.return_value = { - "plugin1": Mock( - enabled=True, - status=PluginStateStatus.ENABLED, - version="1.0.0" - ) - } - - self.plugin_manager.plugin_manifests = {"plugin1": {}} - self.plugin_manager.plugins = {"plugin1": Mock()} - - # Create plugin directory - plugin_dir = self.plugins_dir / "plugin1" - plugin_dir.mkdir() - manifest_path = plugin_dir / "manifest.json" - with open(manifest_path, 'w') as f: - json.dump({"version": "1.0.0", "name": "Plugin 1"}, f) - - # Run reconciliation + self._install("plugin1") + self.observed = live_view({"plugin1": { + "loaded": True, "state": "enabled", "error": None, + "version": "1.0.0", "loaded_at": time.time()}}) + result = self.reconciler.reconcile_state() - - # Verify + self.assertIsInstance(result, ReconciliationResult) self.assertEqual(len(result.inconsistencies_found), 0) self.assertTrue(result.reconciliation_successful) - + def test_plugin_missing_in_config(self): """Test detection of plugin missing in config.""" - # Setup: Plugin exists on disk but not in config self.config_manager.load_config.return_value = {} - - self.state_manager.get_all_states.return_value = {} - - self.plugin_manager.plugin_manifests = {} - self.plugin_manager.plugins = {} - - # Create plugin directory - plugin_dir = self.plugins_dir / "plugin1" - plugin_dir.mkdir() - manifest_path = plugin_dir / "manifest.json" - with open(manifest_path, 'w') as f: - json.dump({"version": "1.0.0", "name": "Plugin 1"}, f) - - # Run reconciliation + self._install("plugin1") + result = self.reconciler.reconcile_state() - - # Verify inconsistency detected + self.assertEqual(len(result.inconsistencies_found), 1) inconsistency = result.inconsistencies_found[0] self.assertEqual(inconsistency.plugin_id, "plugin1") self.assertEqual(inconsistency.inconsistency_type, InconsistencyType.PLUGIN_MISSING_IN_CONFIG) self.assertTrue(inconsistency.can_auto_fix) self.assertEqual(inconsistency.fix_action, FixAction.AUTO_FIX) - + def test_plugin_missing_on_disk(self): """Test detection of plugin missing on disk.""" - # Setup: Plugin in config but not on disk self.config_manager.load_config.return_value = { "plugin1": {"enabled": True} } - - self.state_manager.get_all_states.return_value = {} - - self.plugin_manager.plugin_manifests = {} - self.plugin_manager.plugins = {} - - # Don't create plugin directory - - # Run reconciliation + result = self.reconciler.reconcile_state() - - # Verify inconsistency detected + self.assertEqual(len(result.inconsistencies_found), 1) inconsistency = result.inconsistencies_found[0] self.assertEqual(inconsistency.plugin_id, "plugin1") self.assertEqual(inconsistency.inconsistency_type, InconsistencyType.PLUGIN_MISSING_ON_DISK) self.assertFalse(inconsistency.can_auto_fix) self.assertEqual(inconsistency.fix_action, FixAction.MANUAL_FIX_REQUIRED) - - def test_enabled_state_mismatch(self): - """Test detection of enabled state mismatch.""" - # Setup: Config says enabled=True, state manager says enabled=False + + def test_enabled_but_not_loaded_is_reported_not_fixed(self): + """Enabled in config, installed, but the display reports a failed load: + a finding that names the error, left alone (the display loads by + config on its own) and not counted as needing manual attention.""" self.config_manager.load_config.return_value = { "plugin1": {"enabled": True} } - - self.state_manager.get_all_states.return_value = { - "plugin1": Mock( - enabled=False, - status=PluginStateStatus.DISABLED, - version="1.0.0" - ) - } - - self.plugin_manager.plugin_manifests = {"plugin1": {}} - self.plugin_manager.plugins = {} - - # Create plugin directory - plugin_dir = self.plugins_dir / "plugin1" - plugin_dir.mkdir() - manifest_path = plugin_dir / "manifest.json" - with open(manifest_path, 'w') as f: - json.dump({"version": "1.0.0", "name": "Plugin 1"}, f) - - # Run reconciliation + self._install("plugin1") + self.observed = live_view({"plugin1": { + "loaded": False, "state": "error", + "error": {"type": "ImportError", "message": "No module named 'x'", + "at": time.time(), "recoverable": False}, + "version": None, "loaded_at": None}}) + self.config_manager.save_config = Mock() + result = self.reconciler.reconcile_state() - - # Verify inconsistency detected + self.assertEqual(len(result.inconsistencies_found), 1) inconsistency = result.inconsistencies_found[0] - self.assertEqual(inconsistency.plugin_id, "plugin1") self.assertEqual(inconsistency.inconsistency_type, InconsistencyType.PLUGIN_ENABLED_MISMATCH) - self.assertTrue(inconsistency.can_auto_fix) - self.assertEqual(inconsistency.fix_action, FixAction.AUTO_FIX) - + self.assertEqual(inconsistency.fix_action, FixAction.NO_ACTION) + self.assertFalse(inconsistency.can_auto_fix) + self.assertIn("No module named", inconsistency.description) + self.assertEqual(result.inconsistencies_fixed, []) + self.assertEqual(result.inconsistencies_manual, []) + self.assertTrue(result.reconciliation_successful) + self.config_manager.save_config.assert_not_called() + + def test_enabled_and_absent_from_live_snapshot_is_not_loaded(self): + """A plugin a live snapshot does not list is not loaded.""" + self.config_manager.load_config.return_value = { + "plugin1": {"enabled": True} + } + self._install("plugin1") + self.observed = live_view({}) + + result = self.reconciler.reconcile_state() + + types = [i.inconsistency_type for i in result.inconsistencies_found] + self.assertEqual(types, [InconsistencyType.PLUGIN_ENABLED_MISMATCH]) + + def test_disabled_and_not_loaded_agrees(self): + """Missing "enabled" is disabled (the display's rule), and a plugin + the display has not loaded matches it.""" + self.config_manager.load_config.return_value = {"plugin1": {}} + self._install("plugin1") + self.observed = live_view({}) + + result = self.reconciler.reconcile_state() + + self.assertEqual(result.inconsistencies_found, []) + + def test_loaded_at_an_older_version_is_reported(self): + """The display runs 1.0.0 while 2.0.0 is on disk: restart needed.""" + self.config_manager.load_config.return_value = { + "plugin1": {"enabled": True} + } + self._install("plugin1", version="2.0.0") + self.observed = live_view({"plugin1": { + "loaded": True, "state": "enabled", "error": None, + "version": "1.0.0", "loaded_at": time.time()}}) + + result = self.reconciler.reconcile_state() + + self.assertEqual(len(result.inconsistencies_found), 1) + inconsistency = result.inconsistencies_found[0] + self.assertEqual(inconsistency.inconsistency_type, InconsistencyType.PLUGIN_VERSION_MISMATCH) + self.assertEqual(inconsistency.fix_action, FixAction.NO_ACTION) + self.assertIn("restart", inconsistency.description) + self.assertTrue(result.reconciliation_successful) + + def test_stale_or_missing_snapshot_is_not_compared(self): + """Observed state that is not live says nothing: no findings from it.""" + self.config_manager.load_config.return_value = { + "plugin1": {"enabled": True} + } + self._install("plugin1") + dead = {"plugin1": {"loaded": False, "state": "error", "error": None, + "version": None, "loaded_at": None}} + + for observed in (stale_view(dead), PluginRuntimeView(status="stopped"), + PluginRuntimeView(status="unknown")): + self.observed = observed + result = self.reconciler.reconcile_state() + self.assertEqual(result.inconsistencies_found, [], observed.status) + + def test_no_runtime_source_is_unknown(self): + self.config_manager.load_config.return_value = { + "plugin1": {"enabled": True} + } + self._install("plugin1") + reconciler = StateReconciliation(config_manager=self.config_manager, + plugins_dir=self.plugins_dir) + self.assertEqual(reconciler.reconcile_state().inconsistencies_found, []) + + def test_runtime_source_that_raises_is_unknown(self): + self.config_manager.load_config.return_value = { + "plugin1": {"enabled": True} + } + self._install("plugin1") + + def boom(): + raise RuntimeError("cache unreadable") + + reconciler = StateReconciliation(config_manager=self.config_manager, + plugins_dir=self.plugins_dir, + runtime_source=boom) + result = reconciler.reconcile_state() + self.assertEqual(result.inconsistencies_found, []) + self.assertTrue(result.reconciliation_successful) + + def test_old_constructor_arguments_are_refused(self): + """state_manager / plugin_manager are gone; passing them is an error, + not something silently ignored.""" + with self.assertRaises(TypeError): + StateReconciliation(state_manager=Mock(), config_manager=self.config_manager, + plugin_manager=Mock(), plugins_dir=self.plugins_dir) + + def test_plugin_states_combines_desired_and_observed(self): + """What /plugins/state serves in place of plugin_state.json.""" + self.config_manager.load_config.return_value = { + "plugin1": {"enabled": True}, + "plugin2": {"enabled": False}, + "ghost": {"enabled": True}, + } + self._install("plugin1", version="1.2.0") + self._install("plugin2") + self._install("plugin3") + self.observed = live_view({"plugin1": { + "loaded": True, "state": "enabled", "error": None, + "version": "1.2.0", "loaded_at": 1234.0}}) + + states = self.reconciler.plugin_states() + + self.assertEqual(set(states), {"plugin1", "plugin2", "plugin3", "ghost"}) + self.assertEqual(states["plugin1"], { + "plugin_id": "plugin1", "installed": True, "version": "1.2.0", + "in_config": True, "enabled": True, "loaded": True, + "state": "enabled", "error_info": None, + "loaded_version": "1.2.0", "loaded_at": 1234.0, + }) + self.assertFalse(states["plugin2"]["enabled"]) + self.assertIs(states["plugin2"]["loaded"], False) + self.assertEqual(states["plugin2"]["state"], "unloaded") + self.assertFalse(states["plugin3"]["in_config"]) + self.assertFalse(states["ghost"]["installed"]) + + def test_plugin_states_without_a_live_snapshot_reports_unknown(self): + self.config_manager.load_config.return_value = {"plugin1": {"enabled": True}} + self._install("plugin1") + self.observed = stale_view({"plugin1": {"loaded": True, "state": "enabled"}}) + + record = self.reconciler.plugin_states()["plugin1"] + + self.assertTrue(record["enabled"]) + self.assertIsNone(record["loaded"]) + self.assertIsNone(record["state"]) + self.assertIsNone(record["error_info"]) + def test_auto_fix_plugin_missing_in_config(self): """Test auto-fix of plugin missing in config.""" - # Setup self.config_manager.load_config.return_value = {} - - self.state_manager.get_all_states.return_value = {} - - self.plugin_manager.plugin_manifests = {} - self.plugin_manager.plugins = {} - - # Create plugin directory - plugin_dir = self.plugins_dir / "plugin1" - plugin_dir.mkdir() - manifest_path = plugin_dir / "manifest.json" - with open(manifest_path, 'w') as f: - json.dump({"version": "1.0.0", "name": "Plugin 1"}, f) - - # Mock save_config to track calls + self._install("plugin1") + saved_configs = [] + def save_config(config): saved_configs.append(config) - + self.config_manager.save_config = save_config - - # Run reconciliation + result = self.reconciler.reconcile_state() - - # Verify fix was attempted + self.assertEqual(len(result.inconsistencies_fixed), 1) self.assertEqual(len(saved_configs), 1) self.assertIn("plugin1", saved_configs[0]) self.assertEqual(saved_configs[0]["plugin1"]["enabled"], False) - - def test_auto_fix_enabled_state_mismatch(self): - """Test auto-fix of enabled state mismatch.""" - # Setup: Config says enabled=True, state manager says enabled=False - self.config_manager.load_config.return_value = { - "plugin1": {"enabled": True} - } - - self.state_manager.get_all_states.return_value = { - "plugin1": Mock( - enabled=False, - status=PluginStateStatus.DISABLED, - version="1.0.0" - ) - } - - self.plugin_manager.plugin_manifests = {"plugin1": {}} - self.plugin_manager.plugins = {} - - # Create plugin directory - plugin_dir = self.plugins_dir / "plugin1" - plugin_dir.mkdir() - manifest_path = plugin_dir / "manifest.json" - with open(manifest_path, 'w') as f: - json.dump({"version": "1.0.0", "name": "Plugin 1"}, f) - - # Run reconciliation - result = self.reconciler.reconcile_state() - # config.json is the source of truth for enabled state. The fix syncs - # the state manager to match config (config says True → state set True), - # rather than overwriting the config with the stale state value. - self.assertEqual(len(result.inconsistencies_fixed), 1) - self.state_manager.set_plugin_enabled.assert_called_once_with("plugin1", True) - def test_multiple_inconsistencies(self): """Test reconciliation with multiple inconsistencies.""" - # Setup: Multiple plugins with different issues self.config_manager.load_config.return_value = { "plugin1": {"enabled": True}, # Exists in config but not on disk # plugin2 exists on disk but not in config } - - self.state_manager.get_all_states.return_value = { - "plugin1": Mock( - enabled=True, - status=PluginStateStatus.ENABLED, - version="1.0.0" - ) - } - - self.plugin_manager.plugin_manifests = {} - self.plugin_manager.plugins = {} - - # Create plugin2 directory (exists on disk but not in config) - plugin2_dir = self.plugins_dir / "plugin2" - plugin2_dir.mkdir() - manifest_path = plugin2_dir / "manifest.json" - with open(manifest_path, 'w') as f: - json.dump({"version": "1.0.0", "name": "Plugin 2"}, f) - - # Run reconciliation + self._install("plugin2") + result = self.reconciler.reconcile_state() - - # Verify multiple inconsistencies found + self.assertGreaterEqual(len(result.inconsistencies_found), 2) - - # Check types inconsistency_types = [inc.inconsistency_type for inc in result.inconsistencies_found] self.assertIn(InconsistencyType.PLUGIN_MISSING_ON_DISK, inconsistency_types) self.assertIn(InconsistencyType.PLUGIN_MISSING_IN_CONFIG, inconsistency_types) - + def test_reconciliation_with_exception(self): """Test reconciliation handles exceptions gracefully.""" - # Setup: State manager raises exception when getting states - self.config_manager.load_config.return_value = {} - self.state_manager.get_all_states.side_effect = Exception("State manager error") - - # Run reconciliation + self.config_manager.load_config.side_effect = Exception("Config error") + result = self.reconciler.reconcile_state() - - # Verify error is handled - reconciliation may still succeed if other sources work + self.assertIsInstance(result, ReconciliationResult) - # Note: Reconciliation may still succeed if other sources provide valid state - + def test_fix_failure_handling(self): """Test that fix failures are handled correctly.""" - # Setup: Plugin missing in config, but save fails self.config_manager.load_config.return_value = {} - - self.state_manager.get_all_states.return_value = {} - - self.plugin_manager.plugin_manifests = {} - self.plugin_manager.plugins = {} - - # Create plugin directory - plugin_dir = self.plugins_dir / "plugin1" - plugin_dir.mkdir() - manifest_path = plugin_dir / "manifest.json" - with open(manifest_path, 'w') as f: - json.dump({"version": "1.0.0", "name": "Plugin 1"}, f) - - # Mock save_config to raise exception + self._install("plugin1") self.config_manager.save_config.side_effect = Exception("Save failed") - - # Run reconciliation + result = self.reconciler.reconcile_state() - - # Verify inconsistency detected but not fixed + self.assertEqual(len(result.inconsistencies_found), 1) self.assertEqual(len(result.inconsistencies_fixed), 0) self.assertEqual(len(result.inconsistencies_manual), 1) - + def test_get_config_state_handles_exception(self): """Test that _get_config_state handles exceptions.""" - # Setup: Config manager raises exception self.config_manager.load_config.side_effect = Exception("Config error") - - # Call method directly + state = self.reconciler._get_config_state() - - # Verify empty state returned + self.assertEqual(state, {}) - + def test_get_disk_state_handles_exception(self): """Test that _get_disk_state handles exceptions.""" - # Setup: Make plugins_dir inaccessible with patch.object(self.reconciler, 'plugins_dir', create=True) as mock_dir: mock_dir.exists.side_effect = Exception("Disk error") mock_dir.iterdir.side_effect = Exception("Disk error") - - # Call method directly + state = self.reconciler._get_disk_state() - - # Verify empty state returned + self.assertEqual(state, {}) @@ -352,15 +366,10 @@ class TestStateReconciliationUnrecoverable(unittest.TestCase): self.plugins_dir = self.temp_dir / "plugins" self.plugins_dir.mkdir() - self.state_manager = Mock(spec=PluginStateManager) - self.state_manager.get_all_states.return_value = {} self.config_manager = Mock() self.config_manager.load_config.return_value = { "ghost": {"enabled": True} } - self.plugin_manager = Mock() - self.plugin_manager.plugin_manifests = {} - self.plugin_manager.plugins = {} # Store manager with an empty registry — install_plugin always fails self.store_manager = Mock() @@ -372,9 +381,7 @@ class TestStateReconciliationUnrecoverable(unittest.TestCase): self.store_manager.is_plugin_uninstalled.return_value = False self.reconciler = StateReconciliation( - state_manager=self.state_manager, config_manager=self.config_manager, - plugin_manager=self.plugin_manager, plugins_dir=self.plugins_dir, store_manager=self.store_manager, ) @@ -483,9 +490,7 @@ class TestStateReconciliationUnrecoverable(unittest.TestCase): ) reconciler = StateReconciliation( - state_manager=self.state_manager, config_manager=self.config_manager, - plugin_manager=self.plugin_manager, plugins_dir=self.plugins_dir, store_manager=real_store, ) diff --git a/test/web_interface/test_update_all_plugins.py b/test/web_interface/test_update_all_plugins.py index 8d88a012..55ac31fb 100644 --- a/test/web_interface/test_update_all_plugins.py +++ b/test/web_interface/test_update_all_plugins.py @@ -43,7 +43,6 @@ def store(tmp_path): with patch.object(api_v3, 'plugin_store_manager', sm, 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): yield sm 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 index d802a48e..9956f02e 100644 --- a/test/web_interface/test_web_process_runs_no_plugin_code.py +++ b/test/web_interface/test_web_process_runs_no_plugin_code.py @@ -194,8 +194,11 @@ class TestTheWebProcessNeverRunsAPlugin: 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. + # No display has published a runtime snapshot here, so these are + # unknown rather than invented (test_plugin_runtime_snapshot.py + # covers a live one). assert entry["loaded"] is None and entry["state"] is None + assert body["data"]["runtime"]["status"] == "unknown" # 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"]) == ( diff --git a/web_interface/app.py b/web_interface/app.py index 267307bc..d8862c78 100644 --- a/web_interface/app.py +++ b/web_interface/app.py @@ -39,7 +39,6 @@ from src.plugin_system.store_manager import PluginStoreManager from src.plugin_system.saved_repositories import SavedRepositoriesManager from src.plugin_system.schema_manager import SchemaManager from src.plugin_system.operation_queue import PluginOperationQueue -from src.plugin_system.state_manager import PluginStateManager from src.plugin_system.operation_history import OperationHistory _JOURNALCTL = shutil.which('journalctl') @@ -158,13 +157,10 @@ plugin_catalog = PluginCatalog( # Initialize operation queue for plugin operations operation_queue = PluginOperationQueue(max_history=500) -# Initialize plugin state manager -# Use lazy_load=True to defer file loading until first use (improves startup time) -plugin_state_manager = PluginStateManager( - state_file=str(project_root / "data" / "plugin_state.json"), - auto_save=True, - lazy_load=True -) +# No plugin state file: data/plugin_state.json is retired. Desired state is +# config.json plus the plugins on disk, observed state is the runtime +# snapshot the display publishes (src/plugin_system/plugin_runtime.py). An +# existing file is left where it is, unread; see docs/ARCHITECTURE.md. # Initialize operation history # Use lazy_load=True to defer file loading until first use (improves startup time) @@ -194,7 +190,6 @@ api_v3.plugin_store_manager = plugin_store_manager api_v3.saved_repositories_manager = saved_repositories_manager api_v3.schema_manager = schema_manager api_v3.operation_queue = operation_queue -api_v3.plugin_state_manager = plugin_state_manager api_v3.operation_history = operation_history # Initialize cache manager for API endpoints from src.cache_manager import CacheManager @@ -965,12 +960,12 @@ def _run_startup_reconciliation() -> None: try: from src.plugin_system.state_reconciliation import StateReconciliation + from src.plugin_system.plugin_runtime import read_plugin_runtime reconciler = StateReconciliation( - state_manager=plugin_state_manager, config_manager=config_manager, - plugin_manager=plugin_catalog, plugins_dir=plugins_dir, - store_manager=plugin_store_manager + store_manager=plugin_store_manager, + runtime_source=lambda: read_plugin_runtime(api_v3.cache_manager), ) result = reconciler.reconcile_state() if result.inconsistencies_found: diff --git a/web_interface/blueprints/api_v3/__init__.py b/web_interface/blueprints/api_v3/__init__.py index 6638fa7f..0a24f064 100644 --- a/web_interface/blueprints/api_v3/__init__.py +++ b/web_interface/blueprints/api_v3/__init__.py @@ -720,8 +720,6 @@ def _do_transactional_uninstall(plugin_id, preserve_config): # --- Step 4: finish --- if api_v3.schema_manager: api_v3.schema_manager.invalidate_cache(plugin_id) - if api_v3.plugin_state_manager: - api_v3.plugin_state_manager.remove_plugin_state(plugin_id) # Persistently record the uninstall so a later core `git pull` update # cannot resurrect a built-in plugin (committed under plugin-repos/) that # the user removed. Best-effort: never fail the uninstall over this. @@ -730,6 +728,17 @@ 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_runtime_view(): + """What the display publishes about its plugins (loaded, lifecycle + state, last error, version loaded), judged for staleness. + + Only a ``live`` view reports those facts; a stale, stopped or missing + snapshot answers None for them (see src/plugin_system/plugin_runtime.py). + """ + from src.plugin_system.plugin_runtime import read_plugin_runtime + return read_plugin_runtime(getattr(api_v3, 'cache_manager', None)) + + def _plugin_enabled_in_config(plugin_id: str) -> bool: """Whether config.json enables ``plugin_id``, by the display's rule. diff --git a/web_interface/blueprints/api_v3/plugin_operations.py b/web_interface/blueprints/api_v3/plugin_operations.py index 56f43071..f21299be 100644 --- a/web_interface/blueprints/api_v3/plugin_operations.py +++ b/web_interface/blueprints/api_v3/plugin_operations.py @@ -8,6 +8,7 @@ from web_interface.blueprints.api_v3 import ( exception_error_response, json, jsonify, logger, os, request, stat, success_response, tempfile, ) +import web_interface.blueprints.api_v3 as _pkg @api_v3.route('/plugins/operation/', methods=['GET']) @@ -81,54 +82,102 @@ def clear_operation_history() -> Response: return success_response(message='Operation history cleared') +def _reconciler(store_manager=None): + """A StateReconciliation over config + disk (desired) and the display's + runtime snapshot (observed); None when the catalog is not set up.""" + if not api_v3.plugin_catalog or not api_v3.config_manager: + return None + from src.plugin_system.state_reconciliation import StateReconciliation + return StateReconciliation( + config_manager=api_v3.config_manager, + plugins_dir=Path(api_v3.plugin_catalog.plugins_dir), + store_manager=store_manager, + runtime_source=_pkg._plugin_runtime_view, + ) + + +def _operation_times(plugin_ids): + """``installed_at`` / ``last_updated`` per plugin from the operation + history: the newest successful install, and the newest successful + install or update. None where the history has no record (it keeps the + last 1000 operations, and can be cleared).""" + times = {pid: {'installed_at': None, 'last_updated': None} for pid in plugin_ids} + history = getattr(api_v3, 'operation_history', None) + if not history or not times: + return times + try: + records = history.get_history(limit=100000) + except Exception: + logger.debug("[PluginState] Could not read the operation history", exc_info=True) + return times + for record in records: # newest first + entry = times.get(record.plugin_id) + if entry is None or record.status != 'success': + continue + when = record.timestamp.isoformat() + if record.operation_type == 'install' and entry['installed_at'] is None: + entry['installed_at'] = when + if record.operation_type in ('install', 'update') and entry['last_updated'] is None: + entry['last_updated'] = when + return times + + +def _plugin_state_status(record): + """The old plugin_state.json ``status`` vocabulary, derived.""" + if record.get('state') == 'error': + return 'error' + if not record.get('installed'): + return 'unknown' + return 'enabled' if record.get('enabled') else 'disabled' + + @api_v3.route('/plugins/state', methods=['GET']) def get_plugin_state(): - """Get plugin state from state manager""" + """Every plugin's state: desired (config + disk) and observed (the + display's runtime snapshot). + + Built on each request -- there is no state file any more + (data/plugin_state.json is retired). ``loaded``, ``state``, + ``error_info``, ``loaded_version`` and ``loaded_at`` are null unless the + display's snapshot is live; ``runtime`` beside ``data`` says whether it is. + """ try: - if not api_v3.plugin_state_manager: + reconciler = _reconciler() + if reconciler is None: return error_response( ErrorCode.SYSTEM_ERROR, - 'State manager not initialized', + 'Plugin catalog not initialized', status_code=500 ) - plugin_id = request.args.get('plugin_id') + states = reconciler.plugin_states() + times = _operation_times(states.keys()) + for plugin_id, record in states.items(): + record['status'] = _plugin_state_status(record) + record.update(times.get(plugin_id, {})) + runtime = _pkg._plugin_runtime_view().describe() + plugin_id = request.args.get('plugin_id') if plugin_id: - # Get state for specific plugin - state = api_v3.plugin_state_manager.get_plugin_state(plugin_id) + state = states.get(plugin_id) if not state: return error_response( ErrorCode.PLUGIN_NOT_FOUND, - f'Plugin {plugin_id} not found in state manager', + f'Plugin {plugin_id} is neither installed nor configured', context={'plugin_id': plugin_id}, status_code=404 ) - return success_response(data=state.to_dict()) - else: - # Get all plugin states - all_states = api_v3.plugin_state_manager.get_all_states() - return success_response(data={ - plugin_id: state.to_dict() - for plugin_id, state in all_states.items() - }) + return success_response(data=state, extra={'runtime': runtime}) + return success_response(data=states, extra={'runtime': runtime}) except Exception as e: return exception_error_response(e, ErrorCode.SYSTEM_ERROR) @api_v3.route('/plugins/state/reconcile', methods=['POST']) def reconcile_plugin_state(): - """Reconcile plugin state across all sources""" + """Reconcile desired state (config + disk) with what is installed and + what the display reports running.""" try: - if not api_v3.plugin_state_manager or not api_v3.plugin_catalog: - return error_response( - ErrorCode.SYSTEM_ERROR, - 'State manager or plugin catalog not initialized', - status_code=500 - ) - - from src.plugin_system.state_reconciliation import StateReconciliation - # Parse optional `force` flag from request body, guarding against # non-dict bodies (bare string, array, null) that would raise AttributeError. payload = request.get_json(silent=True) @@ -136,12 +185,13 @@ def reconcile_plugin_state(): payload = {} force = _coerce_to_bool(payload.get('force', False)) - reconciler = StateReconciliation( - state_manager=api_v3.plugin_state_manager, - config_manager=api_v3.config_manager, - plugin_manager=api_v3.plugin_catalog, - plugins_dir=Path(api_v3.plugin_catalog.plugins_dir) - ) + reconciler = _reconciler() + if reconciler is None: + return error_response( + ErrorCode.SYSTEM_ERROR, + 'Config manager or plugin catalog not initialized', + status_code=500 + ) result = reconciler.reconcile_state(force=force) diff --git a/web_interface/blueprints/api_v3/plugin_store.py b/web_interface/blueprints/api_v3/plugin_store.py index 016a3f75..af0cb362 100644 --- a/web_interface/blueprints/api_v3/plugin_store.py +++ b/web_interface/blueprints/api_v3/plugin_store.py @@ -8,7 +8,7 @@ from web_interface.blueprints.api_v3 import ( ErrorCode, OperationType, Path, _do_transactional_uninstall, _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, + error_response, exception_error_response, json, jsonify, logger, request, success_response, validate_request_json, ) from src.common.path_safety import resolve_under, safe_path_component @@ -236,12 +236,8 @@ def update_plugin(): if api_v3.plugin_catalog: api_v3.plugin_catalog.discover_plugins() - # Update state and history - if api_v3.plugin_state_manager: - api_v3.plugin_state_manager.update_plugin_state( - plugin_id, - {'last_updated': datetime.now()} - ) + # Record in history (the only record of when it was updated; + # the version is the manifest on disk). if api_v3.operation_history: version = _get_plugin_version(plugin_id) api_v3.operation_history.record_operation( @@ -467,10 +463,6 @@ def install_plugin(): if api_v3.plugin_catalog: api_v3.plugin_catalog.discover_plugins() - # Update state manager - if api_v3.plugin_state_manager: - api_v3.plugin_state_manager.set_plugin_installed(plugin_id) - # Record in history if api_v3.operation_history: version = _get_plugin_version(plugin_id) @@ -529,8 +521,6 @@ def install_plugin(): api_v3.schema_manager.invalidate_cache(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: version = _get_plugin_version(plugin_id) api_v3.operation_history.record_operation( diff --git a/web_interface/blueprints/api_v3/plugins.py b/web_interface/blueprints/api_v3/plugins.py index 639903ea..71141204 100644 --- a/web_interface/blueprints/api_v3/plugins.py +++ b/web_interface/blueprints/api_v3/plugins.py @@ -47,10 +47,13 @@ def get_installed_plugins(): """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, + from config.json. ``loaded``, ``state``, ``error_info``, + ``loaded_version`` and ``loaded_at`` come from the runtime snapshot the + display publishes (src/plugin_system/plugin_runtime.py), and only while + that snapshot is live: when the display is stopped, hung or has never + published, they are null and ``data.runtime.status`` says why + (``stale``, ``stopped``, ``unknown``) instead of passing on old truth. + Health, metrics and errors are served by /plugins/health, /plugins/metrics and /errors. """ if not api_v3.plugin_catalog or not api_v3.plugin_store_manager: @@ -65,6 +68,8 @@ def get_installed_plugins(): # Load config once before the loop (not per-plugin) full_config = api_v3.config_manager.load_config() if api_v3.config_manager else {} + # One read of the display's snapshot for the whole listing. + runtime = _pkg._plugin_runtime_view() def _build_plugin_entry(plugin_info): plugin_id = plugin_info.get('id') @@ -154,10 +159,9 @@ def get_installed_plugins(): 'icon': plugin_info.get('icon') if isinstance(plugin_info.get('icon'), str) else None, 'enabled': enabled, 'verified': verified, - # Not published by the display process; see the docstring. - 'loaded': None, - 'state': None, - 'error_info': None, + # loaded, state, error_info, loaded_version, loaded_at: the + # display's snapshot, null unless it is live (see the docstring). + **runtime.plugin(plugin_id), 'last_updated': last_updated, 'last_commit': last_commit, 'last_commit_message': last_commit_message, @@ -175,7 +179,8 @@ def get_installed_plugins(): plugins = [r for r in results if r is not None] plugins.extend(_starlark_virtual_plugins()) - return jsonify({'status': 'success', 'data': {'plugins': plugins}}) + return jsonify({'status': 'success', 'data': {'plugins': plugins, + 'runtime': runtime.describe()}}) @api_v3.route('/plugins/toggle', methods=['POST']) @@ -247,10 +252,6 @@ def toggle_plugin(): status_code=500 ) - # Update state manager if available - if api_v3.plugin_state_manager: - api_v3.plugin_state_manager.set_plugin_enabled(plugin_id, enabled) - # Log operation if api_v3.operation_history: api_v3.operation_history.record_operation(