feat(display-controller): hot-reload plugin enable/disable without a restart (#374)

* feat(display-controller): hot-reload plugin enable/disable without a restart

Enabling or disabling a plugin in config previously required restarting the
display service: the plugin list and available_modes were built once at init
and the run loop never revisited them. (Per-plugin config *values* already
hot-reloaded; only the enabled set was restart-only.)

Now the controller reconciles its running plugins against the config's enabled
set whenever that set changes:

- The ConfigService watcher thread only sets a `_pending_plugin_reconcile`
  flag (via a cheap enabled-set diff). It never mutates loop state.
- The run loop applies the reconcile on its own thread (top of each
  iteration, deferred while on-demand is active), so loading/unloading and
  rebuilding available_modes can't race with rendering.
- `_reconcile_enabled_plugins` diffs desired vs running plugins, unloads the
  removed ones (cleanup + on_disable + config-unsubscribe via the new
  `_unregister_plugin`) and loads the added ones, then clamps the rotation
  index so the current mode stays valid.

The per-plugin registration done at startup is extracted into
`_register_loaded_plugin` and reused by the live-enable path so both build
identical state. Extracting it also fixes a latent late-binding bug: the
per-plugin config-change callbacks were closures over the loop variable, so
every plugin's callback targeted the last-loaded instance; each now binds its
own id/instance.

Adds test/test_display_controller_plugin_toggle.py covering live enable,
live disable, index clamping, no-op when unchanged, and the enabled-set diff.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(display-controller): don't exit on empty available_modes, guard rotation modulo

Hot-reload means available_modes can legitimately be empty at startup (no
plugins enabled yet) and become non-empty later via the web UI, or vice
versa mid-run. Fix four issues found reviewing this PR:

- run() exited the process entirely when available_modes was empty at
  startup instead of idling, permanently defeating the point of live
  enable/disable for anyone who starts with zero plugins enabled.
- The mode-rotation step divided by len(available_modes) unconditionally,
  raising ZeroDivisionError if the last enabled plugin is disabled between
  frames.
- _reconcile_enabled_plugins() called .get('enabled', False) on a config
  section without checking it was a dict first, raising AttributeError on
  a malformed config value.
- Minor: pop the config-change callback only after attempting to
  unsubscribe it, and log the exception in the config-read fallback
  instead of swallowing it silently.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ

* fix(display-controller): address review findings on the hot-reload PR

- Idle-wait tick was a fixed 30s sleep, delaying pickup of a plugin
  enabled via the web UI while no modes were active. Shortened to ~1s so
  it's roughly as responsive as the per-frame check once modes exist.
- _unregister_plugin popped the config-change callback from
  _plugin_config_callbacks even when config_service.unsubscribe() raised,
  losing the only reference to it. Now only pops on a successful
  unsubscribe.
- _pending_plugin_reconcile was cleared before _reconcile_enabled_plugins()
  ran, so a retryable failure (e.g. plugin discovery erroring) silently
  dropped the enable/disable request. _reconcile_enabled_plugins() now
  returns True/False and the caller only clears the flag on True.
- Added a warning log for the malformed-config case (a plugin's config
  section present but not a dict) so it's actually visible, and updated
  the existing test to assert it via caplog.

Left the broad `except Exception` around config_service.unsubscribe() as
Exception -- the current implementation is a simple lock+dict/list op that
doesn't document or realistically raise a narrower type, so this is a
defensive catch-all, not user error handling.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: ChuckBuilds <charlesmynard@gmail.com>

scripts/fix_perms/safe_pip_install.sh

@@ -0,0 +1,74 @@
+#!/bin/bash
+# safe_pip_install.sh — Install a requirements.txt as root after validating
+# that the resolved path is the project's own requirements.txt or a plugin's
+# requirements.txt under plugin-repos/ or plugins/.
+#
+# This script is intended to be called via sudo from the web interface, so
+# that packages a plugin declares end up visible to ledmatrix.service (which
+# runs as root) rather than only to whichever non-root user runs the web
+# interface. Plugin code already runs as root once loaded, so installing its
+# declared dependencies as root is not a new trust boundary.
+#
+# Usage: safe_pip_install.sh <requirements_txt_path>
+
+set -euo pipefail
+
+if [ $# -ne 1 ]; then
+    echo "Usage: $0 <requirements_txt_path>" >&2
+    exit 1
+fi
+
+TARGET="$1"
+
+# Determine the project root (parent of scripts/fix_perms/)
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+PROJECT_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)"
+
+# Allowed locations (resolved, no trailing slash):
+#   - the project's own requirements.txt
+#   - any requirements.txt under plugin-repos/ or plugins/
+ALLOWED_EXACT="$(realpath --canonicalize-missing "$PROJECT_ROOT/requirements.txt")"
+ALLOWED_BASES=(
+    "$(realpath --canonicalize-missing "$PROJECT_ROOT/plugin-repos")"
+    "$(realpath --canonicalize-missing "$PROJECT_ROOT/plugins")"
+)
+
+# Resolve the target path (follow symlinks); works even if it doesn't exist.
+RESOLVED_TARGET="$(realpath --canonicalize-missing "$TARGET")"
+
+# Must be named requirements.txt — never install from an arbitrary file.
+if [ "$(basename "$RESOLVED_TARGET")" != "requirements.txt" ]; then
+    echo "DENIED: $RESOLVED_TARGET is not a requirements.txt file" >&2
+    exit 2
+fi
+
+ALLOWED=false
+if [ "$RESOLVED_TARGET" = "$ALLOWED_EXACT" ]; then
+    ALLOWED=true
+else
+    for BASE in "${ALLOWED_BASES[@]}"; do
+        if [[ "$RESOLVED_TARGET" == "$BASE/"* ]]; then
+            ALLOWED=true
+            break
+        fi
+    done
+fi
+
+if [ "$ALLOWED" = false ]; then
+    echo "DENIED: $RESOLVED_TARGET is not an allowed requirements.txt location" >&2
+    echo "Allowed: $ALLOWED_EXACT, or any requirements.txt under: ${ALLOWED_BASES[*]}" >&2
+    exit 2
+fi
+
+if [ ! -f "$RESOLVED_TARGET" ]; then
+    echo "ERROR: $RESOLVED_TARGET does not exist" >&2
+    exit 3
+fi
+
+PYTHON_PATH="$(command -v python3)"
+# --ignore-installed: root's site-packages often has apt/dpkg-managed copies
+# of common libraries (requests, urllib3, ...) with no pip RECORD file, which
+# pip refuses to uninstall in place ("Cannot uninstall: no RECORD file was
+# found"). This tells pip to install the newer version alongside rather than
+# aborting the whole requirements.txt install over one such conflict.
+exec "$PYTHON_PATH" -m pip install --break-system-packages --ignore-installed -r "$RESOLVED_TARGET"

scripts/install/configure_web_sudo.sh

@@ -33,6 +33,7 @@ POWEROFF_PATH=$(command -v poweroff)  || true
 BASH_PATH=$(command -v bash)        || true
 JOURNALCTL_PATH=$(command -v journalctl) || true
 SAFE_RM_PATH="$PROJECT_ROOT/scripts/fix_perms/safe_plugin_rm.sh"
+SAFE_PIP_INSTALL_PATH="$PROJECT_ROOT/scripts/fix_perms/safe_pip_install.sh"
 
 # Validate required commands (systemctl, bash, python3 are essential)
 for CMD_NAME in SYSTEMCTL_PATH BASH_PATH PYTHON_PATH; do
@@ -48,11 +49,15 @@ if [ ${#MISSING_CMDS[@]} -gt 0 ]; then
     exit 1
 fi
 
-# Validate helper script exists
+# Validate helper scripts exist
 if [ ! -f "$SAFE_RM_PATH" ]; then
     echo "Error: Safe plugin removal helper not found: $SAFE_RM_PATH" >&2
     exit 1
 fi
+if [ ! -f "$SAFE_PIP_INSTALL_PATH" ]; then
+    echo "Error: Safe pip install helper not found: $SAFE_PIP_INSTALL_PATH" >&2
+    exit 1
+fi
 
 echo "Command paths:"
 echo "  Python: $PYTHON_PATH"
@@ -62,6 +67,7 @@ echo "  Poweroff: ${POWEROFF_PATH:-(not found, skipping)}"
 echo "  Bash: $BASH_PATH"
 echo "  Journalctl: ${JOURNALCTL_PATH:-(not found, skipping)}"
 echo "  Safe plugin rm: $SAFE_RM_PATH"
+echo "  Safe pip install: $SAFE_PIP_INSTALL_PATH"
 
 # Create a temporary sudoers file
 TEMP_SUDOERS="/tmp/ledmatrix_web_sudoers_$$"
@@ -101,13 +107,22 @@ TEMP_SUDOERS="/tmp/ledmatrix_web_sudoers_$$"
     fi
 
     # Required: python3, bash
-    echo "$WEB_USER ALL=(ALL) NOPASSWD: $PYTHON_PATH $PROJECT_DIR/display_controller.py"
-    echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_DIR/start_display.sh"
-    echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_DIR/stop_display.sh"
+    # NOTE: display_controller.py/start_display.sh/stop_display.sh live at the
+    # project root, not under scripts/install/ (where this script lives) —
+    # must use PROJECT_ROOT here, not PROJECT_DIR.
+    echo "$WEB_USER ALL=(ALL) NOPASSWD: $PYTHON_PATH $PROJECT_ROOT/display_controller.py"
+    echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT/start_display.sh"
+    echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT/stop_display.sh"
     echo ""
     echo "# Allow web user to remove plugin directories via vetted helper script"
     echo "# The helper validates that the target path resolves inside plugin-repos/ or plugins/"
     echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $SAFE_RM_PATH *"
+    echo ""
+    echo "# Allow web user to install a plugin's requirements.txt as root via vetted"
+    echo "# helper script, so packages are visible to root-run ledmatrix.service"
+    echo "# (not just the web interface's own user). The helper validates the target"
+    echo "# is requirements.txt at the project root or under plugin-repos/ or plugins/."
+    echo "$WEB_USER ALL=(ALL) NOPASSWD: $BASH_PATH $SAFE_PIP_INSTALL_PATH *"
 } > "$TEMP_SUDOERS"
 
 echo ""
@@ -126,6 +141,7 @@ echo "- Run display_controller.py directly"
 echo "- Execute start_display.sh and stop_display.sh"
 echo "- Reboot and shutdown the system"
 echo "- Remove plugin directories (for update/uninstall when root-owned files block deletion)"
+echo "- Install plugin/base requirements.txt as root (so ledmatrix.service can see them)"
 echo ""
 
 # Ask for confirmation
@@ -147,6 +163,13 @@ fi
 if ! sudo chmod 755 "$SAFE_RM_PATH"; then
     echo "Warning: Could not set permissions on $SAFE_RM_PATH"
 fi
+echo "Hardening safe_pip_install.sh ownership..."
+if ! sudo chown root:root "$SAFE_PIP_INSTALL_PATH"; then
+    echo "Warning: Could not set ownership on $SAFE_PIP_INSTALL_PATH"
+fi
+if ! sudo chmod 755 "$SAFE_PIP_INSTALL_PATH"; then
+    echo "Warning: Could not set permissions on $SAFE_PIP_INSTALL_PATH"
+fi
 
 if sudo cp "$TEMP_SUDOERS" /etc/sudoers.d/ledmatrix_web; then
     echo "Configuration applied successfully!"
@@ -160,7 +183,7 @@ if sudo cp "$TEMP_SUDOERS" /etc/sudoers.d/ledmatrix_web; then
         echo "✗ systemctl status ledmatrix.service - Failed"
     fi
     
-    if sudo -n test -f "$PROJECT_DIR/start_display.sh"; then
+    if sudo -n test -f "$PROJECT_ROOT/start_display.sh"; then
         echo "✓ File access test - OK"
     else
         echo "✗ File access test - Failed"

src/display_controller.py

@@ -24,7 +24,7 @@ import time
 import os
 import json
 from pathlib import Path
-from typing import Dict, Any, List, Optional
+from typing import Dict, Any, List, Optional, Callable
 from datetime import datetime
 from concurrent.futures import ThreadPoolExecutor, as_completed  # pylint: disable=no-name-in-module
 import pytz
@@ -163,6 +163,13 @@ class DisplayController:
         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]] = {}
