fix(web): find installed plugins before anything has discovered them (#594)

The web process discovers plugins lazily: plugin_manifests is empty until
some endpoint calls discover_plugins(). Three routes consulted it without
discovering, so they misbehaved for as long as nothing else had run --
which, after every ledmatrix-web restart, is until someone opens the
dashboard:

- POST /display/on-demand/start answered 404 "Plugin <id> not found"
  (or "Mode <mode> not found"). Measured on a rig: 404 for over three
  minutes after a web restart, until GET /plugins/installed ran. The
  browser UI loads the plugin list first, so API-only callers (the Home
  Assistant MQTT bridge, scripts) are the ones who hit it.
- POST /plugins/toggle answered 404 "Plugin not found".
- POST /config/main did not recognise a plugin section, so it skipped
  secret separation and merged the section as-is: the plugin's API key
  was written to config.json in plain text instead of config_secrets.json.

Add _discovered_plugin_manifests(), which discovers when nothing has been
yet, and rescans once when a specific plugin id (or, for on-demand by
mode, a mode) is not found, so a plugin installed since the last scan is
found too. _installed_plugin_ids() now uses it.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

test/test_api_v3_lazy_plugin_discovery.py

@@ -0,0 +1,161 @@
+"""Tests that api_v3 routes find installed plugins before anything else has.
+
+The web process discovers plugins lazily (web_interface/app.py): nothing scans
+the plugins directory at startup, and plugin_manifests stays empty until some
+endpoint calls discover_plugins(). Routes that consulted plugin_manifests
+without discovering therefore misbehaved for as long as nothing else had run.
+Measured on a rig after restarting ledmatrix-web:
+
+    POST /api/v3/display/on-demand/start {"plugin_id": "ledmatrix-stocks"}
+    -> 404 "Plugin ledmatrix-stocks not found", for over three minutes,
+       until GET /api/v3/plugins/installed happened to discover plugins.
+
+The browser UI loads the plugin list first, so people rarely saw it; API-only
+callers (the Home Assistant MQTT bridge, scripts) saw it after every restart.
+/plugins/toggle answered the same 404, and /config/main was worse: an
+undiscovered plugin section skipped secret separation and wrote its API key
+into config.json in plain text.
+
+A real PluginManager over a temporary plugins directory, so "empty until
+discovered" is the real behaviour rather than a mock's.
+"""
+
+import json
+import sys
+from pathlib import Path
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+sys.path.insert(0, str(Path(__file__).parent.parent))
+
+from src.config_manager import ConfigManager  # noqa: E402
+from src.plugin_system.plugin_manager import PluginManager  # noqa: E402
+from test._api_v3_test_helpers import api_v3_client, api_v3_module  # noqa: F401,E402
+
+PLUGIN_ID = 'ledmatrix-stocks'
+MODE = 'stocks'
+
+
+def _install(plugins_dir, plugin_id, modes, schema=None):
+    plugin_dir = plugins_dir / plugin_id
+    plugin_dir.mkdir(parents=True)
+    (plugin_dir / 'manifest.json').write_text(json.dumps({
+        'id': plugin_id, 'name': plugin_id, 'version': '1.0.0',
+        'entry_point': 'manager.py', 'class_name': 'Plugin',
+        'display_modes': modes,
+    }))
+    (plugin_dir / 'manager.py').write_text('')
+    if schema is not None:
+        (plugin_dir / 'config_schema.json').write_text(json.dumps(schema))
+
+
+@pytest.fixture
+def plugins_dir(tmp_path):
+    path = tmp_path / 'plugin-repos'
+    path.mkdir()
+    _install(path, PLUGIN_ID, [MODE], schema={
+        'type': 'object',
+        'properties': {
+            'enabled': {'type': 'boolean'},
+            'symbols': {'type': 'string'},
+            'api_key': {'type': 'string', 'x-secret': True},
+        },
+    })
+    return path
+
+
+@pytest.fixture
+def fresh_web_process(api_v3_module, plugins_dir):
+    """The web process right after a restart: nothing discovered yet."""
+    manager = PluginManager(plugins_dir=str(plugins_dir))
+    assert not manager.plugin_manifests
+    api_v3_module.api_v3.plugin_manager = manager
+    return manager
+
+
+@pytest.fixture
+def display_service():
+    """Keep on-demand start away from systemctl and the real cache."""
+    with patch('web_interface.blueprints.api_v3.display._ensure_cache_manager') as cache, \
+         patch('web_interface.blueprints.api_v3.display._get_display_service_status') as status:
+        cache.return_value = MagicMock()
+        status.return_value = {'active': True}
+        yield cache.return_value
+
+
+def _start(client, **body):
+    body.setdefault('start_service', False)
+    return client.post('/api/v3/display/on-demand/start', json=body)
+
+
+class TestOnDemandStart:
+    def test_by_plugin_id(self, api_v3_client, fresh_web_process, display_service):
+        response = _start(api_v3_client, plugin_id=PLUGIN_ID)
+
+        assert response.status_code == 200, response.get_json()
+        request = display_service.set.call_args.args[1]
+        assert (request['plugin_id'], request['mode']) == (PLUGIN_ID, MODE)
+
+    def test_by_mode_alone(self, api_v3_client, fresh_web_process, display_service):
+        response = _start(api_v3_client, mode=MODE)
+
+        assert response.status_code == 200, response.get_json()
+        assert display_service.set.call_args.args[1]['plugin_id'] == PLUGIN_ID
+
+    def test_a_plugin_installed_since_the_last_scan(
+            self, api_v3_client, fresh_web_process, plugins_dir, display_service):
+        fresh_web_process.discover_plugins()
+        _install(plugins_dir, 'ledmatrix-weather', ['weather'])
+
+        assert _start(api_v3_client, plugin_id='ledmatrix-weather').status_code == 200
+        assert _start(api_v3_client, mode=MODE).status_code == 200
+
+    def test_a_mode_installed_since_the_last_scan(
+            self, api_v3_client, fresh_web_process, plugins_dir, display_service):
+        fresh_web_process.discover_plugins()
+        _install(plugins_dir, 'ledmatrix-weather', ['weather'])
+
+        response = _start(api_v3_client, mode='weather')
+
+        assert response.status_code == 200, response.get_json()
+
+    def test_an_unknown_plugin_is_still_not_found(
+            self, api_v3_client, fresh_web_process, display_service):
+        assert _start(api_v3_client, plugin_id='no-such-plugin').status_code == 404
+        assert _start(api_v3_client, mode='no-such-mode').status_code == 404
+        display_service.set.assert_not_called()
+
+
+def test_toggle_finds_the_plugin(api_v3_client, api_v3_module, fresh_web_process):
+    config_manager = api_v3_module.api_v3.config_manager
+    config_manager.load_config.return_value = {PLUGIN_ID: {'enabled': False}}
+    config_manager.save_config_atomic.return_value = MagicMock(
+        status=MagicMock(value='success'))
+
+    response = api_v3_client.post('/api/v3/plugins/toggle',
+                                  json={'plugin_id': PLUGIN_ID, 'enabled': True})
+
+    assert response.status_code == 200, response.get_json()
+    saved = config_manager.save_config_atomic.call_args.args[0]
+    assert saved[PLUGIN_ID]['enabled'] is True
+
+
+def test_main_config_save_keeps_a_plugin_secret_out_of_config_json(
+        api_v3_client, api_v3_module, fresh_web_process, tmp_path):
+    config_file = tmp_path / 'config.json'
+    config_file.write_text('{}')
+    secrets_file = tmp_path / 'config_secrets.json'
+    config_manager = ConfigManager(config_path=str(config_file),
+                                   secrets_path=str(secrets_file))
+    config_manager.template_path = str(tmp_path / 'no-template.json')
+    api_v3_module.api_v3.config_manager = config_manager
+
+    response = api_v3_client.post('/api/v3/config/main', json={
+        PLUGIN_ID: {'symbols': 'AAPL', 'api_key': 's3cret-key'},
+    })
+
+    assert response.status_code == 200, response.get_json()
+    assert 's3cret' not in config_file.read_text()
+    assert json.loads(secrets_file.read_text())[PLUGIN_ID]['api_key'] == 's3cret-key'
+    assert json.loads(config_file.read_text())[PLUGIN_ID]['symbols'] == 'AAPL'

web_interface/blueprints/api_v3/__init__.py

@@ -568,21 +568,41 @@ def _installed_plugin_ids():
     enumerate the installed plugins and read each one's persisted summary by ID
     instead of relying on the tracker's in-memory `get_all_*` view.
     """
-    pm = api_v3.plugin_manager
-    manifests = getattr(pm, 'plugin_manifests', None)
-    if not manifests:
-        # Only pay for a discovery scan when we haven't discovered anything yet;
-        # subsequent polls reuse the already-populated manifest map.
-        try:
-            pm.discover_plugins()
-        except Exception:
-            logger.debug('discover_plugins failed while listing plugin ids', exc_info=True)
-        manifests = getattr(pm, 'plugin_manifests', None)
+    manifests = _discovered_plugin_manifests()
     try:
         return list(manifests.keys()) if manifests else []
     except Exception:
         logger.debug('listing plugin_manifests failed while building plugin ids', exc_info=True)
         return []
+def _discovered_plugin_manifests(plugin_id=None, rescan=False):
+    """The plugin manager's manifests, discovering plugins first if needed.
+
+    The web process discovers plugins lazily (see app.py): nothing scans at
+    startup, so plugin_manifests is empty until some endpoint calls
+    discover_plugins(). A route that looks a plugin up without coming through
+    here answers "not found" for every installed plugin until something else
+    has run -- after a web restart, POST /display/on-demand/start returned 404
+    for minutes on a real rig, and only API-only callers ever noticed.
+
+    Scans when nothing is discovered yet, when ``plugin_id`` is given and not
+    among the manifests (it may have been installed since the last scan), or
+    when ``rescan`` is set (for lookups that are not by id, such as a mode).
+    Otherwise the existing map is reused, so a steady stream of requests
+    for known plugins costs nothing.
+
+    Returns the manifest map, or {} when there is no plugin manager.
+    """
+    pm = api_v3.plugin_manager
+    if pm is None:
+        return {}
+    manifests = getattr(pm, 'plugin_manifests', None)
+    if not manifests or rescan or (plugin_id is not None and plugin_id not in manifests):
+        try:
+            pm.discover_plugins()
+        except Exception:
+            logger.warning('Plugin discovery failed', exc_info=True)
+        manifests = getattr(pm, 'plugin_manifests', None)
+    return manifests or {}
 def _do_transactional_uninstall(plugin_id, preserve_config):
     """Execute an uninstall with snapshot-based rollback.
 

web_interface/blueprints/api_v3/config.py

@@ -960,9 +960,14 @@ def save_main_config():
         # Any key that matches a plugin ID should be saved as plugin config
         # This includes proper secret field handling from schema
         plugin_keys_to_remove = []
+        # Discovered first: a plugin key not recognised here skips secret
+        # separation below and falls through to the generic merge, which wrote
+        # the plugin's API key into config.json in plain text whenever nothing
+        # had yet discovered plugins in this process (any save after a restart).
+        plugin_manifests = _pkg._discovered_plugin_manifests()
         for key in data:
             # Check if this key is a plugin ID
-            if api_v3.plugin_manager and key in api_v3.plugin_manager.plugin_manifests:
+            if api_v3.plugin_manager and key in plugin_manifests:
                 plugin_id = key
                 plugin_config = data[key]
 

web_interface/blueprints/api_v3/display.py

@@ -178,14 +178,20 @@ def start_on_demand_display():
         resolved_mode = mode
 
         if api_v3.plugin_manager:
-            if resolved_plugin and resolved_plugin not in api_v3.plugin_manager.plugin_manifests:
+            if resolved_plugin and resolved_plugin not in _pkg._discovered_plugin_manifests(resolved_plugin):
                 return jsonify({'status': 'error', 'message': f'Plugin {resolved_plugin} not found'}), 404
 
             if resolved_plugin and not resolved_mode:
                 modes = api_v3.plugin_manager.get_plugin_display_modes(resolved_plugin)
                 resolved_mode = modes[0] if modes else resolved_plugin
             elif resolved_mode and not resolved_plugin:
+                _pkg._discovered_plugin_manifests()
                 resolved_plugin = api_v3.plugin_manager.find_plugin_for_mode(resolved_mode)
+                if not resolved_plugin:
+                    # Not among what was discovered: the plugin that declares
+                    # it may have been installed since. Scan once more.
+                    _pkg._discovered_plugin_manifests(rescan=True)
+                    resolved_plugin = api_v3.plugin_manager.find_plugin_for_mode(resolved_mode)
                 if not resolved_plugin:
                     return jsonify({'status': 'error', 'message': f'Mode {resolved_mode} not found'}), 404
 

web_interface/blueprints/api_v3/plugins.py

@@ -453,7 +453,7 @@ def toggle_plugin():
             return _toggle_starlark_app(plugin_id[len('starlark:'):], enabled)
 
         # Check if plugin exists in manifests (discovered but may not be loaded)
-        if plugin_id not in api_v3.plugin_manager.plugin_manifests:
+        if plugin_id not in _pkg._discovered_plugin_manifests(plugin_id):
             return jsonify({'status': 'error', 'message': 'Plugin not found'}), 404
 
         # Update config (this is what the display controller reads)