Merge origin/main into claude/deprecate-unused-plugin-api

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

CHANGELOG.md

@@ -19,6 +19,13 @@ accepts both, but the store flags the old spelling as deprecated
 
 ## Unreleased
 
+- `FontManager.get_font()` returns a BDF font at its native size when asked for
+  a size the file doesn't contain (5x7.bdf at 8 or 10px, say). It used to
+  return PIL's default font, a different typeface, so a plugin that relied on
+  that will now render the font it asked for.
+- `src.wifi_manager.get_wifi_status_path()` — where WiFi status messages for
+  the display are written (`config/wifi_status.json`).
+
 Deprecated, removed in 3.7.0 (each logs a warning on first use; see
 `docs/PLUGIN_API_REFERENCE.md#deprecated-apis` for replacements). Nothing in
 core, the monorepo or the registry's third-party plugins calls them:
@@ -143,6 +150,19 @@ New modules a plugin may import via `src.*` (floor on 3.5.0):
   fetched, which keeps the peak memory of a four-capped-month fetch to about
   16 MB over the sequential path rather than 43 MB — `docs/LOW_MEMORY_BOARDS.md`
   puts a 1 GB Pi 3B+ at under 200 MB of headroom.
+- `ESPNDataSource.fetch_standings` asks each league the endpoint that league
+  actually publishes. It tried `/standings` first whatever the league and fell
+  back to `/rankings` only on a 404, but college leagues answer `/standings`
+  with a 200 that carries no poll, so the fallback never fired: the rank badge
+  simply never appeared and anything keyed off rankings quietly did nothing.
+  Endpoints are now ordered by whether the league publishes a poll, and a 200
+  that lacks the key counts as a miss, so a league answering both still ends up
+  with whichever carries the poll. Only a 404 is routine — that is how a league
+  says it has none; a connection error, a timeout or an unparseable body is
+  logged as an error again, and a bug raised while inspecting the payload is no
+  longer swallowed as a missing poll. This is the implementation the football,
+  baseball and hockey boards already ship; core was the last copy on the old
+  one.
 
 ### Scrolling
 
@@ -372,6 +392,19 @@ New modules a plugin may import via `src.*` (floor on 3.5.0):
   budget, so a rollback finishes inside the unit's 30-minute limit instead of
   being killed mid-way.
 
+### Installers
+
+- The generated `ledmatrix_web` sudoers rules are parsed before they are
+  installed. Both installers built the drop-in from `which` lookups and copied
+  it into `/etc/sudoers.d` without ever checking it, and a malformed file there
+  makes sudo refuse every command for every user — on a headless Pi, that is
+  unrecoverable over SSH. `first_time_install.sh` now runs `visudo -c` on the
+  generated file and, if it does not parse, prints what visudo said and leaves
+  the installed file untouched instead of replacing it with a broken one;
+  `configure_web_sudo.sh` does the same before offering the rules for
+  confirmation. `first_time_install.sh` also built that file at a fixed `/tmp`
+  path as root; `mktemp` now picks the name.
+
 ### Small fixes (update-all, plugin system settings, scripts)
 
 - **Check & Update All** counts a plugin that had nothing to update as

first_time_install.sh

@@ -1535,9 +1535,6 @@ $ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH is-active ledmatrix.service
 $ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH start ledmatrix-web.service
 $ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH stop ledmatrix-web.service
 $ACTUAL_USER ALL=(ALL) NOPASSWD: $SYSTEMCTL_PATH restart ledmatrix-web.service
-$ACTUAL_USER ALL=(ALL) NOPASSWD: $PYTHON_PATH $PROJECT_ROOT_DIR/display_controller.py
-$ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/start_display.sh
-$ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/stop_display.sh
 $ACTUAL_USER ALL=(ALL) NOPASSWD: $BASH_PATH $PROJECT_ROOT_DIR/scripts/fix_perms/safe_plugin_rm.sh *
 # Install a requirements.txt as root via vetted helper, so packages are visible
 # to root-run ledmatrix.service (not just the web interface's own user).

scripts/install/configure_web_sudo.sh

@@ -111,13 +111,6 @@ TEMP_SUDOERS="/tmp/ledmatrix_web_sudoers_$$"
         echo "$WEB_USER ALL=(ALL) NOPASSWD:NOEXEC: $JOURNALCTL_PATH -t ledmatrix *"
     fi
 
-    # Required: python3, bash
-    # 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/"
@@ -155,8 +148,6 @@ echo "- Start/stop/restart the ledmatrix service"
 echo "- Enable/disable the ledmatrix service"
 echo "- Check service status"
 echo "- View system logs via journalctl"
-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)"

scripts/install/one-shot-install.sh