+        # Per-plugin config-change callbacks, kept so we can unsubscribe a
+        # plugin when it is disabled live.
+        self._plugin_config_callbacks: Dict[str, Callable] = {}
+        # Set by the config-watcher thread when the enabled-plugin set changes;
+        # the main run loop reconciles (loads/unloads) on its own thread so
+        # mutating available_modes never races with rendering.
+        self._pending_plugin_reconcile = False
         self.on_demand_active = False
         self.on_demand_mode: Optional[str] = None
         self.on_demand_modes: List[str] = []  # All modes for the on-demand plugin
@@ -331,47 +338,10 @@ class DisplayController:
                         logger.info("✓ Loaded plugin %s in %.3f seconds (%d/%d)", 
                                   plugin_id, result['load_time'], loaded_count, enabled_count)
                         
-                        # Get plugin instance and manifest
-                        plugin_instance = self.plugin_manager.get_plugin(plugin_id)
-                        manifest = self.plugin_manager.plugin_manifests.get(plugin_id, {})
-                        
-                        # Prefer plugin's modes attribute if available (dynamic based on enabled leagues)
-                        # Fall back to manifest display_modes if plugin doesn't provide modes
-                        if plugin_instance and hasattr(plugin_instance, 'modes') and plugin_instance.modes:
-                            display_modes = list(plugin_instance.modes)
-                            logger.debug("Using plugin.modes for %s: %s", plugin_id, display_modes)
-                        else:
-                            display_modes = manifest.get('display_modes', [plugin_id])
-                            logger.debug("Using manifest display_modes for %s: %s", plugin_id, display_modes)
-                        
-                        if isinstance(display_modes, list) and display_modes:
-                            self.plugin_display_modes[plugin_id] = list(display_modes)
-                        else:
-                            display_modes = [plugin_id]
-                            self.plugin_display_modes[plugin_id] = list(display_modes)
-                        
-                        # Subscribe plugin to config changes for hot-reload
-                        if hasattr(self, 'config_service') and hasattr(plugin_instance, 'on_config_change'):
-                            def config_change_callback(old_config: Dict[str, Any], new_config: Dict[str, Any]) -> None:
-                                """Callback for plugin config changes."""
-                                try:
-                                    plugin_instance.on_config_change(new_config)
-                                    logger.debug("Plugin %s notified of config change", plugin_id)
-                                except Exception as e:
-                                    logger.error("Error in plugin %s config change handler: %s", plugin_id, e, exc_info=True)
-                            
-                            self.config_service.subscribe(config_change_callback, plugin_id=plugin_id)
-                            logger.debug("Subscribed plugin %s to config changes", plugin_id)
-                        
-                        # Add plugin modes to available modes
-                        for mode in display_modes:
-                            self.available_modes.append(mode)
-                            self.plugin_modes[mode] = plugin_instance
-                            self.mode_to_plugin_id[mode] = plugin_id
-                            logger.debug("  Added mode: %s", mode)
-                        # Invalidate signature cache so the new instance is re-inspected
-                        self._plugin_accepts_display_mode.pop(plugin_id, None)
-                        
+                        # Register the loaded plugin's modes, config subscription
+                        # and dispatch maps (shared with live enable hot-reload).
+                        self._register_loaded_plugin(plugin_id)
+
                         # Show progress
                         progress_pct = int((loaded_count / enabled_count) * 100)
                         elapsed = time.time() - plugin_time
