fix(display): retry a plugin that is enabled but failed to load

A plugin whose validate_config() returns False is treated as a hard load
failure. The API then reports enabled=true, loaded=false, error=null: the
plugin is simply absent, with nothing saying why. hockey-scoreboard sat in
that state on a live rig for four days.

The recovery path existed but could not be reached. _reconcile_enabled_plugins
computes to_add = desired - current, and a plugin that failed to load is never
in current, so it stays in to_add and would be retried. But the reconcile is
queued by _enabled_set_changed(), which compares only top-level `enabled`
flags -- and the edit that actually fixes such a plugin (enabling a league,
filling in an API key) is nested inside the plugin's own config section. No
top-level flag changes, so no reconcile is queued, and the save that should
have fixed it does nothing. Only toggling some unrelated plugin -- which does
change a top-level flag -- queues the global reconcile that recovers it.

Add a second gate: queue a reconcile when a discovered plugin is enabled in
config but absent from the running set.

It is deliberately narrow rather than "reconcile on any config change".
Reconcile calls discover_plugins(), a ~39-manifest filesystem scan, and it
runs on the render thread; doing that on every config save would trade this
bug for a frame hitch. Gating on plugin_manifests also keeps non-plugin
sections that carry their own `enabled` flag (schedule, display) from
queueing a reconcile they can never satisfy. In the steady state -- every
enabled plugin loaded -- the new check is False and costs nothing.