@@ -65,15 +65,14 @@ retry() {
     local delay_seconds=5
     local status
     while true; do
-        # Run command in a context that disables errexit so we can capture exit code
-        # This prevents errexit from triggering before status=$? runs
-        if ! "$@"; then
-            status=$?
-        else
-            status=0
-        fi
-        if [ $status -eq 0 ]; then
+        # The condition of an if doesn't trip errexit, and in the else branch
+        # $? is the command's own exit status. (This used to be `if ! "$@";
+        # then status=$?`, where $? is the status of the negation -- always 0 --
+        # so a failure never retried and was reported as success.)
+        if "$@"; then
             return 0
+        else
+            status=$?
         fi
         if [ $attempt -ge $max_attempts ]; then
             print_error "Command failed after $attempt attempts: $*"
@@ -259,22 +258,32 @@ main() {
     
     # Update package list first. first_time_install.sh is told the lists are
     # already fresh so it does not repeat this a minute later.
+    # A refresh that still fails after retries (say one unreachable mirror)
+    # only warns: that is what this step effectively did before retry()
+    # could report a failure, and making it fatal would stop installs that
+    # work today.
     if [ "$EUID" -eq 0 ]; then
-        retry apt-get update -qq
+        retry apt-get update -qq || print_warning "apt-get update failed; continuing with the existing package lists"
     else
-        retry sudo apt-get update -qq
+        retry sudo apt-get update -qq || print_warning "apt-get update failed; continuing with the existing package lists"
     fi
     export LEDMATRIX_APT_UPDATED=1
     
     # Install git and curl (needed for cloning and the script itself)
     if ! command -v git >/dev/null 2>&1 || ! command -v curl >/dev/null 2>&1; then
         print_warning "git or curl not found, installing..."
+        # Not fatal here, for the same reason: without git the clone below
+        # fails and stops the install with its own error.
         if [ "$EUID" -eq 0 ]; then
-            retry apt-get install -y git curl
+            retry apt-get install -y git curl || true
         else
-            retry sudo apt-get install -y git curl
+            retry sudo apt-get install -y git curl || true
+        fi
+        if command -v git >/dev/null 2>&1 && command -v curl >/dev/null 2>&1; then
+            print_success "git and curl installed"
+        else
+            print_warning "Could not install git and curl"
         fi
-        print_success "git and curl installed"
     else
         print_success "git and curl already installed"
     fi

src/display_controller.py

@@ -26,7 +26,6 @@ import json
 import threading
 import types
 from contextlib import contextmanager
-from pathlib import Path
 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
@@ -63,9 +62,6 @@ _vegas_mode_imported = False
 VegasModeCoordinator = None
 DEFAULT_DYNAMIC_DURATION_CAP = 180.0
 
-# WiFi status message file path (same as used in wifi_manager.py)
-WIFI_STATUS_FILE = None  # Will be initialized in __init__
-
 class DisplayController:
     """
     Top-level controller that owns the LED display run loop.
@@ -231,13 +227,11 @@ class DisplayController:
         # once live priority ends.
         self._live_resume_index: Optional[int] = None
 
-        # WiFi status message tracking
-        global WIFI_STATUS_FILE
-        if WIFI_STATUS_FILE is None:
-            # Resolve project root (same logic as wifi_manager.py)
-            project_root = Path(__file__).parent.parent.parent.resolve()
-            WIFI_STATUS_FILE = project_root / "config" / "wifi_status.json"
-        self.wifi_status_file = WIFI_STATUS_FILE
+        # WiFi status message tracking. The path comes from wifi_manager so
+        # reader and writer can't drift: this used to resolve three levels up
+        # from src/, one above the repo, and never saw a message.
+        from src.wifi_manager import get_wifi_status_path
+        self.wifi_status_file = get_wifi_status_path()
         self.wifi_status_active = False
         self.wifi_status_expires_at: Optional[float] = None
         # _check_wifi_status_message throttle state (checked at frame rate,
@@ -428,9 +422,7 @@ class DisplayController:
         self._normal_brightness: int = (
             self.config.get('display', {}).get('hardware', {}).get('brightness', 90)
         )
-        self._scroll_speed: float = (
-            self.config.get('display', {}).get('vegas_scroll', {}).get('scroll_speed', 75)
-        )
+        self._scroll_speed: float = self._vegas_scroll_speed(self.config)
 
         # Brightness state tracking for dim schedule
         self.current_brightness = self._normal_brightness
@@ -450,16 +442,7 @@ class DisplayController:
         # Register controller-level hot-reload callback so cached config values
         # (_normal_brightness, _scroll_speed, _tz, minute-gates) stay in sync
         # 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)
-                    or self._enabled_plugin_not_running(new_config)):
-                with self._reconcile_flag_lock:
-                    self._pending_plugin_reconcile = True
-
-        self.config_service.subscribe(_controller_config_change)
+        self.config_service.subscribe(self._controller_config_change)
 
         # Publish initial on-demand state
         try:
@@ -581,6 +564,9 @@ class DisplayController:
         """Check if Vegas mode should be running."""
         if not self.vegas_coordinator:
             return False
+        # A stopped coordinator never reaches run_frame(), where queued config
+        # is applied, so re-enabling Vegas from the web UI would never land.
+        self.vegas_coordinator.apply_pending_config_if_idle()
         if not self.vegas_coordinator.is_enabled:
             return False
         if self.on_demand_active:
@@ -1068,16 +1054,22 @@ class DisplayController:
             self._tick_plugin_updates()
 
     def _get_display_duration(self, mode_key):
-        """Get display duration for a mode."""
-        # Check plugin-specific duration first
-        if mode_key in self.plugin_modes:
-            plugin_instance = self.plugin_modes[mode_key]
-            if hasattr(plugin_instance, 'get_display_duration'):
-                return plugin_instance.get_display_duration()
-        
-        # Fall back to config
-        display_durations = self.config.get('display', {}).get('display_durations', {})
-        return display_durations.get(mode_key, 30)
+        """Seconds to show a mode: the Rotation & Durations page's value for it
+        (display.display_durations), else the plugin's own duration.
+
+        The saved value has to win. Every plugin inherits
+        get_display_duration(), so checking the plugin first meant the page's
+        values were never read.
+        """
+        display_durations = self.config.get('display', {}).get('display_durations', {}) or {}
+        override = display_durations.get(mode_key)
+        if isinstance(override, (int, float)) and not isinstance(override, bool) and override > 0:
+            return float(override)
+
+        plugin_instance = self.plugin_modes.get(mode_key)
+        if plugin_instance is not None and hasattr(plugin_instance, 'get_display_duration'):
+            return plugin_instance.get_display_duration()
+        return 30
 
     def _get_global_dynamic_cap(self) -> Optional[float]:
         """Return global fallback dynamic duration cap."""
@@ -2012,8 +2004,9 @@ class DisplayController:
                     if wifi_status_data:
                         # Display WiFi status message and skip normal rotation
                         if self._display_wifi_status_message(wifi_status_data):
-                            # Sleep for a short time to show the message
-                            # Use a short sleep to allow for quick updates
+                            # The plugin that resumes afterwards must redraw
+                            # the whole panel, not paint over the message.
+                            self.force_change = True
                             self._sleep_with_plugin_updates(0.5)
                             continue  # Skip to next iteration, don't rotate
                         else:
@@ -3226,6 +3219,24 @@ class DisplayController:
             self.current_mode_index %= len(self.available_modes)
             self.current_display_mode = self.available_modes[self.current_mode_index]
 
+    def _controller_config_change(self, old_config: Dict[str, Any], new_config: Dict[str, Any]) -> None:
+        """ConfigService subscriber: runs on the config-watcher thread."""
+        self._refresh_config_cache(new_config)
+        # Vegas keeps its own parsed copy of display.vegas_scroll. Queue the
+        # new one only when it changed: applying it rebuilds the strip.
+        # (getattr: this can fire before __init__ creates the coordinator.)
+        vegas = getattr(self, 'vegas_coordinator', None)
+        if vegas is not None and (
+                (old_config.get('display', {}) or {}).get('vegas_scroll')
+                != (new_config.get('display', {}) or {}).get('vegas_scroll')):
+            vegas.update_config(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)
+                or self._enabled_plugin_not_running(new_config)):
+            with self._reconcile_flag_lock:
+                self._pending_plugin_reconcile = True
+
     def _refresh_config_cache(self, new_config: Dict[str, Any]) -> None:
         """Refresh all config-derived caches when a hot-reload fires.
 
@@ -3238,9 +3249,7 @@ class DisplayController:
         self._normal_brightness = (
             self.config.get('display', {}).get('hardware', {}).get('brightness', 90)
         )
-        self._scroll_speed = (
-            self.config.get('display', {}).get('vegas_scroll', {}).get('scroll_speed', 75)
-        )
+        self._scroll_speed = self._vegas_scroll_speed(self.config)
         # Force the timezone to be re-derived from the new config on next schedule check
         self._tz = None
         # Invalidate minute-gates so the new schedule/dim times take effect immediately
@@ -3250,6 +3259,14 @@ class DisplayController:
         logger.debug("Config cache refreshed (brightness=%s, scroll_speed=%s)",
                      self._normal_brightness, self._scroll_speed)
 
+    @staticmethod
+    def _vegas_scroll_speed(config: Dict[str, Any]) -> float:
+        """Vegas scroll speed in px/s. The default must match VegasModeConfig's
+        (50): a follower dead-reckons with this value between the leader's
+        position packets, so a different default made it run 50% fast."""
+        vegas_cfg = (config.get('display', {}) or {}).get('vegas_scroll', {}) or {}
+        return float(vegas_cfg.get('scroll_speed', 50.0))
+
     def cleanup(self):
         """Clean up resources."""
         # Stop the async update worker first so no in-flight update() call

src/error_aggregator.py

@@ -64,7 +64,7 @@ class ErrorPattern:
             "count": self.count,
             "first_seen": self.first_seen.isoformat(),
             "last_seen": self.last_seen.isoformat(),
-            "affected_plugins": list(set(self.affected_plugins)),
+            "affected_plugins": list(dict.fromkeys(self.affected_plugins)),
             "sample_messages": self.sample_messages[:3],  # Keep only 3 samples
             "severity": self.severity
         }
@@ -195,7 +195,9 @@ class ErrorAggregator:
                 severity = "warning"
 
             # Collect affected plugins
-            affected_plugins = [r.plugin_id for r in recent_same_type if r.plugin_id]
+            # Unique, in first-seen order. Each repeat re-scans the whole
+            # window, so merging duplicates in below grew without bound.
+            affected_plugins = list(dict.fromkeys(r.plugin_id for r in recent_same_type if r.plugin_id))
 
             # Collect sample messages
             sample_messages = list(set(r.message for r in recent_same_type[:5]))
@@ -233,7 +235,8 @@ class ErrorAggregator:
                 self._patterns[pattern_key].count = count
                 self._patterns[pattern_key].last_seen = record.timestamp
                 self._patterns[pattern_key].severity = severity
-                self._patterns[pattern_key].affected_plugins.extend(affected_plugins)
+                known = self._patterns[pattern_key].affected_plugins
+                known.extend(p for p in affected_plugins if p not in known)
 
     def on_pattern_detected(self, callback: Callable[[ErrorPattern], None]) -> None:
         """

src/font_manager.py

@@ -510,8 +510,17 @@ class FontManager:
                     "%spx", font_path, size_px, native_size
                 )
             face = freetype.Face(font_path)
-            # Set character size (width, height) in 1/64th of points
-            face.set_char_size(size_px * 64, size_px * 64, 72, 72)
+            try:
+                # Character size in 1/64th points at 72dpi == pixel size.
+                face.set_char_size(size_px * 64, size_px * 64, 72, 72)
+            except freetype.FT_Exception:
+                # FreeType rejects any size but the strike's own, and get_font
+                # used to answer that with PIL's default font -- a different
+                # typeface. Use the native strike, as element_style does.
+                if native_size is None or native_size == size_px:
+                    raise
+                face = freetype.Face(font_path)
+                face.set_char_size(native_size * 64, native_size * 64, 72, 72)
             return face
         except Exception as e:
             logger.error(f"Error loading BDF font {font_path}: {e}")

src/vegas_mode/coordinator.py

@@ -42,6 +42,12 @@ _FPS_HEALTHY_FRACTION = 0.9
 #: rather than fine.
 _FPS_HEARTBEAT_INTERVAL = 300.0
 
+#: Seconds between live-priority scans while scrolling. The scan asks every
+#: plugin mode has_live_priority() / has_live_content(): 139us per call on a
+#: Pi 4 with two scoreboards (9 modes), 1.7% of a 125fps frame, growing with
+#: every plugin. Game state doesn't change within a quarter second.
+_LIVE_PRIORITY_CHECK_INTERVAL = 0.25
+
 
 def _percentile(ordered: List[float], fraction: float) -> float:
     """Nearest-rank percentile of an already-sorted list.
@@ -70,6 +76,9 @@ class VegasModeCoordinator:
     - Provide status and control interface
     """
 
+    # Class-level so coordinators built without __init__ (tests) have it.
+    _last_live_check: float = float('-inf')
+
     def __init__(
         self,
         config: Dict[str, Any],
@@ -336,9 +345,14 @@ class VegasModeCoordinator:
             # Check for config updates (synchronized access)
             has_pending_update = self._pending_config_update
 
-        # Check for live priority
-        if self._check_live_priority():
-            return False
+        # Check for live priority (throttled; see _LIVE_PRIORITY_CHECK_INTERVAL).
+        # Only a negative result is ever reused: a positive one pauses Vegas,
+        # and run_frame() returns early while paused.
+        now = time.monotonic()
+        if now - self._last_live_check >= _LIVE_PRIORITY_CHECK_INTERVAL:
+            self._last_live_check = now
+            if self._check_live_priority():
+                return False
 
         # Apply pending config update outside lock
         if has_pending_update:
@@ -633,6 +647,18 @@ class VegasModeCoordinator:
 
         logger.debug("Config update queued (version %d)", self._config_version)
 
+    def apply_pending_config_if_idle(self) -> None:
+        """Apply a queued config update while Vegas isn't running.
+
+        run_frame() applies updates between frames but returns early once
+        Vegas has stopped, so without this a disable followed by a re-enable
+        would never take effect. Call from the display thread only.
+        """
+        with self._state_lock:
+            if self._is_active or not self._pending_config_update:
+                return
+        self._apply_pending_config()
+
     def _apply_pending_config(self) -> None:
         """Apply pending configuration update."""
         # Atomically grab pending config and clear it to avoid losing concurrent updates

src/wifi_manager.py

@@ -58,6 +58,12 @@ def get_wifi_config_path():
     
     return Path(project_root) / "config" / "wifi_config.json"
 
+
+def get_wifi_status_path() -> Path:
+    """The status-message file WiFiManager writes and the display controller
+    reads (config/wifi_status.json, next to wifi_config.json)."""
+    return get_wifi_config_path().parent / "wifi_status.json"
+
 HOSTAPD_CONFIG_PATH = Path("/etc/hostapd/hostapd.conf")
 DNSMASQ_CONFIG_PATH = Path("/etc/dnsmasq.d/ledmatrix-captive.conf")
 # Drop-in config for NetworkManager's built-in dnsmasq (ipv4.method=shared).
@@ -182,8 +188,12 @@ class WiFiManager:
                 'duration': duration
             }
             LED_STATUS_FILE.parent.mkdir(parents=True, exist_ok=True)
-            with open(LED_STATUS_FILE, 'w') as f:
+            # Write-then-rename: the display reads this at ~1 Hz and deletes
+            # a file it can't parse, so a half-written one would lose the message.
+            tmp_path = LED_STATUS_FILE.with_name(LED_STATUS_FILE.name + '.tmp')
+            with open(tmp_path, 'w') as f:
                 json.dump(status, f)
+            os.replace(tmp_path, LED_STATUS_FILE)
             logger.info(f"LED message: {message}")
         except Exception as e:
             logger.debug(f"Could not write LED status message: {e}")

test/js/README.md

@@ -15,7 +15,8 @@ npm install                 # jsdom, for the DOM suites only
 node run_all.js
 ```
 
-The unit suites need nothing but node. The DOM suites additionally need a
+The unit suites need nothing but node; `test/test_js_unit_suites.py` runs every
+`unit/*.js` under pytest, so CI covers them. The DOM suites additionally need a
 running web interface, because they test against the **real** server-rendered
 HTML and the **real** API rather than fixtures:
 
@@ -41,6 +42,7 @@ nothing is listening, so it stays useful in a bare checkout.
 | `unit/test_style_editor_layout_leaf_columns.js` | no | `columnsFor()` from `widgets/style-editor.js`: a layout-only key whose own value is a leaf (no x/y sub-object, e.g. a `show_logo` toggle) gets a self-keyed column instead of a blank, uneditable row |
 | `unit/test_style_editor_layout_leaf_collision.js` | no | `columnsFor()` from `widgets/style-editor.js`: a layout-only leaf key still gets its own column even when its name collides with an unrelated element's style sub-field or another layout axis's sub-field |
 | `unit/test_inline_handler_escaping.js` | no | The store, saved-repository and custom-registry inline `onclick` handlers and the live `window.updateImageList` from `plugins_manager.js`: a registry id, URL or uploaded file name carrying `'`, `"` or entities adds no attributes and reaches the handler intact, and the store's View button opens only http(s) links |
+| `unit/test_plugin_action_delegation.js` | no | The document-level card-action delegation and `handlePluginAction` from `plugins_manager.js`, run with the handler inside an IIFE as in the real file: each action is handled once, a Starlark app uninstall goes to `DELETE /starlark/apps/<id>`, and an uninstall is confirmed once |
 | `dom/test_installed_dom.js` | yes | The toolbar in a real DOM: pill/search/sort interaction, the HTMX partial re-swap, and a `getComputedStyle` check that `.filter-pill[data-active]` really matches the emitted markup |
 | `dom/test_store_dom.js` | yes | Store pagination, per-page, category, tri-state Installed button, and persistence across a re-boot, against the live registry |
 | `dom/test_no_double_fetch.js` | yes | Loads the **whole** `plugins_manager.js` and counts requests: typing in the store search must filter the cached list, not refetch `/api/v3/plugins/store/list` |

test/js/run_all.js

@@ -18,7 +18,8 @@ const UNIT = ['unit/test_list_filter.js', 'unit/test_render_cards.js',
               'unit/test_html_escaping.js', 'unit/test_style_editor_element_keys.js',
               'unit/test_style_editor_layout_leaf_columns.js',
               'unit/test_style_editor_layout_leaf_collision.js',
-              'unit/test_update_all.js', 'unit/test_inline_handler_escaping.js'];
+              'unit/test_update_all.js', 'unit/test_inline_handler_escaping.js',
+              'unit/test_plugin_action_delegation.js'];
 const DOM = ['dom/test_installed_dom.js', 'dom/test_store_dom.js', 'dom/test_no_double_fetch.js',
              'dom/test_tools_sections.js'];
 

test/js/unit/test_plugin_action_delegation.js

@@ -0,0 +1,127 @@
+// Installed-plugin card actions go through handlePluginAction exactly once.
+//
+// The document-level delegation in plugins_manager.js tested
+// `typeof handlePluginAction`, which lives inside the plugin-manager IIFE and
+// so was never visible to it. Every click took a copied fallback instead,
+// which stopped propagation (the grid's own listener never ran), asked to
+// confirm an uninstall twice, and sent Starlark app uninstalls to the plugin
+// endpoint instead of DELETE /starlark/apps/<id>.
+//
+// Runs the shipped global delegation and handlePluginAction, sliced out of
+// plugins_manager.js, against a minimal fake DOM.
+
+const fs = require('fs');
+const path = require('path');
+const vm = require('vm');
+
+const SRC = fs.readFileSync(
+  path.resolve(__dirname, '../../../web_interface/static/v3/plugins_manager.js'), 'utf8');
+
+function slice(startMarker, endMarker) {
+  const a = SRC.indexOf(startMarker);
+  const b = SRC.indexOf(endMarker, a);
+  if (a < 0 || b < 0) throw new Error(`could not find ${startMarker} .. ${endMarker}`);
+  return SRC.slice(a, b);
+}
+
+const GLOBAL_DELEGATION = slice('(function setupGlobalEventDelegation() {', '// Note: configurePlugin');
+const HANDLER = slice('function handlePluginAction(event) {', 'function findInstalledPlugin(pluginId)');
+
+let pass = 0, fail = 0;
+const ok = (label, cond, extra) => cond
+  ? (pass++, console.log('  ok   ' + label))
+  : (fail++, console.log('  FAIL ' + label + (extra !== undefined ? '  ' + JSON.stringify(extra) : '')));
+
+function setup() {
+  const listeners = {};
+  const calls = { confirm: 0, fetch: [], uninstallPlugin: [], togglePlugin: [] };
+  const window = {
+    installedPlugins: [{ id: 'clock', enabled: false }],
+    uninstallPlugin: id => calls.uninstallPlugin.push(id),
+    togglePlugin: (id, on) => calls.togglePlugin.push([id, on]),
+  };
+  const ctx = {
+    window,
+    document: {
+      addEventListener: (type, fn, capture) => { (listeners[type] = listeners[type] || []).push(fn); },
+    },
+    confirm: () => { calls.confirm++; return true; },
+    fetch: (url, opts) => { calls.fetch.push([url, opts && opts.method]); return new Promise(() => {}); },
+    alert: () => {},
+    console,
+    setTimeout,
+    debugLog: () => {},
+    getInstalledFilter: () => null,
+  };
+  vm.createContext(ctx);
+  // The handler lives inside the plugin-manager IIFE in the real file, so it
+  // runs in one here too: the global delegation must not see it by name.
+  vm.runInContext(GLOBAL_DELEGATION + '\n(function() {\n' + HANDLER + '\n})();', ctx);
+
+  const click = (action, pluginId) => {
+    const el = {
+      getAttribute: name => ({ 'data-action': action, 'data-plugin-id': pluginId })[name],
+      type: 'button',
+    };
+    let stopped = false;
+    const event = {
+      type: 'click',
+      target: { closest: () => el },
+      preventDefault() {},
+      stopPropagation() { stopped = true; },
+    };
+    for (const fn of listeners.click || []) fn(event);
+    return stopped;
+  };
+  return { window, calls, click, listeners };
+}
+
+(async () => {
+  console.log('\n-- plugin card action delegation --');
+
+  {
+    const t = setup();
+    ok('handlePluginAction is exposed on window', typeof t.window.handlePluginAction === 'function');
+    ok('document-level click listener registered', (t.listeners.click || []).length === 1);
+  }
+
+  {
+    // A non-Element target (a text node, the document) has no closest().
+    const t = setup();
+    let threw = null;
+    try { for (const fn of t.listeners.click) fn({ type: 'click', target: {} }); } catch (e) { threw = e; }
+    ok('non-Element event target is ignored', threw === null, threw && threw.message);
+  }
+
+  {
+    const t = setup();
+    t.click('uninstall', 'starlark:analogclock');
+    ok('Starlark uninstall confirms once', t.calls.confirm === 1, t.calls.confirm);
+    ok('Starlark uninstall hits DELETE /starlark/apps/<id>',
+       t.calls.fetch.length === 1 && t.calls.fetch[0][0] === '/api/v3/starlark/apps/analogclock'
+       && t.calls.fetch[0][1] === 'DELETE', t.calls.fetch);
+    ok('Starlark uninstall does not go to the plugin uninstaller', t.calls.uninstallPlugin.length === 0);
+  }
+
+  {
+    const t = setup();
+    t.click('uninstall', 'clock');
+    await new Promise(r => setTimeout(r, 20));
+    ok('plugin uninstall: handler itself does not confirm (uninstallPlugin does)', t.calls.confirm === 0,
+       t.calls.confirm);
+    ok('plugin uninstall calls uninstallPlugin once', t.calls.uninstallPlugin.length === 1
+       && t.calls.uninstallPlugin[0] === 'clock', t.calls.uninstallPlugin);
+  }
+
+  {
+    const t = setup();
+    const stopped = t.click('toggle', 'clock');
+    await new Promise(r => setTimeout(r, 20));
+    ok('toggle flips the stored state', JSON.stringify(t.calls.togglePlugin) === '[["clock",true]]',
+       t.calls.togglePlugin);
+    ok('handled once (propagation stopped)', stopped === true);
+  }
+
+  console.log(`\n${pass} passed, ${fail} failed`);
+  process.exit(fail ? 1 : 0);
+})();

test/test_api_v3_partial_main_save.py

@@ -210,6 +210,19 @@ class TestVegasCycleDurations:
         assert resp.status_code == 200, resp.get_json()
         assert saved['config']['display']['display_durations']['clock_duration'] == 45
 
+    def test_per_mode_duration_saves_and_blank_clears_it(self, api_v3_client, saved, api_v3_module):
+        # The Rotation page leaves a mode blank to mean "the plugin's own
+        # duration"; a saved value overrides the plugin, so blank must remove
+        # it rather than 400 or pin a number.
+        stored = copy.deepcopy(STORED)
+        stored['display']['display_durations'] = {'weather_current': 40, 'clock': 20}
+        api_v3_module.api_v3.config_manager.load_config.side_effect =             lambda *a, **k: copy.deepcopy(stored)
+        resp = _post_json(api_v3_client, {'__form_section': 'durations',
+                                          'duration__clock': '45',
+                                          'duration__weather_current': ''})
+        assert resp.status_code == 200, resp.get_json()
+        assert saved['config']['display']['display_durations'] == {'clock': 45}
+
 
 class TestRawSaveStartsAutoUpdateSetup:
     @pytest.fixture

test/test_api_v3_plugin_toggle_errors.py

@@ -0,0 +1,29 @@
+"""POST /plugins/toggle reports what actually went wrong.
+
+Every failure used to be mapped to PLUGIN_OPERATION_CONFLICT, so the user was
+told "A plugin operation is already in progress" when, say, the config could
+not be read.
+"""
+
+import json
+
+from test._api_v3_test_helpers import api_v3_client, api_v3_module  # noqa: F401
+
+
+def test_failure_is_not_reported_as_a_conflict(api_v3_client, api_v3_module, monkeypatch):
+    monkeypatch.setattr(api_v3_module, '_discovered_plugin_manifests',
+                        lambda *a, **k: {'clock': {}})
+    api_v3_module.api_v3.config_manager.load_config.side_effect = OSError('disk gone')
+
+    resp = api_v3_client.post('/api/v3/plugins/toggle',
+                              data=json.dumps({'plugin_id': 'clock', 'enabled': True}),
+                              content_type='application/json')
+
+    assert resp.status_code == 500
+    body = resp.get_json()
+    assert body['error_code'] != 'PLUGIN_OPERATION_CONFLICT'
+    assert 'already in progress' not in body['message']
+    assert body['message'] == 'Failed to enable plugin clock'
+    history = api_v3_module.api_v3.operation_history.record_operation
+    history.assert_called_once()
+    assert history.call_args.kwargs['plugin_id'] == 'clock'

test/test_display_controller_optimizations.py

@@ -110,12 +110,14 @@ class TestCachedConfigValues:
         assert controller._normal_brightness == expected
 
     def test_scroll_speed_cached(self, controller):
-        """_scroll_speed must equal what the config says."""
+        """_scroll_speed must equal what the config says, defaulting to Vegas's
+        own default so a follower dead-reckons at the leader's speed."""
+        from src.vegas_mode.config import VegasModeConfig
         expected = (
             controller.config
             .get("display", {})
             .get("vegas_scroll", {})
-            .get("scroll_speed", 75)
+            .get("scroll_speed", VegasModeConfig.from_config({}).scroll_speed)
         )
         assert controller._scroll_speed == expected
 

test/test_display_controller_settings.py

@@ -0,0 +1,121 @@
+"""DisplayController settings that the web UI saves but the display ignored.
+
+- Rotation & Durations: display.display_durations was never read, because
+  every plugin inherits get_display_duration() and the plugin was asked first.
+- WiFi status overlay: the controller looked for wifi_status.json one level
+  above the repo, so WiFiManager's messages never reached the panel.
+- Vegas: settings saved in the web UI never reached the running coordinator,
+  and the follower's scroll-speed default (75) disagreed with Vegas's (50).
+"""
+
+import os
+import threading
+from unittest.mock import MagicMock
+
+os.environ.setdefault("EMULATOR", "true")
+
+from src.display_controller import DisplayController
+
+
+def _controller(config=None, plugin_modes=None):
+    dc = object.__new__(DisplayController)
+    dc.config = config or {}
+    dc.plugin_modes = plugin_modes or {}
+    return dc
+
+
+def _plugin(duration):
+    plugin = MagicMock()
+    plugin.get_display_duration.return_value = duration
+    return plugin
+
+
+class TestDisplayDuration:
+    def test_saved_duration_overrides_the_plugin(self):
+        dc = _controller({'display': {'display_durations': {'clock': 45}}},
+                         {'clock': _plugin(15.0)})
+        assert dc._get_display_duration('clock') == 45.0
+
+    def test_unsaved_mode_uses_the_plugin_duration(self):
+        dc = _controller({'display': {'display_durations': {'other': 45}}},
+                         {'clock': _plugin(12.0)})
+        assert dc._get_display_duration('clock') == 12.0
+
+    def test_invalid_saved_values_fall_back_to_the_plugin(self):
+        for bad in (0, -5, True, '30', None):
+            dc = _controller({'display': {'display_durations': {'clock': bad}}},
+                             {'clock': _plugin(12.0)})
+            assert dc._get_display_duration('clock') == 12.0, bad
+
+    def test_unknown_mode_without_a_plugin_gets_the_default(self):
+        assert _controller()._get_display_duration('nothing') == 30
+
+    def test_hot_reloaded_config_is_used(self):
+        dc = _controller({}, {'clock': _plugin(15.0)})
+        dc._refresh_config_cache({'display': {'display_durations': {'clock': 60}}})
+        assert dc._get_display_duration('clock') == 60.0
+
+
+class TestWifiStatusPath:
+    def test_reader_and_writer_agree(self, test_display_controller):
+        from src.wifi_manager import get_wifi_status_path
+        repo = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+        path = get_wifi_status_path()
+        assert path.name == 'wifi_status.json'
+        # Inside this checkout's config/, not the directory above the repo.
+        assert os.path.normcase(str(path.parent.parent)) == os.path.normcase(repo)
+        assert test_display_controller.wifi_status_file == path
+
+    def test_status_message_is_written_whole(self, tmp_path, monkeypatch):
+        import json
+        import src.wifi_manager as wm
+        target = tmp_path / 'wifi_status.json'
+        monkeypatch.setattr(wm, 'LED_STATUS_FILE', target)
+        # The display polls this file and deletes one it can't parse, so it
+        # must appear by rename, never be seen half-written.
+        renames = []
+        real_replace = os.replace
+        monkeypatch.setattr(wm.os, 'replace',
+                            lambda src, dst: (renames.append(dst), real_replace(src, dst)))
+        manager = object.__new__(wm.WiFiManager)
+        manager._show_led_message('Connecting to home...', duration=10)
+        assert json.loads(target.read_text())['message'] == 'Connecting to home...'
+        assert renames == [target]
+        assert not (tmp_path / 'wifi_status.json.tmp').exists()
+
+
+class TestVegasSettings:
+    def _controller(self):
+        dc = _controller({'display': {'vegas_scroll': {'scroll_speed': 40}}})
+        dc._reconcile_flag_lock = threading.Lock()
+        dc._pending_plugin_reconcile = False
+        dc._enabled_set_changed = lambda old, new: False
+        dc._enabled_plugin_not_running = lambda new: False
+        dc.vegas_coordinator = MagicMock()
+        return dc
+
+    def test_changed_vegas_settings_reach_the_coordinator(self):
+        dc = self._controller()
+        old = {'display': {'vegas_scroll': {'scroll_speed': 40}}}
+        new = {'display': {'vegas_scroll': {'scroll_speed': 80}}}
+        dc._controller_config_change(old, new)
+        dc.vegas_coordinator.update_config.assert_called_once_with(new)
+        assert dc._scroll_speed == 80.0
+
+    def test_unrelated_saves_leave_vegas_alone(self):
+        # Applying a Vegas config rebuilds the strip, so only do it on change.
+        dc = self._controller()
+        old = {'display': {'vegas_scroll': {'scroll_speed': 40}, 'hardware': {'brightness': 50}}}
+        new = {'display': {'vegas_scroll': {'scroll_speed': 40}, 'hardware': {'brightness': 90}}}
+        dc._controller_config_change(old, new)
+        dc.vegas_coordinator.update_config.assert_not_called()
+
+    def test_scroll_speed_default_matches_vegas_config(self):
+        from src.vegas_mode.config import VegasModeConfig
+        assert DisplayController._vegas_scroll_speed({}) == VegasModeConfig.from_config({}).scroll_speed
+
+    def test_stopped_coordinator_applies_queued_config(self):
+        dc = self._controller()
+        dc.on_demand_active = False
+        dc._is_vegas_mode_active()
+        dc.vegas_coordinator.apply_pending_config_if_idle.assert_called_once()

test/test_error_aggregator.py

@@ -154,6 +154,16 @@ class TestPatternDetection:
 
         assert "ValueError" not in aggregator._patterns
 
+    def test_affected_plugins_stays_bounded(self):
+        """Each repeat used to append every plugin in the window again, so a
+        plugin failing in a loop grew this list without limit (3,000 errors
+        from three plugins reached 2.5 million entries)."""
+        aggregator = ErrorAggregator(pattern_threshold=2, pattern_window_minutes=60)
+        for i in range(300):
+            aggregator.record_error(error=ValueError("loop"), plugin_id=f"p{i % 3}")
+        assert aggregator._patterns["ValueError"].affected_plugins == ["p0", "p1", "p2"]
+        assert aggregator._patterns["ValueError"].to_dict()["affected_plugins"] == ["p0", "p1", "p2"]
+
     def test_pattern_severity_increases_with_count(self):
         """Pattern severity should increase with more occurrences."""
         aggregator = ErrorAggregator(

test/test_font_manager.py

@@ -45,6 +45,14 @@ class TestGetFont:
         font = fm.get_font("five_by_seven", 7)
         assert isinstance(font, freetype.Face)
 
+    def test_bdf_at_a_size_it_lacks_uses_its_native_strike(self, fm):
+        # FreeType rejects any size but the strike's own. This used to hand
+        # back PIL's default font, a different typeface, for 5x7 at 8 or 10.
+        for size in (8, 10):
+            font = fm.get_font("five_by_seven", size)
+            assert isinstance(font, freetype.Face), size
+            assert font.size.y_ppem == 7
+
     def test_repeat_call_returns_cached_identity(self, fm):
         first = fm.get_font("press_start", 8)
         hits_before = fm.performance_stats["cache_hits"]

test/test_js_unit_suites.py

@@ -0,0 +1,27 @@
+"""Run every web-interface JS unit suite (test/js/unit/*.js) under pytest.
+
+They need nothing but node, but CI ran only one of them, from
+test/web_interface/test_update_all_plugins.py. The DOM suites need jsdom and
+a running server, so they stay with test/js/run_all.js.
+"""
+
+import shutil
+import subprocess
+from pathlib import Path
+
+import pytest
+
+UNIT_DIR = Path(__file__).resolve().parent / 'js' / 'unit'
+SUITES = sorted(UNIT_DIR.glob('test_*.js'))
+
+
+def test_suites_found():
+    assert SUITES, f"no JS unit suites under {UNIT_DIR}"
+
+
+@pytest.mark.skipif(shutil.which('node') is None, reason='node is not installed')
+@pytest.mark.parametrize('suite', SUITES, ids=[s.name for s in SUITES])
+def test_js_unit_suite(suite):
+    result = subprocess.run([shutil.which('node'), str(suite)], capture_output=True,
+                            text=True, timeout=120, cwd=str(UNIT_DIR.parent))
+    assert result.returncode == 0, result.stdout + result.stderr

test/test_one_shot_retry.py

@@ -0,0 +1,65 @@
+"""retry() in scripts/install/one-shot-install.sh retries, and reports failure.
+
+It used `if ! "$@"; then status=$?`, where $? is the status of the negation,
+always 0: a failed command was never retried and retry() returned success,
+so a failed `git clone` carried on until a later check noticed the missing
+checkout. The apt steps now retry for real but stay non-fatal, as they
+effectively were; a clone that keeps failing stops the install.
+"""
+import re
+import subprocess
+import sys
+from pathlib import Path
+
+import pytest
+
+ONE_SHOT = Path(__file__).resolve().parent.parent / "scripts" / "install" / "one-shot-install.sh"
+
+pytestmark = pytest.mark.skipif(
+    not sys.platform.startswith("linux"), reason="runs the installer's bash under Linux"
+)
+
+
+def _function(name):
+    text = ONE_SHOT.read_text(encoding="utf-8")
+    m = re.search(rf"^{name}\(\) \{{\n.*?^\}}\n", text, re.S | re.M)
+    assert m, f"{name}() not found in one-shot-install.sh"
+    return m.group(0)
+
+
+def _run(snippet):
+    script = (
+        "set -Eeuo pipefail\n"
+        "trap 'echo ERR_TRAP_FIRED >&2; exit 99' ERR\n"
+        "print_error() { echo \"E: $*\" >&2; }\n"
+        "print_warning() { echo \"W: $*\" >&2; }\n"
+        "sleep() { :; }\n"
+        f"{_function('retry')}\n"
+        f"{snippet}\n"
+    )
+    return subprocess.run(["bash", "-c", script], capture_output=True, text=True)
+
+
+def test_failure_is_retried_and_reported():
+    r = _run("n=0; f() { n=$((n+1)); return 7; }\n"
+             "if retry f; then echo OK; else echo \"FAILED $? after $n\"; fi")
+    assert r.stdout.strip() == "FAILED 7 after 3", r.stdout + r.stderr
+
+
+def test_success_on_a_later_attempt():
+    r = _run("n=0; f() { n=$((n+1)); [ $n -ge 2 ]; }\n"
+             "retry f && echo \"OK after $n\"")
+    assert r.stdout.strip() == "OK after 2", r.stdout + r.stderr
+
+
+def test_a_plain_call_that_keeps_failing_stops_the_script():
+    r = _run("retry false\necho SHOULD_NOT_RUN")
+    assert "SHOULD_NOT_RUN" not in r.stdout
+    assert "ERR_TRAP_FIRED" in r.stderr
+
+
+def test_apt_steps_stay_non_fatal():
+    text = ONE_SHOT.read_text(encoding="utf-8")
+    for line in text.splitlines():
+        if re.search(r"\bretry (sudo )?apt-get ", line):
+            assert "||" in line, f"apt step would now abort the install: {line.strip()}"

test/test_vegas_coordinator_config.py

@@ -0,0 +1,92 @@
+"""VegasModeCoordinator: settings updates and the per-frame live check.
+
+- Settings saved in the web UI are queued with update_config() and applied
+  between frames. run_frame() returns early once Vegas has stopped, so a
+  disable followed by a re-enable used to be queued forever.
+- The live-priority scan asks every plugin mode whether it is live. It ran on
+  every frame (125fps); it now runs at most every _LIVE_PRIORITY_CHECK_INTERVAL.
+"""
+
+import threading
+from unittest.mock import MagicMock
+
+from src.vegas_mode import coordinator as coordinator_module
+from src.vegas_mode.config import VegasModeConfig
+from src.vegas_mode.coordinator import VegasModeCoordinator
+
+
+def _coordinator(active=True):
+    # Built without __init__ so no display, stream or render stack is needed.
+    c = VegasModeCoordinator.__new__(VegasModeCoordinator)
+    c.vegas_config = VegasModeConfig.from_config({'display': {'vegas_scroll': {'enabled': True}}})
+    c.render_pipeline = MagicMock()
+    c.render_pipeline.has_deferred.return_value = False
+    c.render_pipeline.needs_extension.return_value = False
+    c.render_pipeline.is_cycle_complete.return_value = False
+    c.stream_manager = MagicMock()
+    c.plugin_adapter = MagicMock()
+    c.stats = {'cycles_completed': 0, 'config_updates': 0}
+    c._state_lock = threading.Lock()
+    c._is_active = active
+    c._is_paused = False
+    c._should_stop = False
+    c._pending_config_update = False
+    c._pending_config = None
+    c._config_version = 0
+    c._live_priority_check = None
+    c._live_priority_active = False
+    c._interrupt_check = None
+    c.sync_manager = None
+    return c
+
+
+class TestConfigWhileStopped:
+    def test_queued_config_applies_while_stopped(self):
+        c = _coordinator(active=False)
+        c.update_config({'display': {'vegas_scroll': {'enabled': True, 'scroll_speed': 90}}})
+        c.start = MagicMock()
+        c.apply_pending_config_if_idle()
+        assert c.vegas_config.scroll_speed == 90
+        assert c._pending_config_update is False
+
+    def test_reenabling_after_a_disable_takes_effect(self):
+        c = _coordinator(active=False)
+        c.vegas_config = VegasModeConfig.from_config({})  # disabled
+        c.start = MagicMock()
+        c.update_config({'display': {'vegas_scroll': {'enabled': True}}})
+        c.apply_pending_config_if_idle()
+        assert c.is_enabled
+        c.start.assert_called_once()
+
+    def test_running_coordinator_waits_for_the_next_frame(self):
+        c = _coordinator(active=True)
+        c.update_config({'display': {'vegas_scroll': {'enabled': True, 'scroll_speed': 90}}})
+        c.apply_pending_config_if_idle()
+        assert c.vegas_config.scroll_speed != 90
+        assert c._pending_config_update is True
+
+
+class TestLivePriorityThrottle:
+    def test_scan_runs_at_most_once_per_interval(self, monkeypatch):
+        now = [1000.0]
+        monkeypatch.setattr(coordinator_module.time, 'monotonic', lambda: now[0])
+        c = _coordinator()
+        c._live_priority_check = MagicMock(return_value=None)
+
+        for _ in range(10):
+            c.run_frame()
+        assert c._live_priority_check.call_count == 1
+
+        now[0] += coordinator_module._LIVE_PRIORITY_CHECK_INTERVAL
+        c.run_frame()
+        assert c._live_priority_check.call_count == 2
+
+    def test_live_content_still_pauses_vegas(self, monkeypatch):
+        monkeypatch.setattr(coordinator_module.time, 'monotonic', lambda: 1000.0)
+        c = _coordinator()
+        c._live_priority_check = MagicMock(return_value='nfl_live')
+        c.pause = MagicMock()
+        assert c.run_frame() is False
+        c.pause.assert_called_once()
+        c.render_pipeline.render_frame.assert_not_called()
+

test/test_web_smoke.py

@@ -186,6 +186,28 @@ def test_durations_page_groups_by_plugin(client):
     assert "Other saved entries" in body
 
 
+def test_durations_page_leaves_unsaved_modes_blank(client):
+    """A saved duration overrides the plugin's own, so the page must not
+    pre-fill one for every mode: the first save would pin them all. Unsaved
+    modes are blank, with the plugin's duration as the placeholder."""
+    import copy
+    from web_interface.blueprints import pages_v3 as pv
+    config = copy.deepcopy(SMOKE_CONFIG)
+    config["clock"]["display_duration"] = 20
+    config["display"]["display_durations"] = {"weather_current": 40}
+    pv.pages_v3.config_manager.load_config.return_value = config
+    body = client.get("/partials/durations").get_data(as_text=True)
+
+    def field(mode):
+        start = body.index(f'id="duration__{mode}"')
+        return body[start:body.index(">", start)]
+
+    assert 'value=""' in field("clock") and 'placeholder="20"' in field("clock")
+    assert 'value="40"' in field("weather_current")
+    assert 'value=""' in field("weather_daily")
+    assert 'placeholder="15"' in field("weather_daily")
+
+
 def test_display_advanced_section_contains_tuning_fields(client):
     body = client.get("/partials/display").get_data(as_text=True)
     adv = body.find('id="display-section-advanced-hardware"')

test/test_web_sudoers_installers_agree.py

@@ -129,3 +129,18 @@ def test_every_granted_helper_is_hardened_in_first_time_install_after_chown():
     assert loop.start() > project_chown, (
         "helper hardening runs before Step 11's project-wide chown, which undoes it")
     assert _granted_helpers() <= set(loop.group(1).split())
+
+
+def test_no_grant_runs_a_file_the_web_user_can_edit():
+    """Every project file granted as root must be a fix_perms helper, which
+    both installers chown root:root (checked above). Anything else under the
+    project root is owned by the user after Step 11's chown, so a NOPASSWD
+    rule for it lets the web user rewrite the file and run it as root. The
+    grants for display_controller.py, start_display.sh and stop_display.sh
+    were exactly that, and nothing ever ran them through sudo."""
+    for installer in (FIRST_TIME, CONFIGURE):
+        for _, command in _grants(installer):
+            for token in command.split():
+                if token.startswith("$PROJECT_ROOT/"):
+                    assert token.startswith("$PROJECT_ROOT/scripts/fix_perms/"), (
+                        f"{installer.name} grants root on a user-owned file: {command}")

web_interface/blueprints/api_v3/config.py

@@ -985,6 +985,10 @@ def save_main_config():
                 mode_key = field[len('duration__'):]
                 if not mode_key:
                     continue
+                if raw_value is None or str(raw_value).strip() == '':
+                    # Blank means "use the plugin's own duration".
+                    current_config['display']['display_durations'].pop(mode_key, None)
+                    continue
                 try:
                     int_value = int(raw_value)
                 except (ValueError, TypeError, OverflowError):

web_interface/blueprints/api_v3/plugins.py

@@ -409,6 +409,8 @@ def manage_plugin_limits(plugin_id):
 @api_v3.route('/plugins/toggle', methods=['POST'])
 def toggle_plugin():
     """Toggle plugin enabled/disabled"""
+    plugin_id = None
+    enabled = None
     try:
         if not api_v3.plugin_manager or not api_v3.config_manager:
             return jsonify({'status': 'error', 'message': 'Plugin or config manager not initialized'}), 500
@@ -506,21 +508,21 @@ def toggle_plugin():
             message=f"Plugin {plugin_id} {'enabled' if enabled else 'disabled'} successfully"
         )
     except Exception as e:
-        from src.web_interface.errors import WebInterfaceError
-        error = WebInterfaceError.from_exception(e, ErrorCode.PLUGIN_OPERATION_CONFLICT)
+        # Not PLUGIN_OPERATION_CONFLICT: that told the user "an operation is
+        # already in progress" whatever actually went wrong.
+        logger.error('Error toggling plugin %s', plugin_id, exc_info=True)
         if api_v3.operation_history:
-            toggle_type = "enable" if ('data' in locals() and data.get('enabled')) else "disable"
             api_v3.operation_history.record_operation(
-                toggle_type,
-                plugin_id=data.get('plugin_id') if 'data' in locals() else None,
+                "enable" if enabled else "disable",
+                plugin_id=plugin_id,
                 status="failed",
                 error=str(e)
             )
+        action = 'enable' if enabled else 'disable' if enabled is not None else 'toggle'
         return error_response(
-            error.error_code,
-            error.message,
-            details=error.details,
-            context=error.context,
+            ErrorCode.UNKNOWN_ERROR,
+            f"Failed to {action} plugin {plugin_id or ''}".rstrip(),
+            details=describe_exception(e),
             status_code=500
         )
 @api_v3.route('/plugins/operation/<operation_id>', methods=['GET'])

web_interface/blueprints/pages_v3.py

@@ -552,14 +552,39 @@ def _load_display_partial():
         logger.error("Error loading partial", exc_info=True)
         return "Error loading partial", 500
 
+def _plugin_default_duration(plugin_id, plugin_config):
+    """Seconds a plugin shows each screen when the Rotation page sets none.
+
+    Mirrors BasePlugin.get_display_duration's config fallback: the plugin's
+    display_duration, else its schema default, else 15.
+    """
+    def _valid(v):
+        return isinstance(v, (int, float)) and not isinstance(v, bool) and v > 0
+
+    value = plugin_config.get('display_duration')
+    if not _valid(value):
+        schema = None
+        schema_mgr = getattr(pages_v3, 'schema_manager', None)
+        if schema_mgr is not None:
+            try:
+                schema = schema_mgr.load_schema(plugin_id)
+            except Exception:
+                logger.debug("durations: no schema for %s", plugin_id, exc_info=True)
+        value = plugin_config_defaults(schema if isinstance(schema, dict) else None).get(
+            'display_duration')
+        if not _valid(value):
+            value = 15
+    return int(value) if float(value).is_integer() else value
+
+
 def _load_durations_partial():
     """Load rotation & durations partial.
 
-    Builds one duration entry per display mode of every enabled plugin
-    (falling back to the display controller's 30s default), overlaid with any
-    values saved in display.display_durations. Historically the template only
-    looped over saved keys, and nothing ever populated them, so the page
-    rendered empty.
+    Builds one duration entry per display mode of every enabled plugin. A mode
+    with a value saved in display.display_durations shows it; the rest are
+    blank, with the plugin's own duration as the placeholder, because a saved
+    value overrides the plugin (see DisplayController._get_display_duration).
+    Pre-filling every mode would pin them all on the first save.
     """
     try:
         if pages_v3.config_manager:
@@ -578,10 +603,12 @@ def _load_durations_partial():
                             continue
                         modes = pages_v3.plugin_manager.get_plugin_display_modes(pid) or [pid]
                         covered_keys.update(modes)
+                        default = _plugin_default_duration(pid, main_config.get(pid, {}) or {})
                         duration_groups.append({
                             'plugin_id': pid,
                             'plugin_name': info.get('name') or pid,
-                            'modes': [{'key': m, 'value': saved.get(m, 30)} for m in modes],
+                            'modes': [{'key': m, 'value': saved.get(m, ''), 'default': default}
+                                      for m in modes],
                         })
                     # Saved keys not owned by any enabled plugin (disabled or
                     # uninstalled plugins) stay visible rather than vanishing.

web_interface/static/v3/plugins_manager.js

@@ -401,124 +401,25 @@ window.__pendingInstalledPlugins = window.__pendingInstalledPlugins || null;
 window.__pendingStorePlugins = window.__pendingStorePlugins || null;
 window.__pluginDomReady = window.__pluginDomReady || false;
 
-// Set up global event delegation for plugin actions (works even before plugins are loaded)
+// Document-level delegation for plugin card actions, so a card works even if
+// it was rendered before the grid's own listener was attached. It hands the
+// event to handlePluginAction, which the plugin-manager IIFE below exposes on
+// window. (It used to test `typeof handlePluginAction`, which is IIFE-scoped
+// and so never visible here: every click took a copied fallback instead, which
+// asked to confirm an uninstall twice and sent Starlark app uninstalls to the
+// plugin endpoint.)
 (function setupGlobalEventDelegation() {
-    // Use document-level delegation so it works for dynamically added content
     const handleGlobalPluginAction = function(event) {
-        // Only handle if it's a plugin action
-        const button = event.target.closest('button[data-action][data-plugin-id]') ||
-                       event.target.closest('input[data-action][data-plugin-id]');
-        if (!button) return;
-
-        const action = button.getAttribute('data-action');
-        const pluginId = button.getAttribute('data-plugin-id');
-
-        // For toggle and configure, ensure functions are available
-        if (action === 'toggle' || action === 'configure') {
-            const funcName = action === 'toggle' ? 'togglePlugin' : 'configurePlugin';
-            if (!window[funcName] || typeof window[funcName] !== 'function') {
-                // Prevent default and stop propagation immediately to avoid double handling
-                event.preventDefault();
-                event.stopPropagation();
-
-                console.warn(`[GLOBAL DELEGATION] ${funcName} not available yet, waiting...`);
-
-                // Capture state synchronously from plugin data (source of truth)
-                let targetChecked = false;
-                if (action === 'toggle') {
-                    const plugin = (window.installedPlugins || []).find(p => p.id === pluginId);
-
-                    let currentEnabled;
-                    if (plugin) {
-                        currentEnabled = Boolean(plugin.enabled);
-                    } else if (button.type === 'checkbox') {
-                        currentEnabled = button.checked;
-                    } else {
-                        currentEnabled = false;
-                    }
-
-                    targetChecked = !currentEnabled; // Toggle to opposite state
-                }
-
-                // Wait for function to be available
-                let attempts = 0;
-                const maxAttempts = 20; // 1 second total
-                const checkInterval = setInterval(() => {
-                    attempts++;
-                    if (window[funcName] && typeof window[funcName] === 'function') {
-                        clearInterval(checkInterval);
-                        // Call the function directly
-                        if (action === 'toggle') {
-                            window.togglePlugin(pluginId, targetChecked);
-                        } else {
-                            window.configurePlugin(pluginId);
-                        }
-                    } else if (attempts >= maxAttempts) {
-                        clearInterval(checkInterval);
-                        console.error(`[GLOBAL DELEGATION] ${funcName} not available after ${maxAttempts} attempts`);
-                        if (typeof showNotification === 'function') {
-                            showNotification(`${funcName} not loaded. Please refresh the page.`, 'error');
-                        }
-                    }
-                }, 50);
-                return; // Don't proceed with normal handling
-            }
-        }
-
-        // Prevent default and stop propagation to avoid double handling
-        event.preventDefault();
-        event.stopPropagation();
-
-        // If handlePluginAction exists, use it; otherwise handle directly
-        if (typeof handlePluginAction === 'function') {
-            handlePluginAction(event);
-        } else {
-            // Fallback: handle directly if functions are available
-            if (action === 'toggle' && window.togglePlugin) {
-                // Get the current enabled state from plugin data (source of truth)
-                const plugin = (window.installedPlugins || []).find(p => p.id === pluginId);
-
-                let currentEnabled;
-                if (plugin) {
-                    currentEnabled = Boolean(plugin.enabled);
-                } else if (button.type === 'checkbox') {
-                    currentEnabled = button.checked;
-                } else {
-                    currentEnabled = false;
-                }
-
-                // Toggle the state - we want the opposite of current state
-                const isChecked = !currentEnabled;
-
-                // Prevent default behavior to avoid double-toggling and change event
-                // (Already done at start of function, but safe to repeat)
-                event.preventDefault();
-                event.stopPropagation();
-
-                debugLog('[DEBUG toggle fallback] Plugin:', pluginId, 'Current enabled (from data):', currentEnabled, 'New state:', isChecked);
-
-                window.togglePlugin(pluginId, isChecked);
-            } else if (action === 'configure' && window.configurePlugin) {
-                event.preventDefault();
-                event.stopPropagation();
-                window.configurePlugin(pluginId);
-            } else if (action === 'update' && window.updatePlugin) {
-                event.preventDefault();
-                event.stopPropagation();
-                debugLog('[DEBUG update fallback] Updating plugin:', pluginId);
-                window.updatePlugin(pluginId);
-            } else if (action === 'uninstall' && window.uninstallPlugin) {
-                event.preventDefault();
-                event.stopPropagation();
-                debugLog('[DEBUG uninstall fallback] Uninstalling plugin:', pluginId);
-                if (confirm(`Are you sure you want to uninstall ${pluginId}?`)) {
-                    window.uninstallPlugin(pluginId);
-                }
-            }
-        }
+        const target = event.target;
+        if (!target || typeof target.closest !== 'function') return;
+        const el = target.closest('button[data-action][data-plugin-id]') ||
+                   target.closest('input[data-action][data-plugin-id]');
+        if (!el || typeof window.handlePluginAction !== 'function') return;
+        window.handlePluginAction(event);
     };
 
-    // Set up delegation on document (capture phase for better reliability)
+    // Capture phase, so this runs before the grid's own listener;
+    // handlePluginAction stops propagation, so an action is handled once.
     document.addEventListener('click', handleGlobalPluginAction, true);
     document.addEventListener('change', handleGlobalPluginAction, true);
     debugLog('[PLUGINS SCRIPT] Global event delegation set up');
@@ -1846,6 +1747,7 @@ function handlePluginAction(event) {
             break;
     }
 }
+window.handlePluginAction = handlePluginAction;
 
 function findInstalledPlugin(pluginId) {
     const plugins = window.installedPlugins || installedPlugins || [];

web_interface/templates/v3/partials/durations.html

@@ -36,7 +36,7 @@
         <div class="bg-gray-50 rounded-lg p-4 space-y-5">
             <div>
                 <h3 class="text-md font-medium text-gray-900 mb-1">Screen Durations</h3>
-                <p class="text-sm text-gray-600">How long each screen stays on before rotating to the next one, in seconds (5&ndash;600, default 30).</p>
+                <p class="text-sm text-gray-600">How long each screen stays on before rotating to the next one, in seconds (5&ndash;600). Leave a screen blank to use its plugin's own duration, shown greyed out.</p>
             </div>
             {% for group in duration_groups %}
             <div>
@@ -45,12 +45,13 @@
                     {% for mode in group.modes %}
                     <div class="form-group" id="setting-durations-{{ mode.key }}" data-setting-key="display.display_durations.{{ mode.key }}">
                         <label for="duration__{{ mode.key }}" class="block text-sm font-medium text-gray-700">
-                            {{ mode.key | replace('_', ' ') | title }}{{ ui.help_tip('How long the ' ~ (mode.key | replace('_', ' ')) ~ ' screen stays on before rotating to the next one, in seconds.\nRange: 5–600. Currently ' ~ mode.value ~ 's.', mode.key | replace('_', ' ') | title) }}
+                            {{ mode.key | replace('_', ' ') | title }}{{ ui.help_tip('How long the ' ~ (mode.key | replace('_', ' ')) ~ ' screen stays on before rotating to the next one, in seconds.\nRange: 5–600.' ~ (' Blank uses the plugin\'s own ' ~ mode.default ~ 's.' if mode.default else ''), mode.key | replace('_', ' ') | title) }}
                         </label>
                         <input type="number"
                                id="duration__{{ mode.key }}"
                                name="duration__{{ mode.key }}"
                                value="{{ mode.value }}"
+                               {% if mode.default %}placeholder="{{ mode.default }}"{% endif %}
                                min="5"
                                max="600"
                                class="form-control">