@@ -447,6 +417,10 @@ class DisplayController:
         # when the user saves settings via the web UI.
         def _controller_config_change(old_config: Dict[str, Any], new_config: Dict[str, Any]) -> None:
             self._refresh_config_cache(new_config)
+            # If a plugin was enabled/disabled, flag a reconcile for the main
+            # loop to apply (loading/unloading off the watcher thread is unsafe).
+            if self._enabled_set_changed(old_config, new_config):
+                self._pending_plugin_reconcile = True
 
         self.config_service.subscribe(_controller_config_change)
 
@@ -1571,10 +1545,11 @@ class DisplayController:
     def run(self):
         """Run the display controller, switching between displays."""
         if not self.available_modes:
-            logger.warning("No display modes are enabled. Exiting.")
-            self.display_manager.cleanup()
-            return
-             
+            logger.warning(
+                "No display modes are enabled at startup; idling until a "
+                "plugin is enabled via the web UI."
+            )
+
         try:
             # Initialize with cached data for fast startup - let background updates refresh naturally
             logger.info("Starting display with cached data (fast startup mode)")
@@ -1582,6 +1557,25 @@ class DisplayController:
             logger.info(f"Initial mode set to: {self.current_display_mode} (index: {self.current_mode_index}, total modes: {len(self.available_modes)})")
             
             while True:
+                # Apply plugin enable/disable edits saved via the web UI. The
+                # config-watcher thread only sets the flag; loading/unloading and
+                # rebuilding available_modes happens here on the render thread so
+                # it can't race with rendering. Deferred while on-demand is active
+                # (the flag stays set) so we don't fight its temporary-enable.
+                if self._pending_plugin_reconcile and not self.on_demand_active:
+                    # Only clear the flag on success -- a retryable failure
+                    # (e.g. discovery) leaves it set so the request isn't lost.
+                    if self._reconcile_enabled_plugins():
+                        self._pending_plugin_reconcile = False
+
+                if not self.available_modes:
+                    # Nothing to render yet. Re-check _pending_plugin_reconcile
+                    # every ~1s (rather than a long sleep) so enabling a plugin
+                    # via the web UI is picked up about as promptly as it would
+                    # be once modes exist and the loop is iterating per-frame.
+                    self._sleep_with_plugin_updates(1)
+                    continue
+
                 # Handle on-demand commands before rendering
                 self._poll_on_demand_requests()
                 self._check_on_demand_expiration()