The same valid-but-unconfigured => hard-fail shape still exists in
text-display, youtube-stats, birdnet-go, ledmatrix-flights and
mqtt-notifications; this makes all of them recoverable without a restart.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
This commit is contained in:
ChuckBuilds
2026-08-22 08:09:11 -04:00
co-authored by Claude Opus 5
parent 6b74506695
commit 4143aa958c
2 changed files with 162 additions and 1 deletions
+37 -1
View File
@@ -463,7 +463,8 @@ class DisplayController:
self._refresh_config_cache(new_config) self._refresh_config_cache(new_config)
# If a plugin was enabled/disabled, flag a reconcile for the main # If a plugin was enabled/disabled, flag a reconcile for the main
# loop to apply (loading/unloading off the watcher thread is unsafe). # loop to apply (loading/unloading off the watcher thread is unsafe).
if self._enabled_set_changed(old_config, new_config): if (self._enabled_set_changed(old_config, new_config)
or self._enabled_plugin_not_running(new_config)):
self._pending_plugin_reconcile = True self._pending_plugin_reconcile = True
self.config_service.subscribe(_controller_config_change) self.config_service.subscribe(_controller_config_change)
@@ -2892,6 +2893,41 @@ class DisplayController:
} }
return enabled_map(old_config) != enabled_map(new_config) return enabled_map(old_config) != enabled_map(new_config)
def _enabled_plugin_not_running(self, new_config: Dict[str, Any]) -> bool:
"""True when a discovered plugin is enabled in config but not running.
``_enabled_set_changed`` compares only top-level ``enabled`` flags, which
misses the case that strands a plugin: one whose ``validate_config()``
returned False is absent from the running set, and the edit that fixes it
(enabling a league, filling in an API key) lives *nested* inside that
plugin's own section. No top-level flag changes, so no reconcile is
queued, and the save that should have fixed it appears to do nothing --
only toggling some unrelated plugin recovers it. hockey-scoreboard sat
enabled-but-absent on a live rig for four days this way.
Deliberately narrow: it fires only for ids the plugin manager has
actually discovered, so non-plugin sections that carry their own
``enabled`` flag (``schedule``, ``display``, ...) don't queue a reconcile
on every save. In the steady state -- everything enabled is loaded --
this is False and costs nothing. That matters because reconcile runs
``discover_plugins()`` on the render thread, where a needless
filesystem scan per config save would show up as a frame hitch.
"""
if self.plugin_manager is None:
return False
try:
known = set(self.plugin_manager.plugin_manifests)
running = set(self.plugin_display_modes)
except (RuntimeError, AttributeError):
# Mid-mutation on the render thread, or a manager without the
# attribute. Let reconcile decide -- it no-ops when nothing differs.
return True
for key, value in new_config.items():
if (key in known and isinstance(value, dict)
and value.get('enabled', False) and key not in running):
return True
return False
def _reconcile_enabled_plugins(self) -> bool: def _reconcile_enabled_plugins(self) -> bool:
"""Load/unload plugins so the running set matches the enabled set in """Load/unload plugins so the running set matches the enabled set in
config. Runs on the main display thread (never the config-watcher config. Runs on the main display thread (never the config-watcher
@@ -6,6 +6,7 @@ These tests cover the reconcile path that loads/unloads plugins and rebuilds
the dispatch maps on the main thread when the enabled set changes. the dispatch maps on the main thread when the enabled set changes.
""" """
import copy
from unittest.mock import MagicMock from unittest.mock import MagicMock
@@ -253,3 +254,127 @@ class TestEnabledSetChanged:
{"a": {"enabled": True, "duration": 30}}, {"a": {"enabled": True, "duration": 30}},
{"a": {"enabled": True, "duration": 45}}, {"a": {"enabled": True, "duration": 45}},
) is False ) is False
class TestEnabledPluginNotRunning:
"""A plugin that fails validate_config() is enabled but absent, and the
config edit that fixes it is nested inside the plugin's own section -- so
the top-level ``enabled`` comparison never sees it. These cover the second
gate that queues a reconcile in that case.
"""
def test_nested_edit_is_invisible_to_the_enabled_set_check(self, test_display_controller):
"""The original gate: proves why a second one is needed."""
controller = test_display_controller
old = {"hockey-scoreboard": {"enabled": True, "nhl": {"enabled": False}}}
new = {"hockey-scoreboard": {"enabled": True, "nhl": {"enabled": True}}}
# Enabling a league changes no top-level flag.
assert controller._enabled_set_changed(old, new) is False
def test_queues_reconcile_when_enabled_plugin_is_absent(self, test_display_controller):
controller = test_display_controller
controller.plugin_manager.plugin_manifests = {"hockey-scoreboard": {}}
controller.plugin_display_modes = {} # failed to load
cfg = {"hockey-scoreboard": {"enabled": True, "nhl": {"enabled": True}}}
assert controller._enabled_plugin_not_running(cfg) is True
def test_quiet_when_every_enabled_plugin_is_running(self, test_display_controller):
controller = test_display_controller
controller.plugin_manager.plugin_manifests = {"hockey-scoreboard": {}}
controller.plugin_display_modes = {"hockey-scoreboard": ["nhl"]}
cfg = {"hockey-scoreboard": {"enabled": True}}
assert controller._enabled_plugin_not_running(cfg) is False
def test_disabled_plugin_does_not_queue(self, test_display_controller):
controller = test_display_controller
controller.plugin_manager.plugin_manifests = {"hockey-scoreboard": {}}
controller.plugin_display_modes = {}
cfg = {"hockey-scoreboard": {"enabled": False}}
assert controller._enabled_plugin_not_running(cfg) is False
def test_non_plugin_sections_do_not_queue(self, test_display_controller):
"""``schedule``/``display`` carry their own ``enabled`` and are never
in plugin_display_modes -- without the manifest check they would queue
a reconcile, and therefore a filesystem scan, on every config save."""
controller = test_display_controller
controller.plugin_manager.plugin_manifests = {"hockey-scoreboard": {}}
controller.plugin_display_modes = {"hockey-scoreboard": ["nhl"]}
cfg = {
"hockey-scoreboard": {"enabled": True},
"schedule": {"enabled": True},
"display": {"enabled": True},
}
assert controller._enabled_plugin_not_running(cfg) is False
def test_non_dict_section_is_ignored(self, test_display_controller):
controller = test_display_controller
controller.plugin_manager.plugin_manifests = {"hockey-scoreboard": {}}
controller.plugin_display_modes = {}
assert controller._enabled_plugin_not_running({"hockey-scoreboard": "nonsense"}) is False
def test_no_plugin_manager_is_quiet(self, test_display_controller):
controller = test_display_controller
controller.plugin_manager = None
assert controller._enabled_plugin_not_running({"x": {"enabled": True}}) is False
class TestReconcileQueuedThroughSubscriber:
"""End-to-end through the real config-change subscriber, not the helper.
Without the second gate this is the four-day-outage path: the plugin is
enabled, absent, and the save that enables its league sets no flag.
"""
@staticmethod
def _subscriber(controller):
subs = controller.config_service._subscribers['*']
for cb in subs:
if getattr(cb, '__name__', '') == '_controller_config_change':
return cb
raise AssertionError(f"controller subscriber not found among {subs}")
@staticmethod
def _configs(controller, plugin_section_old, plugin_section_new):
"""Build two full configs differing only inside the plugin section --
the subscriber refreshes its cache from these, so they must be real."""
base = copy.deepcopy(controller.config)
old = copy.deepcopy(base)
new = copy.deepcopy(base)
old["hockey-scoreboard"] = plugin_section_old
new["hockey-scoreboard"] = plugin_section_new
return old, new
def test_nested_edit_queues_reconcile_for_absent_plugin(self, test_display_controller):
controller = test_display_controller
controller.plugin_manager.plugin_manifests = {"hockey-scoreboard": {}}
controller.plugin_display_modes = {} # validate_config() said False
controller._pending_plugin_reconcile = False
old, new = self._configs(
controller,
{"enabled": True, "nhl": {"enabled": False}},
{"enabled": True, "nhl": {"enabled": True}},
)
# The original gate is blind to this edit ...
assert controller._enabled_set_changed(old, new) is False
self._subscriber(controller)(old, new)
# ... but the reconcile is queued anyway.
assert controller._pending_plugin_reconcile is True
def test_steady_state_does_not_queue_reconcile(self, test_display_controller):
"""Everything enabled is running: an unrelated edit must not queue a
reconcile, or every config save drags a filesystem scan onto the
render thread."""
controller = test_display_controller
controller.plugin_manager.plugin_manifests = {"hockey-scoreboard": {}}
controller.plugin_display_modes = {"hockey-scoreboard": ["nhl"]}
controller._pending_plugin_reconcile = False
old, new = self._configs(
controller,
{"enabled": True, "scroll_speed": 1},
{"enabled": True, "scroll_speed": 2},
)
self._subscriber(controller)(old, new)
assert controller._pending_plugin_reconcile is False