@@ -2319,7 +2313,7 @@ class DisplayController:
                         except Exception as e:
                             logger.warning("Error checking live priority for %s: %s", active_mode, e)
                 
-                if should_rotate:
+                if should_rotate and self.available_modes:
                     self.current_mode_index = (self.current_mode_index + 1) % len(self.available_modes)
                     self.current_display_mode = self.available_modes[self.current_mode_index]
                     self.last_mode_change = time.time()
@@ -2507,6 +2501,176 @@ class DisplayController:
             self.wifi_status_active = False
             self.wifi_status_expires_at = None
 
+    def _register_loaded_plugin(self, plugin_id: str) -> List[str]:
+        """Register an already-loaded plugin's display modes, config-change
+        subscription and dispatch maps with the controller.
+
+        Shared by startup loading and live enable hot-reload so both paths
+        build identical controller state. Returns the registered modes.
+        """
+        plugin_instance = self.plugin_manager.get_plugin(plugin_id)
+        manifest = self.plugin_manager.plugin_manifests.get(plugin_id, {})
+
+        # Prefer the plugin's dynamic modes attribute (e.g. based on enabled
+        # leagues), else fall back to manifest display_modes, else the id.
+        if plugin_instance is not None and getattr(plugin_instance, 'modes', None):
+            display_modes = list(plugin_instance.modes)
+            logger.debug("Using plugin.modes for %s: %s", plugin_id, display_modes)
+        else:
+            display_modes = manifest.get('display_modes', [plugin_id])
+            logger.debug("Using manifest display_modes for %s: %s", plugin_id, display_modes)
+        if not (isinstance(display_modes, list) and display_modes):
+            display_modes = [plugin_id]
+        self.plugin_display_modes[plugin_id] = list(display_modes)
+
+        # Subscribe to config changes for per-plugin hot-reload. Bind plugin_id
+        # and instance as defaults so each plugin's callback targets its own
+        # instance (avoids late-binding when registering many plugins), and
+        # remember the callback so we can unsubscribe on disable.
+        if hasattr(self, 'config_service') and hasattr(plugin_instance, 'on_config_change'):
+            def config_change_callback(old_config: Dict[str, Any], new_config: Dict[str, Any],
+                                       _pid: str = plugin_id, _plugin: Any = plugin_instance) -> None:
+                """Callback for plugin config changes."""
+                try:
+                    _plugin.on_config_change(new_config)
+                    logger.debug("Plugin %s notified of config change", _pid)
+                except Exception as e:
+                    logger.error("Error in plugin %s config change handler: %s", _pid, e, exc_info=True)
+
+            self.config_service.subscribe(config_change_callback, plugin_id=plugin_id)
+            self._plugin_config_callbacks[plugin_id] = config_change_callback
+            logger.debug("Subscribed plugin %s to config changes", plugin_id)
+
+        # Add modes to the dispatch maps.
+        for mode in display_modes:
+            if mode not in self.available_modes:
+                self.available_modes.append(mode)
+            self.plugin_modes[mode] = plugin_instance
+            self.mode_to_plugin_id[mode] = plugin_id
+            logger.debug("  Added mode: %s", mode)
+        # Invalidate signature cache so the new instance is re-inspected.
+        self._plugin_accepts_display_mode.pop(plugin_id, None)
+        return display_modes
+
+    def _unregister_plugin(self, plugin_id: str) -> None:
+        """Remove a plugin's modes, config subscription and instance, then
+        unload it. Used by live disable hot-reload."""
+        modes = self.plugin_display_modes.pop(plugin_id, [])
+        for mode in modes:
+            if mode in self.available_modes:
+                self.available_modes.remove(mode)
+            self.plugin_modes.pop(mode, None)
+            self.mode_to_plugin_id.pop(mode, None)
+
+        # Unsubscribe the plugin's config-change callback. Pop only on a
+        # successful unsubscribe -- if it raises, keep our reference so a
+        # later retry (or at least cleanup) still has the real callback
+        # instead of a lost one.
+        callback = self._plugin_config_callbacks.get(plugin_id)
+        if callback is not None and hasattr(self, 'config_service'):
+            try:
+                self.config_service.unsubscribe(callback, plugin_id=plugin_id)
+            except Exception as e:
+                logger.debug("Error unsubscribing plugin %s from config changes: %s", plugin_id, e)
+            else:
+                self._plugin_config_callbacks.pop(plugin_id, None)
+        else:
+            self._plugin_config_callbacks.pop(plugin_id, None)
+
+        self._plugin_accepts_display_mode.pop(plugin_id, None)
+
+        # Tear down the instance (cleanup + on_disable + module unload).
+        try:
+            self.plugin_manager.unload_plugin(plugin_id)
+        except Exception as e:
+            logger.error("Error unloading plugin %s: %s", plugin_id, e, exc_info=True)
+
+        logger.info("Disabled plugin %s live (removed modes: %s)", plugin_id, modes)
+
+    def _enabled_set_changed(self, old_config: Dict[str, Any], new_config: Dict[str, Any]) -> bool:
+        """True if any top-level section's ``enabled`` flag differs between two
+        configs. A cheap watcher-thread check that gates the full reconcile.
+        Non-plugin sections (e.g. schedule) may match too; the reconcile
+        no-ops for anything that isn't a discovered plugin."""
+        def enabled_map(cfg: Dict[str, Any]) -> Dict[str, bool]:
+            return {
+                key: bool(value.get('enabled', False))
+                for key, value in cfg.items()
+                if isinstance(value, dict)
+            }
+        return enabled_map(old_config) != enabled_map(new_config)
+
+    def _reconcile_enabled_plugins(self) -> bool:
+        """Load/unload plugins so the running set matches the enabled set in
+        config. Runs on the main display thread (never the config-watcher
+        thread) so mutating available_modes is race-free against rendering.
+
+        Returns True if reconciliation completed (including a no-op), or
+        False on a retryable failure -- the caller keeps the pending-reconcile
+        flag set in that case so the request isn't silently dropped."""
+        if self.plugin_manager is None:
+            return True
+        try:
+            config = self.config_service.get_config()
+        except Exception as e:
+            logger.warning("Plugin reconcile: falling back to cached config: %s", e)
+            config = self.config
+        try:
+            discovered = set(self.plugin_manager.discover_plugins())
+        except Exception as e:
+            logger.error("Plugin reconcile: discovery failed: %s", e, exc_info=True)
+            return False
+
+        for p in discovered:
+            if p in config and not isinstance(config.get(p), dict):
+                logger.warning(
+                    "Plugin reconcile: config for %s is a %s, not a dict; treating as disabled",
+                    p, type(config.get(p)).__name__
+                )
+
+        desired = {
+            p for p in discovered
+            if isinstance(config.get(p), dict) and config.get(p, {}).get('enabled', False)
+        }
+        current = set(self.plugin_display_modes.keys())
+        to_add = desired - current
+        to_remove = current - desired
+        if not to_add and not to_remove:
+            return True
+
+        previous_mode = self.current_display_mode
+
+        for plugin_id in to_remove:
+            self._unregister_plugin(plugin_id)
+
+        for plugin_id in to_add:
+            try:
+                if self.plugin_manager.load_plugin(plugin_id):
+                    modes = self._register_loaded_plugin(plugin_id)
+                    logger.info("Enabled plugin %s live (modes: %s)", plugin_id, modes)
+                else:
+                    logger.warning("Plugin reconcile: failed to load %s", plugin_id)
+            except Exception as e:
+                logger.error("Plugin reconcile: error enabling %s: %s", plugin_id, e, exc_info=True)
+
+        self._resync_mode_index_after_change(previous_mode)
+        logger.info("Plugin reconcile complete: +%s -%s (%d modes)",
+                    sorted(to_add), sorted(to_remove), len(self.available_modes))
+        return True
+
+    def _resync_mode_index_after_change(self, previous_mode: Optional[str]) -> None:
+        """Clamp rotation state after available_modes changed. Stays on the
+        previous mode if it survived, otherwise restarts cleanly within range."""
+        if not self.available_modes:
+            self.current_mode_index = 0
+            self.current_display_mode = None
+            return
+        if previous_mode in self.available_modes:
+            self.current_mode_index = self.available_modes.index(previous_mode)
+        else:
+            self.current_mode_index %= len(self.available_modes)
+            self.current_display_mode = self.available_modes[self.current_mode_index]
+
     def _refresh_config_cache(self, new_config: Dict[str, Any]) -> None:
         """Refresh all config-derived caches when a hot-reload fires.
 

test/test_display_controller_plugin_toggle.py

@@ -0,0 +1,255 @@
+"""Tests for live plugin enable/disable hot-reload in DisplayController.
+
+Enabling or disabling a plugin in config used to require a full display
+restart because the plugin list and available_modes were built once at init.
+These tests cover the reconcile path that loads/unloads plugins and rebuilds
+the dispatch maps on the main thread when the enabled set changes.
+"""
+
+from unittest.mock import MagicMock
+
+
+def _make_plugin(modes):
+    plugin = MagicMock()
+    plugin.modes = list(modes)
+    return plugin
+
+
+def _wire_plugin_manager(controller, plugins, discovered=None):
+    """Point the controller's mock plugin_manager at a set of fake plugins.
+
+    `plugins` maps plugin_id -> mock instance (with a .modes list).
+    """
+    pm = controller.plugin_manager
+    pm.discover_plugins.return_value = list(discovered if discovered is not None else plugins.keys())
+    pm.load_plugin.return_value = True
+    pm.unload_plugin.return_value = True
+    pm.plugin_manifests = {}
+    pm.get_plugin.side_effect = lambda pid: plugins.get(pid)
+    return pm
+
+
+def _set_config(controller, cfg):
+    controller.config_service.get_config = lambda: cfg
+
+
+class TestPluginEnableDisableHotReload:
+    def test_enable_plugin_live(self, test_display_controller):
+        controller = test_display_controller
+        assert controller.available_modes == []
+
+        plugin = _make_plugin(["foo"])
+        _wire_plugin_manager(controller, {"foo": plugin})
+        _set_config(controller, {"foo": {"enabled": True}})
+
+        controller._reconcile_enabled_plugins()
+
+        assert "foo" in controller.plugin_display_modes
+        assert "foo" in controller.available_modes
+        assert controller.plugin_modes["foo"] is plugin
+        assert controller.mode_to_plugin_id["foo"] == "foo"
+        controller.plugin_manager.load_plugin.assert_any_call("foo")
+
+    def test_disable_plugin_live(self, test_display_controller):
+        controller = test_display_controller
+        plugin = _make_plugin(["live", "recent"])
+        _wire_plugin_manager(controller, {"sports": plugin}, discovered=["sports"])
+
+        # Enable, then disable.
+        _set_config(controller, {"sports": {"enabled": True}})
+        controller._reconcile_enabled_plugins()
+        assert "sports" in controller.plugin_display_modes
+        assert "live" in controller.available_modes and "recent" in controller.available_modes
+        assert "sports" in controller._plugin_config_callbacks
+
+        _set_config(controller, {"sports": {"enabled": False}})
+        controller._reconcile_enabled_plugins()
+
+        assert "sports" not in controller.plugin_display_modes
+        assert "live" not in controller.available_modes
+        assert "recent" not in controller.available_modes
+        assert "live" not in controller.plugin_modes
+        assert "recent" not in controller.mode_to_plugin_id
+        controller.plugin_manager.unload_plugin.assert_any_call("sports")
+        assert "sports" not in controller._plugin_config_callbacks
+
+    def test_disable_clamps_current_mode_index(self, test_display_controller):
+        controller = test_display_controller
+        p1 = _make_plugin(["a"])
+        p2 = _make_plugin(["b"])
+        _wire_plugin_manager(controller, {"p1": p1, "p2": p2}, discovered=["p1", "p2"])
+
+        _set_config(controller, {"p1": {"enabled": True}, "p2": {"enabled": True}})
+        controller._reconcile_enabled_plugins()
+        # Add order across multiple plugins is set-driven (as at init), so
+        # compare membership, not order.
+        assert set(controller.available_modes) == {"a", "b"}
+
+        # Pretend we're currently showing p2's mode.
+        controller.current_mode_index = controller.available_modes.index("b")
+        controller.current_display_mode = "b"
+
+        _set_config(controller, {"p1": {"enabled": True}, "p2": {"enabled": False}})
+        controller._reconcile_enabled_plugins()
+
+        assert controller.available_modes == ["a"]
+        # Index must be back in range and the display mode no longer the removed one.
+        assert 0 <= controller.current_mode_index < len(controller.available_modes)
+        assert controller.current_display_mode == "a"
+
+    def test_enable_keeps_current_mode(self, test_display_controller):
+        controller = test_display_controller
+        p1 = _make_plugin(["a"])
+        p2 = _make_plugin(["b"])
+        _wire_plugin_manager(controller, {"p1": p1, "p2": p2}, discovered=["p1", "p2"])
+
+        _set_config(controller, {"p1": {"enabled": True}})
+        controller._reconcile_enabled_plugins()
+        controller.current_mode_index = 0
+        controller.current_display_mode = "a"
+
+        # Enabling p2 should not disturb the currently-showing mode.
+        _set_config(controller, {"p1": {"enabled": True}, "p2": {"enabled": True}})
+        controller._reconcile_enabled_plugins()
+
+        assert "b" in controller.available_modes
+        assert controller.current_display_mode == "a"
+        assert controller.available_modes[controller.current_mode_index] == "a"
+
+    def test_noop_when_enabled_set_unchanged(self, test_display_controller):
+        controller = test_display_controller
+        plugin = _make_plugin(["foo"])
+        _wire_plugin_manager(controller, {"foo": plugin}, discovered=["foo"])
+        _set_config(controller, {"foo": {"enabled": True}})
+        controller._reconcile_enabled_plugins()
+
+        load_calls = controller.plugin_manager.load_plugin.call_count
+        unload_calls = controller.plugin_manager.unload_plugin.call_count
+
+        # Reconcile again with no change — must not load/unload anything.
+        controller._reconcile_enabled_plugins()
+        assert controller.plugin_manager.load_plugin.call_count == load_calls
+        assert controller.plugin_manager.unload_plugin.call_count == unload_calls
+
+    def test_reconcile_ignores_non_dict_config_value(self, test_display_controller, caplog):
+        """A malformed config value (e.g. a stray string where a plugin's
+        section should be a dict) must be treated as disabled, not crash
+        the reconcile with AttributeError, and should be logged so it's
+        visible to whoever has to debug the malformed config."""
+        controller = test_display_controller
+        plugin = _make_plugin(["foo"])
+        _wire_plugin_manager(controller, {"foo": plugin}, discovered=["foo"])
+        _set_config(controller, {"foo": "not-a-dict"})
+
+        with caplog.at_level("WARNING"):
+            controller._reconcile_enabled_plugins()  # must not raise
+
+        assert "foo" not in controller.plugin_display_modes
+        assert "foo" not in controller.available_modes
+        assert any("foo" in r.message and "not a dict" in r.message for r in caplog.records)
+
+    def test_disable_keeps_callback_when_unsubscribe_fails(self, test_display_controller):
+        """If config_service.unsubscribe() raises, _unregister_plugin must
+        keep the callback in _plugin_config_callbacks rather than losing the
+        only reference to it (it still tears down the plugin itself)."""
+        controller = test_display_controller
+        plugin = _make_plugin(["live"])
+        _wire_plugin_manager(controller, {"sports": plugin}, discovered=["sports"])
+
+        _set_config(controller, {"sports": {"enabled": True}})
+        controller._reconcile_enabled_plugins()
+        assert "sports" in controller._plugin_config_callbacks
+
+        controller.config_service.unsubscribe = MagicMock(side_effect=RuntimeError("boom"))
+
+        _set_config(controller, {"sports": {"enabled": False}})
+        controller._reconcile_enabled_plugins()
+
+        assert "sports" not in controller.plugin_display_modes
+        assert "sports" in controller._plugin_config_callbacks
+
+
+class TestReconcileReturnValue:
+    """_reconcile_enabled_plugins() returns True/False so the caller (run()'s
+    loop) only clears _pending_plugin_reconcile on success, keeping a
+    retryable failure's request alive instead of silently dropping it."""
+
+    def test_returns_true_on_success(self, test_display_controller):
+        controller = test_display_controller
+        plugin = _make_plugin(["foo"])
+        _wire_plugin_manager(controller, {"foo": plugin}, discovered=["foo"])
+        _set_config(controller, {"foo": {"enabled": True}})
+        assert controller._reconcile_enabled_plugins() is True
+
+    def test_returns_true_for_noop(self, test_display_controller):
+        controller = test_display_controller
+        _wire_plugin_manager(controller, {}, discovered=[])
+        _set_config(controller, {})
+        assert controller._reconcile_enabled_plugins() is True
+
+    def test_returns_false_on_discovery_failure(self, test_display_controller):
+        controller = test_display_controller
+        controller.plugin_manager.discover_plugins.side_effect = RuntimeError("boom")
+        _set_config(controller, {})
+        assert controller._reconcile_enabled_plugins() is False
+
+    def test_returns_true_when_no_plugin_manager(self, test_display_controller):
+        controller = test_display_controller
+        controller.plugin_manager = None
+        assert controller._reconcile_enabled_plugins() is True
+
+
+class TestRunWithNoModesEnabled:
+    """Before hot-reload, an empty available_modes at startup was permanent
+    -- the display never came back without a restart. Now that a plugin can
+    be enabled live from the web UI, run() must idle rather than exit."""
+
+    def test_idles_instead_of_exiting(self, test_display_controller):
+        controller = test_display_controller
+        assert controller.available_modes == []
+
+        sleep_calls = []
+
+        def fake_sleep(duration, tick_interval=1.0):
+            sleep_calls.append(duration)
+            if len(sleep_calls) >= 3:
+                # Stand in for the process being torn down; run() catches
+                # this via its broad except + finally, same as any other
+                # unexpected error during the loop.
+                raise RuntimeError("stop-test-loop")
+
+        controller._sleep_with_plugin_updates = fake_sleep
+
+        controller.run()
+
+        # Old behavior returned before ever reaching the loop body, so
+        # _sleep_with_plugin_updates would never have been called. The idle
+        # tick is short (not a long sleep) so a plugin enabled via the web
+        # UI while idle is picked up about as promptly as it would be once
+        # modes exist and the loop is iterating per-frame.
+        assert sleep_calls == [1, 1, 1]
+
+
+class TestEnabledSetChanged:
+    def test_detects_toggle(self, test_display_controller):
+        c = test_display_controller
+        assert c._enabled_set_changed({"a": {"enabled": True}}, {"a": {"enabled": False}}) is True
+
+    def test_no_change(self, test_display_controller):
+        c = test_display_controller
+        cfg = {"a": {"enabled": True}, "b": {"enabled": False}}
+        assert c._enabled_set_changed(cfg, dict(cfg)) is False
+
+    def test_new_enabled_section(self, test_display_controller):
+        c = test_display_controller
+        assert c._enabled_set_changed(
+            {"a": {"enabled": True}},
+            {"a": {"enabled": True}, "b": {"enabled": True}},
+        ) is True
+
+    def test_ignores_non_enabled_value_edits(self, test_display_controller):
+        c = test_display_controller
+        assert c._enabled_set_changed(
+            {"a": {"enabled": True, "duration": 30}},
+            {"a": {"enabled": True, "duration": 45}},
+        ) is False

web_interface/blueprints/api_v3.py

@@ -43,6 +43,92 @@ def _truncate_output(stdout: str, stderr: str) -> str:
     return combined
 
 
+def _pip_install_requirements(req_file: Path, timeout: int) -> subprocess.CompletedProcess:
+    """Install a requirements.txt file, preferring the vetted sudo wrapper so
+    the packages are visible to root-run ledmatrix.service — not just to
+    whichever non-root user runs this web process. Falls back to installing
+    for the current process only if the wrapper isn't set up yet (i.e. the
+    admin hasn't run scripts/install/configure_web_sudo.sh since upgrading),
+    so the button still does *something* useful rather than hard-failing.
+    """
+    wrapper = PROJECT_ROOT / 'scripts' / 'fix_perms' / 'safe_pip_install.sh'
+    if wrapper.exists():
+        # Must invoke via an explicit `bash <path>` — matching both the
+        # sudoers rule configure_web_sudo.sh provisions ($BASH_PATH
+        # $SAFE_PIP_INSTALL_PATH *) and the existing safe_plugin_rm.sh call
+        # in src/common/permission_utils.py. Calling the script path directly
+        # (relying on its shebang) makes sudo check a different command line
+        # than what's actually allowlisted, so `sudo -n` denies it on any
+        # install that only has the specific rules this script provisions —
+        # it only appeared to work in prior testing because that device also
+        # had a broader, non-standard NOPASSWD: ALL grant.
+        #
+        # $BASH_PATH is resolved once at setup time (configure_web_sudo.sh's
+        # `command -v bash`) and baked into the static sudoers file as a
+        # literal path; sudo requires an exact string match against that, so
+        # if this process's own PATH resolves bash somewhere else, the
+        # sudoers rule won't match here either. Try the standard Debian/
+        # Raspberry Pi OS locations first, then this process's own
+        # resolution, so a divergence in just one of them doesn't break this.
+        bash_candidates = []
+        for candidate in ('/usr/bin/bash', '/bin/bash', shutil.which('bash')):
+            if candidate and candidate not in bash_candidates:
+                bash_candidates.append(candidate)
+
+        result = None
+        for bash_path in bash_candidates:
+            result = subprocess.run(
+                ['sudo', '-n', bash_path, str(wrapper), str(req_file)],
+                capture_output=True, text=True, timeout=timeout, cwd=str(PROJECT_ROOT)
+            )
+            if result.returncode == 0:
+                return result
+            # Best-effort distinction between "sudo rejected this exact
+            # command line" (no matching NOPASSWD rule for this bash path —
+            # worth trying the next candidate) and "sudo ran it but the
+            # wrapper/pip itself failed" (a real error — stop and surface it
+            # rather than uselessly retrying other bash paths or doubling up
+            # with a redundant non-root install attempt).
+            denied = any(
+                phrase in result.stderr
+                for phrase in ('a password is required', 'is not allowed to run', 'no tty present')
+            )
+            if not denied:
+                logger.warning(
+                    "[Pip Install] Root install failed (rc=%s) for %s: %s",
+                    result.returncode, req_file, result.stderr.strip()[:500],
+                )
+                return result
+
+        logger.warning(
+            "[Pip Install] Root wrapper denied via sudo for %s; falling back "
+            "to user-level install: %s",
+            req_file, result.stderr.strip()[:500] if result else 'no bash candidates found',
+        )
+        note = (
+            f"[Root install unavailable ({(result.stderr.strip() if result else 'sudo denied') or 'sudo denied'}); "
+            "installed for the web service's user only. Packages may not be "
+            "visible to ledmatrix.service if it runs as a different user — "
+            "run scripts/install/configure_web_sudo.sh to fix this.]\n"
+        )
+    else:
+        logger.warning(
+            "[Pip Install] safe_pip_install.sh not found; falling back to user-level install for %s",
+            req_file,
+        )
+        note = (
+            "[safe_pip_install.sh not found; installed for the web service's "
+            "user only. Run scripts/install/configure_web_sudo.sh to enable "
+            "root installs visible to ledmatrix.service.]\n"
+        )
+    result = subprocess.run(
+        [sys.executable, '-m', 'pip', 'install', '--break-system-packages', '-r', str(req_file)],
+        capture_output=True, text=True, timeout=timeout, cwd=str(PROJECT_ROOT)
+    )
+    result.stdout = note + (result.stdout or '')
+    return result
+
+
 def _scrub_git_remote_url(url: str) -> str:
     """Strip embedded username/password from an HTTPS remote URL before returning it to the UI."""
     try:
@@ -1671,10 +1757,7 @@ def execute_system_action():
             req_file = PROJECT_ROOT / 'requirements.txt'
             if not req_file.exists():
                 return jsonify({'status': 'error', 'message': 'No requirements.txt found at project root'})
-            result = subprocess.run(
-                [sys.executable, '-m', 'pip', 'install', '--break-system-packages', '-r', str(req_file)],
-                capture_output=True, text=True, timeout=120, cwd=str(PROJECT_ROOT)
-            )
+            result = _pip_install_requirements(req_file, timeout=120)
             return jsonify({
                 'status': 'success' if result.returncode == 0 else 'error',
                 'message': 'Base requirements installed successfully' if result.returncode == 0 else 'pip install failed',
@@ -1695,10 +1778,7 @@ def execute_system_action():
                     req = p / 'requirements.txt'
                     if p.is_dir() and req.exists():
                         try:
-                            r = subprocess.run(
-                                [sys.executable, '-m', 'pip', 'install', '--break-system-packages', '-r', str(req)],
-                                capture_output=True, text=True, timeout=60
-                            )
+                            r = _pip_install_requirements(req, timeout=60)
                             results.append({
                                 'plugin': p.name,
                                 'ok': r.returncode == 0,

web_interface/static/v3/js/widgets/array-table.js

@@ -174,11 +174,16 @@
         cell.style.verticalAlign = 'middle';
 
         if (colType === 'boolean') {
-            // Boolean: hidden sentinel + visible checkbox
+            // Boolean: hidden sentinel + visible checkbox, same `name` (so
+            // unchecked boxes still submit "false"). Keep the hidden's value
+            // synced to the checkbox at all times — some form-collection
+            // paths prefer whichever of the two same-named inputs comes
+            // first/last in the DOM, and a stale hidden value previously
+            // caused this field to silently revert to false on every save.
             const hidden = document.createElement('input');
             hidden.type  = 'hidden';
             hidden.name  = inputName;
-            hidden.value = 'false';
+            hidden.value = String(Boolean(colValue));
             cell.appendChild(hidden);
 
             const cb = document.createElement('input');
@@ -187,6 +192,9 @@
             cb.checked   = Boolean(colValue);
             cb.value     = 'true';
             cb.className = 'h-4 w-4 text-blue-600';
+            cb.addEventListener('change', () => {
+                hidden.value = String(cb.checked);
+            });
             cell.appendChild(cb);
 
         } else if (colType === 'integer' || colType === 'number') {

web_interface/templates/v3/partials/plugin_config.html

@@ -450,15 +450,16 @@
                                             </div>
                                         </td>
                                         <td class="px-4 py-3 whitespace-nowrap text-center">
-                                            <input type="hidden" name="{{ full_key }}.{{ item_index }}.enabled" value="false">
-                                            <input type="checkbox" 
+                                            <input type="hidden" name="{{ full_key }}.{{ item_index }}.enabled" value="{{ 'true' if item.get('enabled', true) else 'false' }}">
+                                            <input type="checkbox"
                                                    name="{{ full_key }}.{{ item_index }}.enabled"
                                                    {% if item.get('enabled', true) %}checked{% endif %}
                                                    value="true"
+                                                   onchange="this.previousElementSibling.value = this.checked ? 'true' : 'false'"
                                                    class="h-4 w-4 text-blue-600">
                                         </td>
                                         <td class="px-4 py-3 whitespace-nowrap text-center">
-                                            <button type="button" 
+                                            <button type="button"
                                                     onclick="removeCustomFeedRow(this)"
                                                     class="text-red-600 hover:text-red-800 px-2 py-1">
                                                 <i class="fas fa-trash"></i>
@@ -549,11 +550,18 @@
                                         {% else %}{% set td_min_w = '110px' %}{% endif %}
                                         <td class="px-3 py-3 whitespace-nowrap" style="min-width:{{ td_min_w }};vertical-align:middle">
                                             {% if col_type == 'boolean' %}
-                                                <input type="hidden" name="{{ full_key }}.{{ item_index }}.{{ col_name }}" value="false">
+                                                {# Hidden sentinel ensures unchecked boxes still submit "false" (browsers
+                                                   omit unchecked checkboxes entirely). It shares the checkbox's `name`,
+                                                   so it must always mirror the checkbox's actual state — both here at
+                                                   render time and on every toggle — or whichever of the two same-named
+                                                   inputs the save request happens to prefer can silently revert this
+                                                   field to false regardless of what's checked. #}
+                                                <input type="hidden" name="{{ full_key }}.{{ item_index }}.{{ col_name }}" value="{{ 'true' if col_value else 'false' }}">
                                                 <input type="checkbox"
                                                        name="{{ full_key }}.{{ item_index }}.{{ col_name }}"
                                                        {% if col_value %}checked{% endif %}
                                                        value="true"
+                                                       onchange="this.previousElementSibling.value = this.checked ? 'true' : 'false'"
                                                        class="h-4 w-4 text-blue-600">
                                             {% elif col_type == 'integer' or col_type == 'number' %}
                                                 <input type="number"