fix(plugins): build get_state_info() as one locked snapshot

Every field was read under its own lock, so an unload running concurrently
could be observed half-done: 'state' read before clear_state() removed it
and 'state_history_count' read after, handing PluginManager.get_plugin_info()
a plugin that is ENABLED with zero transitions.

The whole payload is now built in one critical section. _lock is an RLock,
so the helpers called inside it can still take it.

The regression test runs a reader against a thread that repeatedly fills and
clears the same plugin, and fails on the first torn snapshot. Verified by
removing only the lock: fails on 3 of 3 runs, passes on 3 of 3 with it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW

src/background_data_service.py

@@ -14,12 +14,11 @@ Key Features:
 - Memory-efficient data storage
 """
 
-import itertools
 import time
 import logging
 import threading
 import requests
-from typing import Dict, Any, Optional, Callable, List
+from typing import Dict, Any, Optional, Callable
 from dataclasses import dataclass, field
 from enum import Enum
 import queue
@@ -51,11 +50,6 @@ class FetchRequest:
     max_retries: int = 3
     priority: int = 1  # Higher number = higher priority
     callback: Optional[Callable] = None
-    # Callbacks from submitters that JOINED this fetch instead of starting a
-    # duplicate one. The primary `callback` above belongs to whoever created
-    # the request; these belong to everyone who asked for the same cache_key
-    # while it was still in flight.
-    extra_callbacks: List[Callable] = field(default_factory=list)
     created_at: float = field(default_factory=time.time)
     status: FetchStatus = FetchStatus.PENDING
     result: Optional[Any] = None
@@ -96,20 +90,6 @@ class BackgroundDataService:
         
         # Thread management
         self.executor = ThreadPoolExecutor(max_workers=max_workers, thread_name_prefix="BackgroundData")
-        # cache_key -> request_id for fetches currently in flight. Submitting
-        # the same key twice used to start two identical fetches: request_id
-        # carries a millisecond timestamp, so every submit looked new, and
-        # active_requests is keyed by it rather than by what is being fetched.
-        # On a real board the season-schedule key is requested by both the
-        # Recent and the Upcoming manager, which miss the cache in the same
-        # millisecond and each download and parse the same payload.
-        self._inflight_by_cache_key: Dict[str, str] = {}
-        # request_id was sport_year_milliseconds, which is not unique: two
-        # submits inside the same millisecond produced the SAME id, so one
-        # silently replaced the other in active_requests and completed_requests.
-        # Rare before, but dedupe hands this id back to every joiner as their
-        # handle for get_result(), so it has to be unique. A counter is enough.
-        self._request_seq = itertools.count()
         self.active_requests: Dict[str, FetchRequest] = {}
         self.completed_requests: Dict[str, FetchResult] = {}
         self.request_queue = queue.PriorityQueue()
@@ -197,9 +177,7 @@ class BackgroundDataService:
         if cache_key is None:
             cache_key = self.get_sport_cache_key(sport)
         
-        with self._lock:
-            request_id = (f"{sport}_{year}_{int(time.time() * 1000)}"
-                          f"_{next(self._request_seq)}")
+        request_id = f"{sport}_{year}_{int(time.time() * 1000)}"
         
         # Check cache first
         cached_data = self.cache_manager.get(cache_key)
@@ -240,29 +218,7 @@ class BackgroundDataService:
         )
         
         with self._lock:
-            existing_id = self._inflight_by_cache_key.get(cache_key)
-            existing = self.active_requests.get(existing_id) if existing_id else None
-            if existing_id and existing is None:
-                # Stranded index entry: the request it names is gone. Drop it and
-                # fetch normally. Looking the request up rather than trusting the
-                # id is what stops a stale entry wedging a key forever.
-                del self._inflight_by_cache_key[cache_key]
-            if existing is not None:
-                # Someone is already fetching this key. Ride along rather than
-                # duplicating the download, the parse and the resident copy.
-                if callback:
-                    existing.extra_callbacks.append(callback)
-                self.stats['deduplicated_requests'] = (
-                    self.stats.get('deduplicated_requests', 0) + 1
-                )
-                logger.info(
-                    "Joined in-flight fetch %s for %s (cache_key=%s) instead of "
-                    "starting a duplicate", existing_id, sport, cache_key
-                )
-                return existing_id
-
             self.active_requests[request_id] = request
-            self._inflight_by_cache_key[cache_key] = request_id
             self.stats['total_requests'] += 1
             self.stats['cache_misses'] += 1
         
@@ -313,28 +269,6 @@ class BackgroundDataService:
             # Log data validation
             logger.debug(f"Validated {len(events)} events for {request.sport} {request.year}")
             
-            # A cancelled request must not commit anything. Cancelling
-            # releases the cache_key, so a replacement fetch for the same key
-            # may already be in flight or finished -- writing this response to
-            # the cache now would overwrite fresher data with the response
-            # nobody wanted. The worker has no way to abort the HTTP call, so
-            # this is where the work gets discarded.
-            with self._lock:
-                cancelled = request.status == FetchStatus.CANCELLED
-            if cancelled:
-                logger.info(
-                    "Discarding response for cancelled request %s; %s may "
-                    "already belong to a replacement fetch",
-                    request.id, request.cache_key
-                )
-                return FetchResult(
-                    request_id=request.id,
-                    success=False,
-                    error="cancelled",
-                    fetch_time=time.time() - start_time,
-                    retry_count=request.retry_count
-                )
-
             # Cache the data
             self.cache_manager.set(request.cache_key, data)
             
@@ -377,22 +311,6 @@ class BackgroundDataService:
                 self.completed_requests[request.id] = result
                 if request.id in self.active_requests:
                     del self.active_requests[request.id]
-                # Stop accepting joiners and take the callback list in the same
-                # critical section. A submitter that arrives after this point
-                # finds no in-flight entry and either hits the cache (written
-                # above, before the result was built) or starts a fresh fetch --
-                # what it must never do is join a fetch whose callbacks have
-                # already run and then never be called.
-                if self._inflight_by_cache_key.get(request.cache_key) == request.id:
-                    del self._inflight_by_cache_key[request.cache_key]
-                # A cancelled request delivers nothing: its joiners were told
-                # about a fetch that has been abandoned, and a replacement will
-                # call them via its own request.
-                if request.status == FetchStatus.CANCELLED:
-                    callbacks = []
-                else:
-                    callbacks = ([request.callback] if request.callback else [])
-                    callbacks.extend(request.extra_callbacks)
                 
                 # Update statistics
                 if result.success:
@@ -409,11 +327,10 @@ class BackgroundDataService:
             # Periodic cleanup after storing result
             self._cleanup_completed_requests()
             
-            # Call every callback: the original submitter's and any that joined
-            # this fetch. One raising must not stop the others being delivered.
-            for cb in callbacks:
+            # Call callback if provided
+            if request.callback:
                 try:
-                    cb(result)
+                    request.callback(result)
                 except Exception as e:
                     logger.error(f"Error in callback for request {request.id}: {e}")
         
@@ -523,11 +440,6 @@ class BackgroundDataService:
                 request = self.active_requests[request_id]
                 request.status = FetchStatus.CANCELLED
                 del self.active_requests[request_id]
-                # Cancelling is the other way a request leaves active_requests,
-                # so the in-flight index has to be released here too or the key
-                # stays pointed at a request that no longer exists.
-                if self._inflight_by_cache_key.get(request.cache_key) == request_id:
-                    del self._inflight_by_cache_key[request.cache_key]
                 logger.info(f"Cancelled request {request_id}")
                 return True
             return False

src/common/scroll_helper.py

@@ -328,6 +328,10 @@ class ScrollHelper:
             elapsed_time = current_time - (self.scroll_start_time or current_time)
             # The image already includes display_width padding, so we only need total_scroll_width
             required_total_distance = self.total_scroll_width
+            # Progress telemetry, emitted every few seconds for the whole of
+            # every scroll. It says how far along a marquee is, which is what
+            # you turn debug on to watch and not something an operator needs
+            # in the journal on a device that scrolls all day.
             self.logger.debug(
                 "Scroll progress: elapsed=%.2fs, target=%.2fs, total_scrolled=%.0f/%d px (%.1f%%)",
                 elapsed_time,

src/display_controller.py

@@ -181,6 +181,16 @@ 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]] = {}
+        # plugin_display_modes is mutated only by _register_loaded_plugin /
+        # _unregister_plugin on the render thread, but the config-watcher
+        # thread reads it in _enabled_plugin_not_running. Both mutation sites
+        # run during reconcile (rare), so this lock never touches the per-frame
+        # path -- the hot-path reads are same-thread as the writes.
+        self._plugin_modes_lock = threading.Lock()
+        # Guards the consume-and-clear of _pending_plugin_reconcile. Only taken
+        # when a reconcile is actually pending or a config change arrives, both
+        # rare -- the per-frame path just reads the bool.
+        self._reconcile_flag_lock = threading.Lock()
         # Per-plugin config-change callbacks, kept so we can unsubscribe a
         # plugin when it is disabled live.
         self._plugin_config_callbacks: Dict[str, Callable] = {}
@@ -463,8 +473,10 @@ class DisplayController:
             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
+            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)
 
@@ -1749,11 +1761,12 @@ class DisplayController:
                 # 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.
+                # The lock-free read is a fast path only; it can be a false
+                # negative (the watcher setting the flag just after it is read
+                # is seen next iteration), never a false positive that loses a
+                # request.
                 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
+                    self._service_pending_reconcile()
 
                 if not self.available_modes:
                     # Nothing to render yet. Re-check _pending_plugin_reconcile
@@ -2813,7 +2826,8 @@ class DisplayController:
             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)
+        with self._plugin_modes_lock:
+            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
@@ -2847,7 +2861,8 @@ class DisplayController:
     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, [])
+        with self._plugin_modes_lock:
+            modes = self.plugin_display_modes.pop(plugin_id, [])
         for mode in modes:
             if mode in self.available_modes:
                 self.available_modes.remove(mode)
@@ -2892,6 +2907,67 @@ class DisplayController:
             }
         return enabled_map(old_config) != enabled_map(new_config)
 
+    def _service_pending_reconcile(self) -> None:
+        """Consume a pending reconcile request and run it.
+
+        The request is consumed BEFORE reconciling, not cleared after. Clearing
+        after would drop any config change that lands while reconcile is
+        running: reconcile has already read its config by then, so the clear
+        erases a request it never served and the newest config never
+        reconciles -- the same "your save did nothing" failure this whole path
+        exists to prevent. Consuming first means such a request stays set and
+        is picked up on the next pass.
+
+        A retryable failure (e.g. discovery) re-arms the flag.
+        """
+        with self._reconcile_flag_lock:
+            pending = self._pending_plugin_reconcile
+            self._pending_plugin_reconcile = False
+        if pending and not self._reconcile_enabled_plugins():
+            with self._reconcile_flag_lock:
+                self._pending_plugin_reconcile = True
+
+    def _enabled_plugin_not_running(self, new_config: Dict[str, Any]) -> bool:
+        """True when a discovered plugin is enabled in config but not running.
+
+        ``_enabled_set_changed`` compares only top-level ``enabled`` flags, which
+        misses the case that strands a plugin: one whose ``validate_config()``
+        returned False is absent from the running set, and the edit that fixes it
+        (enabling a league, filling in an API key) lives *nested* inside that
+        plugin's own section. No top-level flag changes, so no reconcile is
+        queued, and the save that should have fixed it appears to do nothing --
+        only toggling some unrelated plugin recovers it. hockey-scoreboard sat
+        enabled-but-absent on a live rig for four days this way.
+
+        Deliberately narrow: it fires only for ids the plugin manager has
+        actually discovered, so non-plugin sections that carry their own
+        ``enabled`` flag (``schedule``, ``display``, ...) don't queue a reconcile
+        on every save. In the steady state -- everything enabled is loaded --
+        this is False and costs nothing. That matters because reconcile runs
+        ``discover_plugins()`` on the render thread, where a needless
+        filesystem scan per config save would show up as a frame hitch.
+
+        Runs on the config-watcher thread, so both mappings it reads are
+        snapshotted under the lock that guards their writes.
+        """
+        if self.plugin_manager is None:
+            return False
+        # Two snapshots, each taken under its own lock and never nested, so a
+        # half-written mapping is never observed and this can't deadlock
+        # against discovery (which holds the discovery lock while rebuilding).
+        try:
+            known = self.plugin_manager.discovered_plugin_ids()
+        except AttributeError:
+            # Older manager without the accessor: fall back to a plain read.
+            known = set(getattr(self.plugin_manager, 'plugin_manifests', ()) or ())
+        with self._plugin_modes_lock:
+            running = set(self.plugin_display_modes)
+        for key, value in new_config.items():
+            if (key in known and isinstance(value, dict)
+                    and value.get('enabled', False) and key not in running):
+                return True
+        return False
+
     def _reconcile_enabled_plugins(self) -> bool:
         """Load/unload plugins so the running set matches the enabled set in
         config. Runs on the main display thread (never the config-watcher

src/plugin_system/plugin_manager.py

@@ -631,6 +631,17 @@ class PluginManager:
         
         return self.load_plugin(plugin_id)
     
+    def discovered_plugin_ids(self) -> set:
+        """Snapshot of the discovered plugin ids, taken under the discovery lock.
+
+        Callers on other threads (the config watcher) must not iterate
+        ``plugin_manifests`` directly: discovery rebuilds it entry by entry, so
+        an unsynchronised reader can see a half-populated mapping or raise
+        "dictionary changed size during iteration".
+        """
+        with self._discovery_lock:
+            return set(self.plugin_manifests)
+
     def get_plugin(self, plugin_id: str) -> Optional[Any]:
         """
         Get a loaded plugin instance by ID.

src/plugin_system/plugin_state.py

@@ -6,14 +6,40 @@ with state transitions and queries.
 """
 
 import threading
+import time
+from collections import deque
 from enum import Enum
-from typing import Optional, Dict, Any
+from typing import Optional, Dict, Any, Deque, List, Tuple
 from datetime import datetime
 import logging
 
 from src.logging_config import get_logger
 
 
+# The history is diagnostic only -- nothing reads the entries themselves, just
+# their count -- but it is appended to on the hot scheduling path: every update
+# cycle records RUNNING on reserve and ENABLED on finish. Unbounded, that is
+# 2,880 entries per plugin per day at the default 60s interval, which on a 1 GB
+# Pi exhausts memory in weeks.
+#
+# Two limits, because a single entry count answers the wrong question. What a
+# reader wants is "the last couple of hours", and how many transitions that is
+# depends entirely on the plugin's update interval -- which on a real board
+# spans 2s to 3600s. A flat 200 entries is 4.2 days for the slowest plugin and
+# 3.3 minutes for the fastest, so the plugin churning hardest, the one worth
+# looking at, keeps the least history.
+#
+# So: trim by AGE first, which makes the retained window comparable across
+# plugins whatever their cadence...
+STATE_HISTORY_MAX_AGE_SECONDS = 2 * 60 * 60
+
+# ...and cap by COUNT second, purely as a memory ceiling for the fast pollers
+# whose age window would otherwise run to thousands of entries. At ~230 bytes
+# an entry this is ~0.5 MB per plugin worst case, and only plugins updating
+# faster than roughly every 4s can reach it.
+MAX_STATE_HISTORY_PER_PLUGIN = 2000
+
+
 class PluginState(Enum):
     """Plugin state enumeration."""
     UNLOADED = "unloaded"  # Plugin not loaded
@@ -37,11 +63,43 @@ class PluginStateManager:
         self.logger = logger or get_logger(__name__)
         self._lock = threading.RLock()
         self._states: Dict[str, PluginState] = {}
-        self._state_history: Dict[str, list] = {}
+        # (monotonic timestamp, transition). The clock is monotonic so a DST
+        # shift or an NTP step cannot make entries look old and flush the
+        # history; the human-readable timestamp lives inside the transition.
+        self._state_history: Dict[str, Deque[Tuple[float, Dict[str, Any]]]] = {}
+        # Lifetime transition totals, kept separately so the count reported by
+        # get_state_info() stays truthful once the history above starts rolling.
+        self._state_transition_counts: Dict[str, int] = {}
         self._error_info: Dict[str, Dict[str, Any]] = {}
         self._last_update: Dict[str, datetime] = {}
         self._last_display: Dict[str, datetime] = {}
     
+    def _record_transition(
+        self,
+        plugin_id: str,
+        transition: Dict[str, Any]
+    ) -> None:
+        """Append a transition to the plugin's bounded history.
+
+        Callers must already hold ``_lock``. The deque discards its oldest
+        entry once it is full, so the history cannot grow without bound; the
+        lifetime total is tracked separately for get_state_info().
+        """
+        history = self._state_history.get(plugin_id)
+        if history is None:
+            history = deque(maxlen=MAX_STATE_HISTORY_PER_PLUGIN)
+            self._state_history[plugin_id] = history
+        now = time.monotonic()
+        history.append((now, transition))
+        # Age out first; the deque's maxlen is the backstop for plugins that
+        # produce more than the ceiling within the window.
+        cutoff = now - STATE_HISTORY_MAX_AGE_SECONDS
+        while history and history[0][0] < cutoff:
+            history.popleft()
+        self._state_transition_counts[plugin_id] = (
+            self._state_transition_counts.get(plugin_id, 0) + 1
+        )
+
     def set_state(
         self,
         plugin_id: str,
@@ -60,16 +118,13 @@ class PluginStateManager:
             old_state = self._states.get(plugin_id, PluginState.UNLOADED)
             self._states[plugin_id] = state
 
-            if plugin_id not in self._state_history:
-                self._state_history[plugin_id] = []
-
             transition = {
                 'timestamp': datetime.now(),
                 'from': old_state.value,
                 'to': state.value,
                 'error': str(error) if error else None
             }
-            self._state_history[plugin_id].append(transition)
+            self._record_transition(plugin_id, transition)
 
             # Store error info if transitioning to ERROR state
             if state == PluginState.ERROR and error:
@@ -126,17 +181,29 @@ class PluginStateManager:
         state = self.get_state(plugin_id)
         return state == PluginState.ENABLED
     
-    def get_state_history(self, plugin_id: str) -> list:
+    def get_state_history(self, plugin_id: str) -> List[Dict[str, Any]]:
         """
         Get state transition history for a plugin.
-        
+
+        Retention is by age first -- transitions older than
+        STATE_HISTORY_MAX_AGE_SECONDS are dropped -- and by count second, at
+        MAX_STATE_HISTORY_PER_PLUGIN, which only binds for plugins updating
+        fast enough to exceed it inside that window.
+
         Args:
             plugin_id: Plugin identifier
-            
+
         Returns:
-            List of state transitions
+            List of recent state transitions, oldest first. Both the list and
+            the transition dicts are copies, so callers cannot mutate the
+            manager's own history. The values inside a transition are all
+            immutable, so a shallow copy per entry is enough.
         """
-        return self._state_history.get(plugin_id, [])
+        with self._lock:
+            return [
+                dict(transition)
+                for _stamp, transition in self._state_history.get(plugin_id, ())
+            ]
     
     def set_error_info(self, plugin_id: str, error_info: Dict[str, Any]) -> None:
         """
@@ -179,9 +246,7 @@ class PluginStateManager:
             old_state = self._states.get(plugin_id, PluginState.UNLOADED)
             self._states[plugin_id] = state
 
-            if plugin_id not in self._state_history:
-                self._state_history[plugin_id] = []
-            self._state_history[plugin_id].append({
+            self._record_transition(plugin_id, {
                 'timestamp': datetime.now(),
                 'from': old_state.value,
                 'to': state.value,
@@ -241,26 +306,40 @@ class PluginStateManager:
         Returns:
             Dictionary with state information
         """
-        state = self.get_state(plugin_id)
-        info = {
-            'state': state.value,
-            'is_loaded': self.is_loaded(plugin_id),
-            'is_enabled': self.is_enabled(plugin_id),
-            'is_running': self.is_running(plugin_id),
-            'is_error': self.is_error(plugin_id),
-            'can_execute': self.can_execute(plugin_id),
-            'last_update': self.get_last_update(plugin_id),
-            'last_display': self.get_last_display(plugin_id),
-            'error_info': self.get_error_info(plugin_id),
-            'state_history_count': len(self.get_state_history(plugin_id))
-        }
+        # One snapshot, one critical section. Each field was read under its own
+        # lock, so an unload running concurrently could be observed half-done:
+        # 'state' read before clear_state() removed it and
+        # 'state_history_count' read after, giving a caller a plugin that is
+        # ENABLED with zero transitions. _lock is an RLock, so the helpers
+        # below can still take it.
+        with self._lock:
+            state = self.get_state(plugin_id)
+            info = {
+                'state': state.value,
+                'is_loaded': self.is_loaded(plugin_id),
+                'is_enabled': self.is_enabled(plugin_id),
+                'is_running': self.is_running(plugin_id),
+                'is_error': self.is_error(plugin_id),
+                'can_execute': self.can_execute(plugin_id),
+                'last_update': self.get_last_update(plugin_id),
+                'last_display': self.get_last_display(plugin_id),
+                'error_info': self.get_error_info(plugin_id),
+                'state_history_count': self._state_transition_counts.get(plugin_id, 0)
+            }
         return info
     
     def clear_state(self, plugin_id: str) -> None:
-        """Clear all state information for a plugin."""
-        self._states.pop(plugin_id, None)
-        self._state_history.pop(plugin_id, None)
-        self._error_info.pop(plugin_id, None)
-        self._last_update.pop(plugin_id, None)
-        self._last_display.pop(plugin_id, None)
+        """Clear all state information for a plugin.
+
+        Held under ``_lock`` so the five dicts are dropped as one unit: every
+        other mutator takes the lock, and without it a concurrent set_state()
+        could interleave and leave a plugin with history but no state.
+        """
+        with self._lock:
+            self._states.pop(plugin_id, None)
+            self._state_history.pop(plugin_id, None)
+            self._state_transition_counts.pop(plugin_id, None)
+            self._error_info.pop(plugin_id, None)
+            self._last_update.pop(plugin_id, None)
+            self._last_display.pop(plugin_id, None)
 

src/vegas_mode/coordinator.py

@@ -31,6 +31,18 @@ if TYPE_CHECKING:
 logger = logging.getLogger(__name__)
 
 
+#: Degradation threshold, as a fraction of target_fps. A marquee jitters a
+#: little all the time, so "anything under target" would report constantly and
+#: mean nothing; 90% of target is the point where a shortfall is real. At a
+#: 60fps target that is 54fps -- 55fps is a normal wobble and stays at DEBUG,
+#: which is deliberate, not an off-by-one.
+_FPS_HEALTHY_FRACTION = 0.9
+
+#: A healthy marquee still reports this often, so silence means stopped
+#: rather than fine.
+_FPS_HEARTBEAT_INTERVAL = 300.0
+
+
 def _percentile(ordered: List[float], fraction: float) -> float:
     """Nearest-rank percentile of an already-sorted list.
 
@@ -96,6 +108,11 @@ class VegasModeCoordinator:
         self._is_active = False
         self._is_paused = False
         self._should_stop = False
+        # Frame-rate health, tracked across run_iteration() calls so the
+        # heartbeat is one-per-interval rather than one-per-cycle, and so a
+        # recovery spanning two cycles is still reported. Reset on start().
+        self._fps_last_health_log = 0.0
+        self._fps_was_degraded = False
         self._state_lock = threading.Lock()
 
         # Live priority tracking
@@ -248,6 +265,11 @@ class VegasModeCoordinator:
             self._is_active = True
             self._should_stop = False
             self._start_time = time.time()
+            # A fresh run starts with a clean health slate: no stale
+            # "was degraded" from the previous run, and a heartbeat that is
+            # due immediately so the first sample confirms the marquee is up.
+            self._fps_last_health_log = 0.0
+            self._fps_was_degraded = False
 
         # Line up the next group immediately, so the first extension is already
         # warm rather than stalling the scroll to fetch it.
@@ -395,8 +417,18 @@ class VegasModeCoordinator:
             duration = self.render_pipeline.get_dynamic_duration()
         start_time = time.time()
         frame_count = 0
-        fps_log_interval = 5.0  # Log FPS every 5 seconds
-        last_fps_log_time = start_time
+        fps_log_interval = 5.0  # Sample FPS every 5 seconds
+        # Health state lives on the coordinator, not here: run_iteration() is
+        # called once per cycle, so locals reset every few seconds. That made
+        # `last_fps_health_log = 0.0` fire the "heartbeat" on the first sample
+        # of every iteration rather than once per interval, and a recovery
+        # that crossed an iteration boundary was never reported at all --
+        # was_degraded had already gone back to False.
+        # Monotonic, and deliberately not start_time: start_time is wall
+        # clock and is used below to report the iteration's duration. Mixing
+        # the two here would make every delta hugely negative and silence the
+        # frame-rate reporting altogether.
+        last_fps_log_time = time.monotonic()
         fps_frame_count = 0
         # A mean hides stutter completely. At 120fps a five-second window is
         # ~600 frames, so a 200ms freeze -- plainly visible on a marquee --
@@ -408,7 +440,13 @@ class VegasModeCoordinator:
         logger.info("Starting Vegas iteration for %.1fs", duration)
 
         while True:
-            frame_started = time.time()
+            # Monotonic, like the FPS window below. These devices have no RTC,
+            # so the wall clock jumps by however wrong boot time was the moment
+            # NTP first syncs. A backward jump makes frame_elapsed negative,
+            # and `frame_interval - frame_elapsed` then sleeps for longer than
+            # the whole budget -- the render loop stalls for the size of the
+            # correction. A forward jump inflates p99 and worst-frame instead.
+            frame_started = time.monotonic()
 
             # Check for STATIC mode plugin that should pause scroll
             static_plugin = self._check_static_plugin_trigger()
@@ -436,7 +474,7 @@ class VegasModeCoordinator:
             # quarter of the budget spent not rendering. Subtracting the work
             # already done keeps the pacing target while reclaiming that time,
             # and yields the GIL either way so other threads still run.
-            frame_elapsed = time.time() - frame_started
+            frame_elapsed = time.monotonic() - frame_started
             time.sleep(max(0.0, frame_interval - frame_elapsed))
 
             # Measured before the sleep: time spent working, not pacing.
@@ -448,16 +486,42 @@ class VegasModeCoordinator:
             frame_count += 1
             fps_frame_count += 1
 
-            # Periodic FPS logging
-            current_time = time.time()
+            # Periodic FPS logging. Reported at INFO only when the frame rate
+            # is actually worth an operator's attention -- a shortfall against
+            # target, or the recovery from one -- with a slow heartbeat so a
+            # healthy marquee still shows a pulse.
+            #
+            # Measured over two hours on a running rig: 1410 samples, 98.5%
+            # of them within 10% of target. The 1.5% that were not included a
+            # reading of 8.6fps against a target of 60 -- a real stall, and
+            # completely invisible inside 1389 lines reading "59.6".
+            # Monotonic: every use of this value in the block below is a
+            # duration, and these devices have no RTC, so the wall clock jumps
+            # by however wrong boot time was the moment NTP first syncs. That
+            # would not only mis-fire the heartbeat, it would corrupt the
+            # frame rate itself, since fps is frames divided by this delta.
+            current_time = time.monotonic()
             if current_time - last_fps_log_time >= fps_log_interval:
                 fps = fps_frame_count / (current_time - last_fps_log_time)
                 p99 = _percentile(sorted(frame_times), 0.99)
-                logger.info(
-                    "Vegas FPS: %.1f (target: %d, frames: %d) p99 %.1fms worst %.1fms",
-                    fps, self.vegas_config.target_fps, fps_frame_count,
-                    p99 * 1000.0, frame_worst * 1000.0
-                )
+                target = self.vegas_config.target_fps
+                degraded = target > 0 and fps < target * _FPS_HEALTHY_FRACTION
+                due = (current_time - self._fps_last_health_log
+                       >= _FPS_HEARTBEAT_INTERVAL)
+                if degraded or self._fps_was_degraded or due:
+                    logger.info(
+                        "Vegas FPS: %.1f (target: %d, frames: %d) p99 %.1fms worst %.1fms",
+                        fps, target, fps_frame_count,
+                        p99 * 1000.0, frame_worst * 1000.0
+                    )
+                    self._fps_last_health_log = current_time
+                else:
+                    logger.debug(
+                        "Vegas FPS: %.1f (target: %d, frames: %d) p99 %.1fms worst %.1fms",
+                        fps, target, fps_frame_count,
+                        p99 * 1000.0, frame_worst * 1000.0
+                    )
+                self._fps_was_degraded = degraded
                 last_fps_log_time = current_time
                 fps_frame_count = 0
                 frame_worst = 0.0

src/web_interface/secret_helpers.py

@@ -5,7 +5,7 @@ Provides functions for identifying, masking, separating, and filtering
 secret fields in plugin configurations based on JSON Schema x-secret markers.
 """
 
-from typing import Any, Dict, Set, Tuple
+from typing import Any, Dict, Optional, Set, Tuple
 
 
 def find_secret_fields(properties: Dict[str, Any], prefix: str = '') -> Set[str]:
@@ -202,11 +202,89 @@ def remove_empty_secrets(secrets: Dict[str, Any]) -> Dict[str, Any]:
             nested = remove_empty_secrets(v)
             if nested:
                 result[k] = nested
+        elif isinstance(v, list):
+            # Lists used to fall through to the scalar branch below and be
+            # kept verbatim, blanks and all. Because lists merge by
+            # *replacement*, saving any unrelated setting then wrote
+            # [{"token": ""}, ...] straight over the stored list and
+            # destroyed every credential in it.
+            pruned = _prune_secret_list(v)
+            if pruned is not None:
+                result[k] = pruned
         elif v is not None and not (isinstance(v, str) and v.strip() == ''):
             result[k] = v
     return result
 
 
+def _prune_secret_list(items: list) -> Optional[list]:
+    """Strip blanks from inside a list of secrets, preserving every index.
+
+    The rest of the system treats a secrets list as *parallel* to the regular
+    one -- ``sec[i]`` holds the secret fields of item ``i``, and ``{}`` means
+    "item i has none" (see ConfigManager._strip_secrets_recursive). So an
+    emptied dict item stays ``{}``: putting ``None`` there makes that list stop
+    looking parallel, and the stripper then drops the whole key from the main
+    config, taking the non-secret fields with it.
+
+    A blank *scalar* becomes ``None``, meaning "no update at this index" --
+    :func:`merge_secrets` substitutes whatever is stored there. Returns
+    ``None`` when nothing in the list carries a real value, so the caller drops
+    the key and leaves the stored list untouched.
+    """
+    pruned: list = []
+    has_real_value = False
+    for item in items:
+        if isinstance(item, dict):
+            kept = remove_empty_secrets(item)
+            pruned.append(kept)
+            has_real_value = has_real_value or bool(kept)
+        elif isinstance(item, list):
+            sub = _prune_secret_list(item)
+            pruned.append(sub if sub is not None else [])
+            has_real_value = has_real_value or sub is not None
+        elif item is not None and not (isinstance(item, str) and item.strip() == ''):
+            pruned.append(item)
+            has_real_value = True
+        else:
+            pruned.append(None)
+    return pruned if has_real_value else None
+
+
+def merge_secrets(stored: Any, incoming: Any) -> Any:
+    """Merge submitted secrets over stored ones, element-wise inside lists.
+
+    ``deep_merge`` replaces a list wholesale. For secrets that is destructive:
+    an incoming list that carries a real value for one entry and ``None`` for
+    the rest would drop the stored credentials of every other entry. Here a
+    list merges by index, and ``None`` means "keep what is stored".
+
+    Entries are matched by *position*, which is what the config form gives us
+    -- there is no schema-declared identity to key on, and it is the same
+    contract ConfigManager._strip_secrets_recursive already relies on. The
+    incoming list's length wins, so deleting an item deletes its secrets;
+    an item the client left blank keeps whatever is stored at that index.
+    """
+    if isinstance(stored, dict) and isinstance(incoming, dict):
+        merged = dict(stored)
+        for key, value in incoming.items():
+            merged[key] = (merge_secrets(stored[key], value)
+                           if key in stored else value)
+        return merged
+    if isinstance(stored, list) and isinstance(incoming, list):
+        # The incoming list sets the length -- the regular config's list is
+        # authoritative about how many items exist, and this one runs parallel
+        # to it. Removing an entry must therefore remove its secrets too.
+        merged_list = []
+        for index, item in enumerate(incoming):
+            stored_item = stored[index] if index < len(stored) else None
+            merged_list.append(stored_item if item is None
+                               else merge_secrets(stored_item, item))
+        return merged_list
+    if incoming is None:
+        return stored
+    return incoming
+
+
 def strip_masked_values(secrets: Dict[str, Any]) -> Dict[str, Any]:
     """Remove values a client echoed back rather than changed.
 

test/test_api_v3_non_finite_numbers.py

@@ -0,0 +1,85 @@
+"""Non-finite JSON numbers must be rejected, not raise.
+
+json.loads accepts Infinity/-Infinity/NaN by default (they are not valid JSON,
+but Python's parser emits them) and Flask's get_json passes them straight
+through. int(float('inf')) raises OverflowError, which is neither ValueError
+nor TypeError -- so validation blocks that carefully caught those let it
+through and Flask turned it into a 500.
+
+The damage was not the status code. /config/dim-schedule answered with
+CONFIG_SAVE_FAILED and suggested "Check file permissions on config directory"
+and "Check available disk space" for what was actually an invalid number.
+
+NaN already returned 400 (int(nan) raises ValueError), which is why this only
+showed up for the infinities.
+"""
+import sys
+from pathlib import Path
+
+import pytest
+
+sys.path.insert(0, str(Path(__file__).parent.parent))
+
+from test._api_v3_test_helpers import api_v3_client, api_v3_module  # noqa: F401,E402
+
+
+#: (route, field) that returned 500 before OverflowError was caught. Both
+#: infinity signs are exercised: int() raises OverflowError for either, but
+#: only one of them was in the original report, and a guard that special-cased
+#: the sign would pass a one-sided test.
+NON_FINITE_ROUTES = [
+    ('/api/v3/config/dim-schedule', 'dim_brightness'),
+    ('/api/v3/errors/clear', 'max_age_hours'),
+    ('/api/v3/config/main', 'multiplexing'),
+    ('/api/v3/config/main', 'row_address_type'),
+]
+NON_FINITE_CASES = [
+    (route, '{"%s": %s}' % (field, literal))
+    for route, field in NON_FINITE_ROUTES
+    for literal in ('Infinity', '-Infinity')
+]
+
+
+@pytest.mark.parametrize("route,body", NON_FINITE_CASES)
+def test_infinity_is_a_client_error_not_a_server_error(api_v3_client, route, body):
+    """Exactly 400, not merely "some 4xx".
+
+    Accepting any 4xx would let a 404 pass, so renaming one of these routes
+    would leave the test green while testing nothing -- the failure mode this
+    whole file exists to catch.
+    """
+    response = api_v3_client.post(route, data=body, content_type='application/json')
+    assert response.status_code == 400, (
+        f"{route} with {body} answered {response.status_code}; expected 400"
+    )
+
+
+@pytest.mark.parametrize("route,body", [
+    ('/api/v3/config/dim-schedule', '{"dim_brightness": NaN}'),
+    ('/api/v3/errors/clear', '{"max_age_hours": NaN}'),
+])
+def test_nan_is_also_a_client_error(api_v3_client, route, body):
+    """int(nan) raises ValueError so this path already worked -- pinned so a
+    refactor that narrows the except tuple cannot quietly break it."""
+    response = api_v3_client.post(route, data=body, content_type='application/json')
+    assert response.status_code == 400
+
+
+def test_a_valid_number_is_accepted(api_v3_client, api_v3_module, monkeypatch):
+    """Prove the widened except did not start swallowing ordinary input.
+
+    Asserting "not a 400" would not show that: the mocked save path fails for
+    any input, so the assertion would hold even if validation had rejected the
+    value. Give load_config a real dict and stub the atomic save, and the
+    endpoint reaches its success response -- which only happens if 30 passed
+    validation.
+    """
+    api_v3_module.api_v3.config_manager.load_config.return_value = {}
+    monkeypatch.setattr(api_v3_module, '_save_config_atomic',
+                        lambda *a, **k: (True, ''))
+    response = api_v3_client.post(
+        '/api/v3/config/dim-schedule',
+        data='{"dim_brightness": 30}',
+        content_type='application/json',
+    )
+    assert response.status_code == 200, response.get_data(as_text=True)[:200]

test/test_background_fetch_dedupe.py

@@ -1,297 +0,0 @@
-"""A second request for a key already being fetched must join, not duplicate.
-
-request_id embeds a millisecond timestamp and active_requests is keyed by it,
-so every submit looked new and nothing compared what was actually being
-fetched. On a real board the season-schedule cache_key is requested by both
-the Recent and the Upcoming manager: they miss the cache in the same
-millisecond and each start a full download and parse of the same payload.
-Measured on a running board, 138 background fetches in 24 hours arriving in
-pairs at identical timestamps -- half of them redundant.
-
-The cost of a duplicate is a second download, a second JSON parse (the
-expensive part on a Pi), and a second parsed copy resident at the same time.
-Schedules on that board run from 256KB to 20MB. It also consumes a second of
-the three executor slots with identical work, which is what makes two large
-parses peak simultaneously.
-"""
-
-import threading
-import time
-from unittest.mock import MagicMock, Mock, patch
-
-import pytest
-
-from src.background_data_service import BackgroundDataService
-
-
-PAYLOAD = {"events": [{"id": f"g{i}"} for i in range(20)]}
-
-
-@pytest.fixture
-def cache():
-    m = MagicMock()
-    m.get.return_value = None          # always a miss: force the fetch path
-    m.set.return_value = None
-    return m
-
-
-@pytest.fixture
-def service(cache):
-    svc = BackgroundDataService(cache, max_workers=3, request_timeout=5)
-    yield svc
-    svc.shutdown(wait=False)
-
-
-def _resp():
-    r = Mock()
-    r.json.return_value = PAYLOAD
-    r.raise_for_status.return_value = None
-    return r
-
-
-def _wait(service, req_id, timeout=5):
-    deadline = time.time() + timeout
-    while not service.is_request_complete(req_id) and time.time() < deadline:
-        time.sleep(0.02)
-
-
-class _BlockingSession:
-    """Holds the first fetch open so a second can be submitted mid-flight."""
-
-    def __init__(self):
-        self.calls = 0
-        self.release = threading.Event()
-        self.started = threading.Event()
-
-    def get(self, *a, **k):
-        self.calls += 1
-        self.started.set()
-        self.release.wait(timeout=5)
-        return _resp()
-
-
-def test_a_second_submit_for_the_same_key_does_not_fetch_twice(service):
-    session = _BlockingSession()
-    with patch.object(service, "session", session):
-        first = service.submit_fetch_request(
-            sport="nba", year=2026, url="https://x/s", cache_key="nba_2026",
-            callback=lambda r: None, max_retries=0)
-        assert session.started.wait(timeout=5)
-
-        second = service.submit_fetch_request(
-            sport="nba", year=2026, url="https://x/s", cache_key="nba_2026",
-            callback=lambda r: None, max_retries=0)
-
-        assert second == first, "the joiner should share the in-flight request id"
-        session.release.set()
-        _wait(service, first)
-
-    assert session.calls == 1, f"the payload was fetched {session.calls} times"
-
-
-def test_the_joiner_still_gets_its_callback(service):
-    session = _BlockingSession()
-    seen = []
-    with patch.object(service, "session", session):
-        first = service.submit_fetch_request(
-            sport="nba", year=2026, url="https://x/s", cache_key="k",
-            callback=lambda r: seen.append("first"), max_retries=0)
-        assert session.started.wait(timeout=5)
-        joined = service.submit_fetch_request(
-            sport="nba", year=2026, url="https://x/s", cache_key="k",
-            callback=lambda r: seen.append("second"), max_retries=0)
-        # Assert the coalescing happened, otherwise this passes trivially:
-        # two independent requests would each fire their own callback and the
-        # test would say nothing about the joined path.
-        assert joined == first
-        session.release.set()
-        _wait(service, first)
-
-    deadline = time.time() + 5
-    while len(seen) < 2 and time.time() < deadline:
-        time.sleep(0.02)
-    assert sorted(seen) == ["first", "second"], (
-        f"both submitters must be called back, got {seen}")
-
-
-def test_one_callback_raising_does_not_silence_the_other(service):
-    session = _BlockingSession()
-    seen = []
-
-    def boom(result):
-        raise RuntimeError("consumer blew up")
-
-    with patch.object(service, "session", session):
-        first = service.submit_fetch_request(
-            sport="nba", year=2026, url="https://x/s", cache_key="k",
-            callback=boom, max_retries=0)
-        assert session.started.wait(timeout=5)
-        joined = service.submit_fetch_request(
-            sport="nba", year=2026, url="https://x/s", cache_key="k",
-            callback=lambda r: seen.append("survivor"), max_retries=0)
-        # Same reason: without coalescing these are separate requests and
-        # neither callback can affect the other.
-        assert joined == first
-        session.release.set()
-        _wait(service, first)
-
-    deadline = time.time() + 5
-    while not seen and time.time() < deadline:
-        time.sleep(0.02)
-    assert seen == ["survivor"]
-
-
-def test_different_keys_are_not_coalesced(service):
-    session = _BlockingSession()
-    with patch.object(service, "session", session):
-        a = service.submit_fetch_request(
-            sport="nba", year=2026, url="https://x/a", cache_key="key_a",
-            callback=lambda r: None, max_retries=0)
-        assert session.started.wait(timeout=5)
-        b = service.submit_fetch_request(
-            sport="nhl", year=2026, url="https://x/b", cache_key="key_b",
-            callback=lambda r: None, max_retries=0)
-        assert a != b, "different cache keys must not share a request"
-        session.release.set()
-        _wait(service, a)
-        _wait(service, b)
-    assert session.calls == 2
-
-
-def test_a_later_submit_after_completion_fetches_again(service):
-    """Dedupe is for concurrent requests only, not a second cache layer."""
-    with patch.object(service.session, "get", side_effect=[_resp(), _resp()]) as get:
-        first = service.submit_fetch_request(
-            sport="nba", year=2026, url="https://x/s", cache_key="k",
-            callback=lambda r: None, max_retries=0)
-        _wait(service, first)
-        second = service.submit_fetch_request(
-            sport="nba", year=2026, url="https://x/s", cache_key="k",
-            callback=lambda r: None, max_retries=0)
-        _wait(service, second)
-    assert first != second
-    assert get.call_count == 2
-
-
-def test_cancelling_releases_the_key(service):
-    """A cancelled request must not wedge its key against future fetches."""
-    session = _BlockingSession()
-    with patch.object(service, "session", session):
-        first = service.submit_fetch_request(
-            sport="nba", year=2026, url="https://x/s", cache_key="k",
-            callback=lambda r: None, max_retries=0)
-        assert session.started.wait(timeout=5)
-        service.cancel_request(first)
-        assert "k" not in service._inflight_by_cache_key
-        session.release.set()
-
-
-def test_a_stranded_index_entry_cannot_wedge_a_key(service):
-    """Defensive: the request is looked up, not trusted from the id alone."""
-    service._inflight_by_cache_key["ghost"] = "no_such_request"
-    with patch.object(service.session, "get", return_value=_resp()):
-        req = service.submit_fetch_request(
-            sport="nba", year=2026, url="https://x/s", cache_key="ghost",
-            callback=lambda r: None, max_retries=0)
-        _wait(service, req)
-    assert service.get_result(req).success is True
-
-
-def test_the_deduplicated_count_is_reported(service):
-    session = _BlockingSession()
-    with patch.object(service, "session", session):
-        first = service.submit_fetch_request(
-            sport="nba", year=2026, url="https://x/s", cache_key="k",
-            callback=lambda r: None, max_retries=0)
-        assert session.started.wait(timeout=5)
-        service.submit_fetch_request(
-            sport="nba", year=2026, url="https://x/s", cache_key="k",
-            max_retries=0)
-        session.release.set()
-        _wait(service, first)
-    assert service.get_statistics().get("deduplicated_requests") == 1
-
-
-def test_a_cancelled_worker_cannot_overwrite_its_replacement(service, cache):
-    """Cancelling frees the key, so a replacement may already own it.
-
-    The worker cannot abort an HTTP call in flight, so when the cancelled one
-    finally returns it must discard its response rather than write it. Without
-    that, the sequence is: cancel A, submit B for the same key, B fetches and
-    caches fresh data, A returns and overwrites it with the response nobody
-    wanted -- and calls A's callbacks too.
-    """
-    slow = _BlockingSession()
-    stale = {"events": [{"id": "STALE"}]}
-    slow_resp = Mock()
-    slow_resp.json.return_value = stale
-    slow_resp.raise_for_status.return_value = None
-
-    def blocked_get(*a, **k):
-        slow.calls += 1
-        slow.started.set()
-        slow.release.wait(timeout=5)
-        return slow_resp
-
-    called = []
-    with patch.object(service.session, "get", side_effect=blocked_get):
-        first = service.submit_fetch_request(
-            sport="nba", year=2026, url="https://x/s", cache_key="k",
-            callback=lambda r: called.append("cancelled_one"), max_retries=0)
-        assert slow.started.wait(timeout=5)
-
-        service.cancel_request(first)
-        assert "k" not in service._inflight_by_cache_key
-
-    # The replacement writes the fresh value while the cancelled fetch is held.
-    fresh = {"events": [{"id": "FRESH"}]}
-    fresh_resp = Mock()
-    fresh_resp.json.return_value = fresh
-    fresh_resp.raise_for_status.return_value = None
-    with patch.object(service.session, "get", return_value=fresh_resp):
-        second = service.submit_fetch_request(
-            sport="nba", year=2026, url="https://x/s", cache_key="k",
-            callback=lambda r: called.append("replacement"), max_retries=0)
-        _wait(service, second)
-
-    assert cache.set.call_args[0][1] == fresh, "replacement must own the cache"
-
-    # Now let the cancelled fetch finish. It must write nothing and call nobody.
-    # Wait for the worker to actually finish rather than sleeping: a fixed
-    # sleep is a race under load, and a slow worker would make this pass for
-    # the wrong reason. A cancelled request is still filed in
-    # completed_requests, so that is the signal it has run to completion.
-    writes_before = cache.set.call_count
-    slow.release.set()
-    deadline = time.time() + 5
-    while first not in service.completed_requests and time.time() < deadline:
-        time.sleep(0.02)
-    assert first in service.completed_requests, "cancelled worker never finished"
-
-    assert cache.set.call_count == writes_before, (
-        "the cancelled worker wrote to the cache after its replacement")
-    assert cache.set.call_args[0][1] == fresh, "stale data overwrote fresh"
-    assert "cancelled_one" not in called, (
-        "a cancelled request must not deliver callbacks")
-
-
-def test_request_ids_are_unique_within_a_millisecond(service):
-    """request_id was sport_year_milliseconds, which collides.
-
-    Two submits inside the same millisecond produced the SAME id, so one
-    silently replaced the other in active_requests and completed_requests.
-    Dedupe hands this id back to every joiner as their handle for
-    get_result(), so uniqueness is now load-bearing rather than incidental.
-    """
-    # Stub the executor rather than the session: this is about what submit
-    # hands back, and letting 50 workers loose would outlive the patch and
-    # make real network calls.
-    with patch.object(service.executor, "submit"):
-        ids = [
-            service.submit_fetch_request(
-                sport="nba", year=2026, url="https://x/s",
-                cache_key=f"key_{i}",           # distinct keys: no dedupe
-                callback=lambda r: None, max_retries=0)
-            for i in range(50)
-        ]
-    assert len(set(ids)) == len(ids), "request ids collided"

test/test_display_controller_plugin_toggle.py

@@ -6,6 +6,7 @@ These tests cover the reconcile path that loads/unloads plugins and rebuilds
 the dispatch maps on the main thread when the enabled set changes.
 """
 
+import copy
 from unittest.mock import MagicMock
 
 
@@ -253,3 +254,182 @@ class TestEnabledSetChanged:
             {"a": {"enabled": True, "duration": 30}},
             {"a": {"enabled": True, "duration": 45}},
         ) is False
+
+
+class TestEnabledPluginNotRunning:
+    """A plugin that fails validate_config() is enabled but absent, and the
+    config edit that fixes it is nested inside the plugin's own section -- so
+    the top-level ``enabled`` comparison never sees it. These cover the second
+    gate that queues a reconcile in that case.
+    """
+
+    def test_nested_edit_is_invisible_to_the_enabled_set_check(self, test_display_controller):
+        """The original gate: proves why a second one is needed."""
+        controller = test_display_controller
+        old = {"hockey-scoreboard": {"enabled": True, "nhl": {"enabled": False}}}
+        new = {"hockey-scoreboard": {"enabled": True, "nhl": {"enabled": True}}}
+        # Enabling a league changes no top-level flag.
+        assert controller._enabled_set_changed(old, new) is False
+
+    def test_queues_reconcile_when_enabled_plugin_is_absent(self, test_display_controller):
+        controller = test_display_controller
+        controller.plugin_manager.plugin_manifests = {"hockey-scoreboard": {}}
+        controller.plugin_manager.discovered_plugin_ids.return_value = {"hockey-scoreboard"}
+        controller.plugin_display_modes = {}  # failed to load
+        cfg = {"hockey-scoreboard": {"enabled": True, "nhl": {"enabled": True}}}
+        assert controller._enabled_plugin_not_running(cfg) is True
+
+    def test_quiet_when_every_enabled_plugin_is_running(self, test_display_controller):
+        controller = test_display_controller
+        controller.plugin_manager.plugin_manifests = {"hockey-scoreboard": {}}
+        controller.plugin_manager.discovered_plugin_ids.return_value = {"hockey-scoreboard"}
+        controller.plugin_display_modes = {"hockey-scoreboard": ["nhl"]}
+        cfg = {"hockey-scoreboard": {"enabled": True}}
+        assert controller._enabled_plugin_not_running(cfg) is False
+
+    def test_disabled_plugin_does_not_queue(self, test_display_controller):
+        controller = test_display_controller
+        controller.plugin_manager.plugin_manifests = {"hockey-scoreboard": {}}
+        controller.plugin_manager.discovered_plugin_ids.return_value = {"hockey-scoreboard"}
+        controller.plugin_display_modes = {}
+        cfg = {"hockey-scoreboard": {"enabled": False}}
+        assert controller._enabled_plugin_not_running(cfg) is False
+
+    def test_non_plugin_sections_do_not_queue(self, test_display_controller):
+        """``schedule``/``display`` carry their own ``enabled`` and are never
+        in plugin_display_modes -- without the manifest check they would queue
+        a reconcile, and therefore a filesystem scan, on every config save."""
+        controller = test_display_controller
+        controller.plugin_manager.plugin_manifests = {"hockey-scoreboard": {}}
+        controller.plugin_manager.discovered_plugin_ids.return_value = {"hockey-scoreboard"}
+        controller.plugin_display_modes = {"hockey-scoreboard": ["nhl"]}
+        cfg = {
+            "hockey-scoreboard": {"enabled": True},
+            "schedule": {"enabled": True},
+            "display": {"enabled": True},
+        }
+        assert controller._enabled_plugin_not_running(cfg) is False
+
+    def test_non_dict_section_is_ignored(self, test_display_controller):
+        controller = test_display_controller
+        controller.plugin_manager.plugin_manifests = {"hockey-scoreboard": {}}
+        controller.plugin_manager.discovered_plugin_ids.return_value = {"hockey-scoreboard"}
+        controller.plugin_display_modes = {}
+        assert controller._enabled_plugin_not_running({"hockey-scoreboard": "nonsense"}) is False
+
+    def test_no_plugin_manager_is_quiet(self, test_display_controller):
+        controller = test_display_controller
+        controller.plugin_manager = None
+        assert controller._enabled_plugin_not_running({"x": {"enabled": True}}) is False
+
+
+class TestReconcileQueuedThroughSubscriber:
+    """End-to-end through the real config-change subscriber, not the helper.
+
+    Without the second gate this is the four-day-outage path: the plugin is
+    enabled, absent, and the save that enables its league sets no flag.
+    """
+
+    @staticmethod
+    def _subscriber(controller):
+        subs = controller.config_service._subscribers['*']
+        for cb in subs:
+            if getattr(cb, '__name__', '') == '_controller_config_change':
+                return cb
+        raise AssertionError(f"controller subscriber not found among {subs}")
+
+    @staticmethod
+    def _configs(controller, plugin_section_old, plugin_section_new):
+        """Build two full configs differing only inside the plugin section --
+        the subscriber refreshes its cache from these, so they must be real."""
+        base = copy.deepcopy(controller.config)
+        old = copy.deepcopy(base)
+        new = copy.deepcopy(base)
+        old["hockey-scoreboard"] = plugin_section_old
+        new["hockey-scoreboard"] = plugin_section_new
+        return old, new
+
+    def test_nested_edit_queues_reconcile_for_absent_plugin(self, test_display_controller):
+        controller = test_display_controller
+        controller.plugin_manager.plugin_manifests = {"hockey-scoreboard": {}}
+        controller.plugin_manager.discovered_plugin_ids.return_value = {"hockey-scoreboard"}
+        controller.plugin_display_modes = {}          # validate_config() said False
+        controller._pending_plugin_reconcile = False
+
+        old, new = self._configs(
+            controller,
+            {"enabled": True, "nhl": {"enabled": False}},
+            {"enabled": True, "nhl": {"enabled": True}},
+        )
+        # The original gate is blind to this edit ...
+        assert controller._enabled_set_changed(old, new) is False
+        self._subscriber(controller)(old, new)
+        # ... but the reconcile is queued anyway.
+        assert controller._pending_plugin_reconcile is True
+
+    def test_steady_state_does_not_queue_reconcile(self, test_display_controller):
+        """Everything enabled is running: an unrelated edit must not queue a
+        reconcile, or every config save drags a filesystem scan onto the
+        render thread."""
+        controller = test_display_controller
+        controller.plugin_manager.plugin_manifests = {"hockey-scoreboard": {}}
+        controller.plugin_manager.discovered_plugin_ids.return_value = {"hockey-scoreboard"}
+        controller.plugin_display_modes = {"hockey-scoreboard": ["nhl"]}
+        controller._pending_plugin_reconcile = False
+
+        old, new = self._configs(
+            controller,
+            {"enabled": True, "scroll_speed": 1},
+            {"enabled": True, "scroll_speed": 2},
+        )
+        self._subscriber(controller)(old, new)
+
+        assert controller._pending_plugin_reconcile is False
+
+
+class TestPendingReconcileNotLost:
+    """A config change arriving *during* reconcile must not be discarded.
+
+    The flag used to be cleared after a successful reconcile. Reconcile has
+    already read its config by then, so that clear erased a request it never
+    served and the newest config never reconciled -- the same "my save did
+    nothing" symptom this path exists to prevent.
+    """
+
+    def test_request_arriving_during_reconcile_survives(self, test_display_controller):
+        controller = test_display_controller
+        controller._pending_plugin_reconcile = True
+
+        def reconcile_and_race():
+            # The watcher thread queues another change while we are mid-flight.
+            with controller._reconcile_flag_lock:
+                controller._pending_plugin_reconcile = True
+            return True
+
+        controller._reconcile_enabled_plugins = reconcile_and_race
+        controller._service_pending_reconcile()
+
+        assert controller._pending_plugin_reconcile is True, \
+            "a config change landing during reconcile was discarded"
+
+    def test_flag_cleared_on_a_quiet_success(self, test_display_controller):
+        controller = test_display_controller
+        controller._pending_plugin_reconcile = True
+        controller._reconcile_enabled_plugins = lambda: True
+        controller._service_pending_reconcile()
+        assert controller._pending_plugin_reconcile is False
+
+    def test_retryable_failure_rearms(self, test_display_controller):
+        controller = test_display_controller
+        controller._pending_plugin_reconcile = True
+        controller._reconcile_enabled_plugins = lambda: False
+        controller._service_pending_reconcile()
+        assert controller._pending_plugin_reconcile is True
+
+    def test_no_reconcile_when_nothing_pending(self, test_display_controller):
+        controller = test_display_controller
+        controller._pending_plugin_reconcile = False
+        calls = []
+        controller._reconcile_enabled_plugins = lambda: calls.append(1) or True
+        controller._service_pending_reconcile()
+        assert calls == []

test/test_install_lowmem.py

@@ -13,6 +13,7 @@ need root and mutate the system, so they are exercised manually instead.
 """
 
 import subprocess
+import tempfile
 from pathlib import Path
 
 import pytest
@@ -31,6 +32,16 @@ def run_lib(snippet: str, env: dict | None = None) -> subprocess.CompletedProces
     )
 
 
+def _fstype_of(path: object) -> str:
+    """Filesystem type backing ``path``, via the same tool the helper uses."""
+    result = subprocess.run(
+        ["findmnt", "-no", "FSTYPE", "--target", str(path)],
+        capture_output=True, text=True,
+        env={"PATH": "/usr/bin:/bin:/usr/sbin:/sbin"},
+    )
+    return result.stdout.strip()
+
+
 def call(fn: str, *args: object, env: dict | None = None) -> str:
     joined = " ".join(str(a) for a in args)
     result = run_lib(f"{fn} {joined}", env=env)
@@ -195,8 +206,29 @@ class TestOomDetection:
 
 class TestDiskBackedTmpdir:
     def test_returns_nothing_when_tmpdir_is_already_disk_backed(self, tmp_path):
-        # tmp_path is on the regular filesystem, so the default must be kept.
-        assert call("lm_disk_backed_tmpdir", env={"TMPDIR": str(tmp_path)}) == ""
+        # Do not assume tmp_path is disk-backed. Debian 13 -- the platform this
+        # helper exists for -- mounts /tmp as tmpfs, and pytest puts tmp_path
+        # under /tmp, so this asserted against a *memory*-backed directory and
+        # failed on the target platform while the helper behaved exactly as
+        # designed. Search for a directory whose backing store is really disk.
+        scratch = None
+        disk_backed = None
+        for candidate in (tmp_path, Path("/var/tmp"), LIB.parent):
+            if _fstype_of(candidate) not in ("tmpfs", "ramfs", ""):
+                if candidate is tmp_path:
+                    disk_backed = candidate
+                else:
+                    scratch = Path(tempfile.mkdtemp(dir=str(candidate)))
+                    disk_backed = scratch
+                break
+        if disk_backed is None:
+            pytest.skip("no disk-backed directory available to test against")
+        try:
+            assert call("lm_disk_backed_tmpdir",
+                        env={"TMPDIR": str(disk_backed)}) == ""
+        finally:
+            if scratch is not None:
+                scratch.rmdir()
 
     def test_redirects_away_from_a_memory_backed_tmpdir(self):
         # Debian 13 mounts /tmp as tmpfs, which would otherwise hold the whole

test/test_logging_config.py

@@ -89,11 +89,17 @@ class TestContextualFormatter:
         assert "hello" in out
 
     def test_location_toggle(self):
+        # Assert on the whole "module.func:lineno" token, not a bare ":42".
+        # The formatted line starts with an HH:MM:SS timestamp, so a bare
+        # ":{lineno}" also matches the clock whenever the minute or second
+        # happens to equal the line number -- about 3% of runs, which is a
+        # flaky failure with nothing wrong.
         record = make_record()
+        location = f"{record.module}.{record.funcName}:{record.lineno}"
         with_loc = ContextualFormatter(include_location=True).format(record)
         without = ContextualFormatter(include_location=False).format(record)
-        assert f":{record.lineno}" in with_loc
-        assert f":{record.lineno}" not in without
+        assert location in with_loc
+        assert location not in without
 
     def test_record_not_mutated_no_double_prefix(self):
         # Regression: a record is formatted once PER HANDLER. The formatter

test/test_plugin_manager_discovered_ids.py

@@ -0,0 +1,63 @@
+"""Tests for PluginManager.discovered_plugin_ids().
+
+The config-watcher thread needs the set of discovered plugin ids while the
+render thread may be rebuilding plugin_manifests. Iterating that dict directly
+can observe a half-populated mapping or raise "dictionary changed size during
+iteration", so the accessor snapshots it under the discovery lock.
+"""
+
+import tempfile
+import threading
+from pathlib import Path
+
+import pytest
+
+from src.plugin_system.plugin_manager import PluginManager
+
+
+@pytest.fixture
+def pm():
+    with tempfile.TemporaryDirectory() as tmp:
+        yield PluginManager(plugins_dir=str(Path(tmp) / "plugins"))
+
+
+def test_returns_the_discovered_ids(pm):
+    pm.plugin_manifests = {"clock-simple": {}, "hockey-scoreboard": {}}
+    assert pm.discovered_plugin_ids() == {"clock-simple", "hockey-scoreboard"}
+
+
+def test_empty_when_nothing_discovered(pm):
+    pm.plugin_manifests = {}
+    assert pm.discovered_plugin_ids() == set()
+
+
+def test_is_a_snapshot_not_a_live_view(pm):
+    """The caller iterates the result on another thread; it must not alias
+    the mapping discovery is still writing to."""
+    pm.plugin_manifests = {"clock-simple": {}}
+    snapshot = pm.discovered_plugin_ids()
+    pm.plugin_manifests["hockey-scoreboard"] = {}
+    assert snapshot == {"clock-simple"}
+
+
+def test_takes_the_discovery_lock(pm):
+    """Guards against the lock being dropped in a later refactor: with the
+    lock held by another thread the call must block rather than read."""
+    pm.plugin_manifests = {"clock-simple": {}}
+    finished = threading.Event()
+
+    def call():
+        pm.discovered_plugin_ids()
+        finished.set()
+
+    pm._discovery_lock.acquire()
+    try:
+        # RLock is reentrant per-thread, so use a *different* thread to prove
+        # the accessor actually waits on it.
+        t = threading.Thread(target=call, daemon=True)
+        t.start()
+        assert not finished.wait(timeout=0.3), "accessor did not take the discovery lock"
+    finally:
+        pm._discovery_lock.release()
+    t.join(timeout=2)
+    assert finished.is_set()

test/test_plugin_state_history_cap.py

@@ -0,0 +1,166 @@
+"""Plugin state history must not grow without bound.
+
+`PluginStateManager` recorded every state transition in a per-plugin list and
+never trimmed it. The only code that removed entries was `clear_state()`, called
+solely from `PluginManager.unload_plugin()`, so a plugin that stays loaded --
+i.e. normal operation -- never released a single entry.
+
+The list is written on the hot scheduling path. Every update cycle appends
+twice: `_reserve_for_update()` sets RUNNING and `_finish()` sets ENABLED back
+again. At the default 60-second update interval that is 2,880 entries per
+plugin per day, and nothing ever reads the entries -- `get_state_info()` only
+takes their `len()`. It is pure dead weight.
+
+Measured against the unpatched class, ten plugins on a 60s interval retain
+864,010 transitions after thirty simulated days, for 231 MB of heap. On a 1 GB
+Pi that is fatal on its own, and the failure is not a clean OOM: once
+MemAvailable falls far enough, fork() starts returning ENOMEM, so sshd accepts
+connections and closes them before its banner while the kernel still answers
+pings. The board looks like a hardware fault and needs a power cycle.
+
+These tests pin the cap, the retention order, and the one piece of behaviour the
+cap must not change: `state_history_count` is surfaced through the web API, so
+it has to keep reporting the lifetime total rather than plateauing at the cap.
+"""
+
+import os
+import sys
+
+import pytest
+
+sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
+
+from src.plugin_system.plugin_state import (  # noqa: E402
+    MAX_STATE_HISTORY_PER_PLUGIN,
+    PluginState,
+    PluginStateManager,
+)
+
+
+def _cycle_updates(manager, plugin_id, cycles):
+    """Drive the real scheduling path: RUNNING on reserve, ENABLED on finish."""
+    for _ in range(cycles):
+        manager.set_state(plugin_id, PluginState.RUNNING)
+        manager.set_state(plugin_id, PluginState.ENABLED)
+
+
+def test_state_history_is_capped():
+    """A day of updates must not retain a day of transitions."""
+    manager = PluginStateManager()
+    manager.set_state("clock", PluginState.ENABLED)
+
+    # One simulated day at the default 60s update interval.
+    _cycle_updates(manager, "clock", 1440)
+
+    history = manager.get_state_history("clock")
+    assert len(history) <= MAX_STATE_HISTORY_PER_PLUGIN, (
+        f"history grew to {len(history)} entries; it is never trimmed"
+    )
+
+
+def test_state_history_keeps_the_most_recent_transitions():
+    """Trimming drops the oldest entries, not the newest."""
+    manager = PluginStateManager()
+    manager.set_state("clock", PluginState.ENABLED)
+    _cycle_updates(manager, "clock", MAX_STATE_HISTORY_PER_PLUGIN)
+
+    history = manager.get_state_history("clock")
+
+    # The scheduling cycle ends on ENABLED, so the newest entry is the
+    # RUNNING -> ENABLED half of the last cycle.
+    assert history[-1]["from"] == PluginState.RUNNING.value
+    assert history[-1]["to"] == PluginState.ENABLED.value
+
+    # And the very first ENABLED transition has aged out.
+    assert history[0]["from"] != PluginState.UNLOADED.value
+
+
+def test_state_history_count_reports_lifetime_total():
+    """The count exposed through the API must not plateau at the cap.
+
+    `get_state_info()['state_history_count']` is surfaced by the web UI. Capping
+    the retained list must not turn it into "entries we happen to still hold".
+    """
+    manager = PluginStateManager()
+    manager.set_state("clock", PluginState.ENABLED)
+    total = 1
+
+    cycles = MAX_STATE_HISTORY_PER_PLUGIN * 2
+    _cycle_updates(manager, "clock", cycles)
+    total += cycles * 2
+
+    info = manager.get_state_info("clock")
+    assert info["state_history_count"] == total
+    assert len(manager.get_state_history("clock")) <= MAX_STATE_HISTORY_PER_PLUGIN
+
+
+def test_error_transitions_are_capped_too():
+    """set_state_with_error() appends to the same list and needs the same cap."""
+    manager = PluginStateManager()
+    manager.set_state("clock", PluginState.ENABLED)
+
+    for _ in range(MAX_STATE_HISTORY_PER_PLUGIN * 2):
+        manager.set_state_with_error(
+            "clock",
+            PluginState.ENABLED,
+            {"reason": "update timeout"},
+            error=RuntimeError("boom"),
+        )
+
+    assert len(manager.get_state_history("clock")) <= MAX_STATE_HISTORY_PER_PLUGIN
+
+
+def test_history_is_isolated_per_plugin():
+    """The cap is per plugin, not shared across the manager."""
+    manager = PluginStateManager()
+    for plugin_id in ("clock", "weather"):
+        manager.set_state(plugin_id, PluginState.ENABLED)
+        _cycle_updates(manager, plugin_id, 50)
+
+    assert len(manager.get_state_history("clock")) == 101
+    assert len(manager.get_state_history("weather")) == 101
+
+
+def test_get_state_history_returns_a_copy():
+    """Callers must not be able to mutate the manager's internal history."""
+    manager = PluginStateManager()
+    manager.set_state("clock", PluginState.ENABLED)
+
+    history = manager.get_state_history("clock")
+    history.clear()
+
+    assert len(manager.get_state_history("clock")) == 1
+
+
+def test_get_state_history_entries_are_copies():
+    """Copying the outer list is not enough -- the entries are handed out too.
+
+    A caller holding a returned transition must not be able to rewrite the
+    manager's record of what happened.
+    """
+    manager = PluginStateManager()
+    manager.set_state("clock", PluginState.ENABLED)
+
+    entry = manager.get_state_history("clock")[0]
+    entry["to"] = "tampered"
+    entry["error"] = "injected"
+
+    stored = manager.get_state_history("clock")[0]
+    assert stored["to"] == PluginState.ENABLED.value
+    assert stored["error"] is None
+
+
+def test_clear_state_drops_history():
+    """Unloading a plugin still releases everything it accumulated."""
+    manager = PluginStateManager()
+    manager.set_state("clock", PluginState.ENABLED)
+    _cycle_updates(manager, "clock", 10)
+
+    manager.clear_state("clock")
+
+    assert manager.get_state_history("clock") == []
+    assert manager.get_state_info("clock")["state_history_count"] == 0
+
+
+if __name__ == "__main__":
+    sys.exit(pytest.main([__file__, "-v"]))

test/test_plugin_state_history_retention.py

@@ -0,0 +1,209 @@
+"""Retention is bounded by age first and by count second.
+
+The cap added in the parent change is a flat entry count, and an entry count
+answers the wrong question. What a reader wants from this history is "the last
+couple of hours"; how many transitions that is depends entirely on the
+plugin's update interval, which on a real board spans 2s to 3600s. A flat 200
+entries is 4.2 days of history for the slowest plugin and 3.3 minutes for the
+fastest -- so the plugin churning hardest, the one actually worth looking at,
+keeps the least.
+
+Trimming by age makes the retained window comparable whatever the cadence, and
+the count then serves only as a memory ceiling for pollers fast enough to
+produce thousands of transitions inside that window.
+"""
+
+import time
+import pytest
+
+from src.plugin_system.plugin_state import (
+    PluginState,
+    PluginStateManager,
+    MAX_STATE_HISTORY_PER_PLUGIN,
+    STATE_HISTORY_MAX_AGE_SECONDS,
+)
+
+
+class FakeClock:
+    """A monotonic clock the test drives, so no test has to sleep."""
+
+    def __init__(self):
+        self.t = 1000.0
+
+    def __call__(self):
+        return self.t
+
+    def advance(self, seconds):
+        self.t += seconds
+
+
+@pytest.fixture
+def clock(monkeypatch):
+    c = FakeClock()
+    monkeypatch.setattr("src.plugin_system.plugin_state.time.monotonic", c)
+    return c
+
+
+def _cycle(manager, plugin_id, clock, interval, cycles):
+    """One update cycle: RUNNING on reserve, ENABLED on finish."""
+    for _ in range(cycles):
+        manager.set_state(plugin_id, PluginState.RUNNING)
+        manager.set_state(plugin_id, PluginState.ENABLED)
+        clock.advance(interval)
+
+
+def test_transitions_older_than_the_window_are_dropped(clock):
+    m = PluginStateManager()
+    _cycle(m, "clock", clock, interval=60, cycles=10)
+    assert len(m.get_state_history("clock")) == 20
+
+    # Nothing happens for longer than the window, then one more cycle.
+    clock.advance(STATE_HISTORY_MAX_AGE_SECONDS + 1)
+    _cycle(m, "clock", clock, interval=60, cycles=1)
+
+    assert len(m.get_state_history("clock")) == 2, (
+        "only the transitions inside the window should survive")
+
+
+def test_every_plugin_keeps_the_same_WINDOW_not_the_same_COUNT(clock):
+    """The point of the age policy, stated as the property that distinguishes it.
+
+    Run both plugins for three times the retention window. Under a flat count
+    cap the slow one would still be holding transitions from hours before the
+    window, because it never produces enough entries to evict them. Under the
+    age policy each plugin retains its own last two hours and no more --
+    different entry counts, same span of time.
+    """
+    window = STATE_HISTORY_MAX_AGE_SECONDS
+    m = PluginStateManager()
+
+    _cycle(m, "slow", clock, interval=60, cycles=(3 * window) // 60)
+    slow = len(m.get_state_history("slow"))
+
+    # Assert the property directly rather than a derived count. The guarantee
+    # is about the SPAN of retained history, not its age against the current
+    # clock: trimming happens on append, so a plugin that has gone quiet keeps
+    # its last window until it writes again. That is intentional -- it is
+    # bounded either way, and a lazy trim costs nothing on the hot path.
+    stamps = [stamp for stamp, _ in m._state_history["slow"]]
+    assert stamps[-1] - stamps[0] <= window, (
+        f"retained history spans {stamps[-1] - stamps[0]:.0f}s, "
+        f"window is {window}s")
+    assert slow < 2 * ((3 * window) // 60), (
+        f"slow plugin kept {slow} entries -- three windows' worth was retained")
+
+    clock.t = 1000.0
+    _cycle(m, "fast", clock, interval=2, cycles=(3 * window) // 2)
+    fast = len(m.get_state_history("fast"))
+
+    # Different counts, and the fast poller keeps more of them -- under a flat
+    # count cap these would be equal and the fast one would cover minutes.
+    assert fast > slow, f"fast={fast} slow={slow}"
+
+
+def test_the_count_ceiling_still_bounds_a_fast_poller(clock):
+    """Age alone would let a 2s plugin hold 7,200 entries."""
+    m = PluginStateManager()
+    _cycle(m, "flights", clock, interval=2, cycles=STATE_HISTORY_MAX_AGE_SECONDS)
+    assert len(m.get_state_history("flights")) <= MAX_STATE_HISTORY_PER_PLUGIN
+
+
+def test_a_burst_inside_the_window_is_capped_not_kept(clock):
+    """Transitions with no time between them still cannot grow without bound."""
+    m = PluginStateManager()
+    for _ in range(MAX_STATE_HISTORY_PER_PLUGIN * 3):
+        m.set_state("flapping", PluginState.RUNNING)   # clock never advances
+    assert len(m.get_state_history("flapping")) <= MAX_STATE_HISTORY_PER_PLUGIN
+
+
+def test_ageing_out_does_not_disturb_the_lifetime_count(clock):
+    m = PluginStateManager()
+    _cycle(m, "clock", clock, interval=60, cycles=10)
+    clock.advance(STATE_HISTORY_MAX_AGE_SECONDS + 1)
+    _cycle(m, "clock", clock, interval=60, cycles=1)
+
+    assert len(m.get_state_history("clock")) == 2
+    assert m.get_state_info("clock")["state_history_count"] == 22, (
+        "the lifetime total must survive trimming, it is the flap signal")
+
+
+def test_the_surviving_entries_are_the_recent_ones(clock):
+    m = PluginStateManager()
+    _cycle(m, "clock", clock, interval=60, cycles=5)
+    clock.advance(STATE_HISTORY_MAX_AGE_SECONDS + 1)
+    m.set_state("clock", PluginState.ERROR)
+
+    history = m.get_state_history("clock")
+    assert [h["to"] for h in history] == ["error"]
+
+
+def test_a_monotonic_clock_is_used_not_the_wall_clock(clock):
+    """A DST shift or NTP step must not flush the history.
+
+    The trim reads time.monotonic(); the human-readable datetime inside each
+    transition is for display only.
+    """
+    m = PluginStateManager()
+    _cycle(m, "clock", clock, interval=60, cycles=3)
+    before = len(m.get_state_history("clock"))
+
+    import datetime as real_datetime
+
+    class ShiftedDatetime(real_datetime.datetime):
+        @classmethod
+        def now(cls, tz=None):
+            return real_datetime.datetime(1999, 1, 1)   # clock jumps backwards
+
+    import src.plugin_system.plugin_state as ps
+    original = ps.datetime
+    ps.datetime = ShiftedDatetime
+    try:
+        m.set_state("clock", PluginState.ENABLED)
+    finally:
+        ps.datetime = original
+
+    assert len(m.get_state_history("clock")) == before + 1, (
+        "a wall-clock jump must not trim anything")
+
+
+def test_get_state_info_is_a_consistent_snapshot():
+    """An unload running concurrently must not be observed half-done.
+
+    Each field used to be read under its own lock, so clear_state() could
+    interleave: 'state' read before the removal, 'state_history_count' after,
+    handing a caller a plugin that is ENABLED with zero transitions. The whole
+    payload is now built in one critical section.
+    """
+    import threading
+
+    m = PluginStateManager()
+    for _ in range(50):
+        m.set_state("clock", PluginState.RUNNING)
+        m.set_state("clock", PluginState.ENABLED)
+
+    inconsistent = []
+    stop = threading.Event()
+
+    def reader():
+        while not stop.is_set():
+            info = m.get_state_info("clock")
+            # Either fully present or fully cleared -- never a live state with
+            # a wiped count.
+            if info["state"] != PluginState.UNLOADED.value and \
+                    info["state_history_count"] == 0:
+                inconsistent.append(info)
+                return
+
+    def clearer():
+        for _ in range(200):
+            for _ in range(20):
+                m.set_state("clock", PluginState.ENABLED)
+            m.clear_state("clock")
+
+    t = threading.Thread(target=reader, daemon=True)
+    t.start()
+    clearer()
+    stop.set()
+    t.join(timeout=5)
+
+    assert not inconsistent, f"observed a torn snapshot: {inconsistent[:1]}"

test/test_vegas_fps_health.py

@@ -0,0 +1,99 @@
+"""Frame pacing and FPS health reporting must not depend on the wall clock.
+
+These devices have no RTC, so the system clock jumps by however wrong boot
+time was the moment NTP first syncs. The render loop sleeps the *remainder*
+of each frame budget:
+
+    frame_elapsed = <now> - frame_started
+    time.sleep(max(0.0, frame_interval - frame_elapsed))
+
+With a wall-clock `now`, a backward jump makes frame_elapsed negative, so
+`frame_interval - frame_elapsed` exceeds the whole budget and the render loop
+stalls for the size of the correction. A forward jump instead inflates the
+p99 and worst-frame numbers the telemetry reports.
+"""
+import ast
+import sys
+from pathlib import Path
+
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
+
+COORD = (Path(__file__).resolve().parent.parent
+         / "src" / "vegas_mode" / "coordinator.py")
+TREE = ast.parse(COORD.read_text(encoding="utf-8"))
+
+
+def _assignments_of(name):
+    """Every `name = <expr>` in the module, as unparsed source."""
+    out = []
+    for node in ast.walk(TREE):
+        if isinstance(node, ast.Assign):
+            for target in node.targets:
+                if isinstance(target, ast.Name) and target.id == name:
+                    out.append((node.lineno, ast.unparse(node.value)))
+    return out
+
+
+def test_per_frame_timestamps_are_monotonic():
+    for name in ("frame_started", "frame_elapsed"):
+        assigns = _assignments_of(name)
+        assert assigns, f"{name} is no longer assigned -- has the loop changed?"
+        for lineno, expr in assigns:
+            assert "time.time()" not in expr, (
+                f"{name} at line {lineno} uses the wall clock ({expr!r}). A "
+                "backward NTP step makes the per-frame delta negative and the "
+                "loop then sleeps longer than the whole frame budget.")
+            assert "time.monotonic()" in expr, (
+                f"{name} at line {lineno} is {expr!r}, expected monotonic")
+
+
+def test_the_fps_window_is_monotonic():
+    for lineno, expr in _assignments_of("current_time"):
+        assert "time.monotonic()" in expr, (
+            f"current_time at line {lineno} is {expr!r}; fps is frames divided "
+            "by this delta, so a clock step would corrupt the rate itself")
+
+
+def test_health_state_is_not_reset_every_iteration():
+    """run_iteration() runs once per cycle -- locals here reset every few seconds.
+
+    As locals, `last_fps_health_log = 0.0` made the 300s heartbeat fire on the
+    first sample of every iteration, and a recovery spanning two iterations was
+    never reported because was_degraded had already gone back to False.
+    """
+    run_iteration = next(
+        (n for n in ast.walk(TREE)
+         if isinstance(n, ast.FunctionDef) and n.name == "run_iteration"), None)
+    assert run_iteration is not None, "run_iteration() not found"
+
+    local_names = {t.id for n in ast.walk(run_iteration)
+                   if isinstance(n, ast.Assign)
+                   for t in n.targets if isinstance(t, ast.Name)}
+    for leaked in ("last_fps_health_log", "was_degraded"):
+        assert leaked not in local_names, (
+            f"{leaked} is a local of run_iteration() again, so it resets every "
+            "cycle -- the heartbeat degenerates to once per iteration")
+
+    body = ast.unparse(run_iteration)
+    assert "self._fps_last_health_log" in body and "self._fps_was_degraded" in body, (
+        "the health state should live on the coordinator, across iterations")
+
+
+def test_start_clears_stale_health_state():
+    """A new run must not inherit "was degraded" from the previous one."""
+    start = next((n for n in ast.walk(TREE)
+                  if isinstance(n, ast.FunctionDef) and n.name == "start"), None)
+    assert start is not None, "start() not found"
+    body = ast.unparse(start)
+    assert "self._fps_last_health_log" in body and "self._fps_was_degraded" in body, (
+        "start() does not reset the FPS health state")
+
+
+def test_the_degraded_threshold_is_documented():
+    """The 90% band is deliberate; say so where the constant is defined."""
+    source = COORD.read_text(encoding="utf-8")
+    idx = source.index("_FPS_HEALTHY_FRACTION = ")
+    preamble = source[max(0, idx - 700):idx]
+    assert "90%" in preamble or "0.9" in preamble, (
+        "the degradation threshold is not explained at its definition, so "
+        "'below target' reads as a bug rather than a deliberate band")

test/web_interface/test_api_v3_secret_roundtrip.py

@@ -229,6 +229,44 @@ class TestSavePluginConfig:
             "REAL-KEY-0123456789", "an unrelated edit destroyed the API key"
         assert env.fresh_load()[PLUGIN_ID]["city"] == "Dallas"
 
+    def test_an_unrelated_edit_does_not_erase_array_item_secrets(self, env):
+        """The scalar api_key case above, but for a list of credentials.
+
+        remove_empty_secrets recursed into dicts only, so a list went into
+        deep_merge untouched -- and lists merge by *replacement*. Saving any
+        unrelated field posted [{"token": ""}, ...] straight over the stored
+        array and destroyed every token in it at once.
+        """
+        assert self._save(env, {"accounts": [
+            {"name": "a", "token": "REAL-A"},
+            {"name": "b", "token": "REAL-B"},
+        ], "city": "Austin"}).status_code == 200
+
+        # the user changes the city; both masked tokens ride along blank
+        assert self._save(env, {"accounts": [
+            {"name": "a", "token": ""},
+            {"name": "b", "token": ""},
+        ], "city": "Dallas"}).status_code == 200
+
+        merged = env.fresh_load()[PLUGIN_ID]
+        assert [a.get("token") for a in merged["accounts"]] == \
+            ["REAL-A", "REAL-B"], "an unrelated edit destroyed the array secrets"
+        assert [a["name"] for a in merged["accounts"]] == ["a", "b"]
+        assert merged["city"] == "Dallas"
+
+    def test_one_array_secret_can_be_changed_without_losing_the_rest(self, env):
+        assert self._save(env, {"accounts": [
+            {"name": "a", "token": "REAL-A"},
+            {"name": "b", "token": "REAL-B"},
+        ]}).status_code == 200
+        assert self._save(env, {"accounts": [
+            {"name": "a", "token": ""},
+            {"name": "b", "token": "NEW-B"},
+        ]}).status_code == 200
+
+        merged = env.fresh_load()[PLUGIN_ID]
+        assert [a.get("token") for a in merged["accounts"]] == ["REAL-A", "NEW-B"]
+
     def test_a_secret_can_still_be_changed(self, env):
         """Dropping blanks must not stop a real new value from being saved."""
         self._save(env, {"api_key": "first-key"})

test/web_interface/test_config_logging_omits_secrets.py

@@ -0,0 +1,45 @@
+"""The validation logging ran before separate_secrets, so it logged credentials.
+
+api_v3's plugin-config save logged `Full config: {plugin_config}` at INFO and
+`Config that failed: {plugin_config}` at ERROR. Both run *before*
+separate_secrets(), so plugin_config still held the values the user just typed
+into the form -- API keys and tokens went to the journal in clear text.
+"""
+import re
+from pathlib import Path
+
+import pytest
+
+SOURCE = (Path(__file__).resolve().parents[2]
+          / "web_interface" / "blueprints" / "api_v3.py")
+
+#: Objects that still hold submitted secret values at the point these log
+#: calls run. Interpolating one whole into a log message leaks credentials.
+UNREDACTED = ("plugin_config", "secrets_config", "current_secrets")
+
+
+def _logging_lines():
+    for number, line in enumerate(SOURCE.read_text(encoding="utf-8").splitlines(), 1):
+        stripped = line.strip()
+        if stripped.startswith("#"):
+            continue
+        if re.match(r"logger\.(debug|info|warning|error|critical|exception)\(", stripped):
+            yield number, stripped
+
+
+@pytest.mark.parametrize("name", UNREDACTED)
+def test_no_log_call_interpolates_a_whole_secret_bearing_object(name):
+    # {name} or {name['k']} leaks; {list(name.keys())} and {len(name)} do not.
+    bare = re.compile(r"\{" + re.escape(name) + r"(\[[^\]]*\])*\}")
+    offenders = [f"{n}: {text}" for n, text in _logging_lines() if bare.search(text)]
+    assert not offenders, (
+        f"{name} still holds submitted secrets where these log calls run:\n  "
+        + "\n  ".join(offenders))
+
+
+def test_the_guard_would_notice_a_reintroduced_leak():
+    """Pin the detector itself, so a rewrite cannot silently stop matching."""
+    bare = re.compile(r"\{" + re.escape("plugin_config") + r"(\[[^\]]*\])*\}")
+    assert bare.search('logger.info(f"Full config: {plugin_config}")')
+    assert bare.search("logger.error(f\"{plugin_config['api_key']}\")")
+    assert not bare.search('logger.info(f"{list(plugin_config.keys())}")')

test/web_interface/test_secret_helpers.py

@@ -17,6 +17,7 @@ from src.web_interface.secret_helpers import (
     separate_secrets,
     mask_secret_fields,
     mask_all_secret_values,
+    merge_secrets,
     remove_empty_secrets,
 )
 
@@ -239,3 +240,67 @@ class TestRemoveEmptySecrets:
     def test_keeps_falsey_non_string_values(self):
         # 0 and False are neither None nor blank strings — they are kept.
         assert remove_empty_secrets({"a": 0, "b": False}) == {"a": 0, "b": False}
+
+
+class TestArrayItemSecrets:
+    """Lists merge by replacement, so a blanked array wipes stored credentials.
+
+    remove_empty_secrets recursed into dicts but let a list through untouched,
+    so [{"token": ""}] went straight into deep_merge and overwrote the stored
+    list. Saving any unrelated setting destroyed every token in the array.
+    """
+
+    STORED = {"accounts": [{"name": "a", "token": "REAL-A"},
+                           {"name": "b", "token": "REAL-B"}]}
+
+    def test_an_unrelated_save_keeps_every_stored_token(self):
+        posted = {"accounts": [{"name": "a", "token": ""},
+                               {"name": "b", "token": ""}]}
+        merged = merge_secrets(self.STORED, remove_empty_secrets(posted))
+        assert [a["token"] for a in merged["accounts"]] == ["REAL-A", "REAL-B"]
+
+    def test_editing_one_entry_leaves_the_others_alone(self):
+        posted = {"accounts": [{"name": "a", "token": ""},
+                               {"name": "b", "token": "NEW-B"}]}
+        merged = merge_secrets(self.STORED, remove_empty_secrets(posted))
+        assert [a["token"] for a in merged["accounts"]] == ["REAL-A", "NEW-B"]
+
+    def test_a_new_entry_is_appended(self):
+        posted = {"accounts": [{"name": "a", "token": ""},
+                               {"name": "b", "token": ""},
+                               {"name": "c", "token": "NEW-C"}]}
+        merged = merge_secrets(self.STORED, remove_empty_secrets(posted))
+        assert [a["token"] for a in merged["accounts"]] == \
+            ["REAL-A", "REAL-B", "NEW-C"]
+
+    def test_a_list_of_bare_strings_merges_by_index(self):
+        merged = merge_secrets({"keys": ["K1", "K2", "K3"]},
+                               remove_empty_secrets({"keys": ["", "K2-NEW", ""]}))
+        assert merged["keys"] == ["K1", "K2-NEW", "K3"]
+
+    def test_an_all_blank_list_is_dropped_entirely(self):
+        posted = {"accounts": [{"token": ""}, {"token": ""}]}
+        assert "accounts" not in remove_empty_secrets(posted)
+
+    def test_plain_dict_secrets_are_unaffected(self):
+        merged = merge_secrets({"api_key": "OLD", "other": "keep"},
+                               remove_empty_secrets({"api_key": "", "other": "changed"}))
+        assert merged == {"api_key": "OLD", "other": "changed"}
+
+    def test_a_removed_entry_takes_its_secret_with_it(self):
+        """The regular config's list is authoritative about how many items
+        exist, and the secrets list runs parallel to it -- see
+        ConfigManager._strip_secrets_recursive. So a shorter incoming list
+        must shorten the stored secrets too, or the two fall out of step."""
+        posted = {"accounts": [{"name": "a", "token": "NEW-A"}]}
+        merged = merge_secrets(self.STORED, remove_empty_secrets(posted))
+        assert [a["token"] for a in merged["accounts"]] == ["NEW-A"]
+
+    def test_an_emptied_item_stays_a_dict_not_none(self):
+        """None there stops the list looking parallel, and
+        _strip_secrets_recursive then drops the whole key from the main
+        config -- deleting the item's non-secret fields as well."""
+        pruned = remove_empty_secrets(
+            {"accounts": [{"token": "real"}, {"token": ""}]})
+        assert pruned["accounts"] == [{"token": "real"}, {}]
+        assert None not in pruned["accounts"]

web_interface/blueprints/api_v3.py

@@ -22,7 +22,8 @@ logger = logging.getLogger(__name__)
 from src.web_interface.api_helpers import success_response, error_response, validate_request_json
 from src.web_interface.errors import ErrorCode
 from src.web_interface.secret_helpers import (find_secret_fields, mask_all_secret_values,
-                                              remove_empty_secrets, separate_secrets,
+                                              merge_secrets, remove_empty_secrets,
+                                              separate_secrets,
                                               strip_masked_values)
 from src.web_interface.error_handler import describe_exception, redact_text
 from src.plugin_system.operation_types import OperationType
@@ -597,7 +598,7 @@ def save_dim_schedule_config():
                 dim_brightness = 30
             else:
                 dim_brightness = int(dim_brightness_raw)
-        except (ValueError, TypeError):
+        except (ValueError, TypeError, OverflowError):
             return error_response(
                 ErrorCode.VALIDATION_ERROR,
                 "dim_brightness must be an integer between 0 and 100",
@@ -797,7 +798,7 @@ def save_main_config():
                 }), 400
             try:
                 target_fps = int(raw_target_fps)
-            except (ValueError, TypeError):
+            except (ValueError, TypeError, OverflowError):
                 return jsonify({
                     'status': 'error',
                     'message': "Invalid value for target_fps: must be an integer"
@@ -867,7 +868,7 @@ def save_main_config():
                     mux_val = int(data['multiplexing'])
                     if mux_val < 0 or mux_val > 22:
                         return jsonify({'status': 'error', 'message': f"Invalid multiplexing value '{data['multiplexing']}'. Must be an integer from 0 to 22."}), 400
-                except (ValueError, TypeError):
+                except (ValueError, TypeError, OverflowError):
                     return jsonify({'status': 'error', 'message': f"Invalid multiplexing value '{data['multiplexing']}'. Must be an integer from 0 to 22."}), 400
 
             # Validate pixel_mapper_config (free-form mapper string, e.g. "U-mapper;Rotate:90")
@@ -885,7 +886,7 @@ def save_main_config():
                     rat_val = int(data['row_address_type'])
                     if rat_val < 0 or rat_val > 4:
                         return jsonify({'status': 'error', 'message': f"Invalid row_address_type '{data['row_address_type']}'. Must be an integer from 0 to 4."}), 400
-                except (ValueError, TypeError):
+                except (ValueError, TypeError, OverflowError):
                     return jsonify({'status': 'error', 'message': f"Invalid row_address_type '{data['row_address_type']}'. Must be an integer from 0 to 4."}), 400
 
             # Handle hardware settings
@@ -910,7 +911,7 @@ def save_main_config():
                     if rp1_val not in (0, 1):
                         return jsonify({'status': 'error', 'message': "rp1_rio must be 0 (PIO) or 1 (RIO)"}), 400
                     current_config['display']['runtime']['rp1_rio'] = rp1_val
-                except (ValueError, TypeError):
+                except (ValueError, TypeError, OverflowError):
                     return jsonify({'status': 'error', 'message': "rp1_rio must be 0 or 1"}), 400
 
             # Handle checkboxes - coerce to bool to ensure proper JSON types
@@ -963,7 +964,7 @@ def save_main_config():
                 copies = None
                 try:
                     copies = int(data['double_sided_copies'])
-                except (ValueError, TypeError):
+                except (ValueError, TypeError, OverflowError):
                     if enabled:
                         return jsonify({'status': 'error', 'message': "Double-sided copies must be an integer"}), 400
                 if copies is not None and not (2 <= copies <= 8):
@@ -1036,7 +1037,7 @@ def save_main_config():
             if data.get('vegas_extend_threshold_screens') not in ('', None):
                 try:
                     screens = float(data['vegas_extend_threshold_screens'])
-                except (ValueError, TypeError):
+                except (ValueError, TypeError, OverflowError):
                     return jsonify({
                         'status': 'error',
                         'message': "Invalid value for vegas_extend_threshold_screens: "
@@ -1053,7 +1054,7 @@ def save_main_config():
             if data.get('vegas_max_plugin_width_ratio') not in ('', None):
                 try:
                     ratio = float(data['vegas_max_plugin_width_ratio'])
-                except (ValueError, TypeError):
+                except (ValueError, TypeError, OverflowError):
                     return jsonify({
                         'status': 'error',
                         'message': "Invalid value for vegas_max_plugin_width_ratio: "
@@ -1101,7 +1102,7 @@ def save_main_config():
                         continue
                     try:
                         int_value = int(raw_value)
-                    except (ValueError, TypeError):
+                    except (ValueError, TypeError, OverflowError):
                         return jsonify({
                             'status': 'error',
                             'message': f"Invalid value for {field_name}: must be an integer"
@@ -1153,7 +1154,7 @@ def save_main_config():
                     if not (1024 <= port_val <= 65535):
                         return jsonify({'status': 'error', 'message': "sync_port must be between 1024 and 65535"}), 400
                     current_config['sync']['port'] = port_val
-                except (ValueError, TypeError):
+                except (ValueError, TypeError, OverflowError):
                     return jsonify({'status': 'error', 'message': "sync_port must be an integer"}), 400
 
             if "sync_follower_position" in data:
@@ -1197,7 +1198,7 @@ def save_main_config():
                 raw_value = data.pop(field)
                 try:
                     int_value = int(raw_value)
-                except (ValueError, TypeError):
+                except (ValueError, TypeError, OverflowError):
                     return jsonify({'status': 'error',
                                     'message': f"Invalid duration for {field}: must be an integer"}), 400
                 current_config['display']['display_durations'][field] = int_value
@@ -1220,7 +1221,7 @@ def save_main_config():
                     continue
                 try:
                     int_value = int(raw_value)
-                except (ValueError, TypeError):
+                except (ValueError, TypeError, OverflowError):
                     return jsonify({'status': 'error',
                                     'message': f"Invalid duration for mode '{mode_key}': must be an integer"}), 400
                 current_config['display']['display_durations'][mode_key] = int_value
@@ -1296,7 +1297,10 @@ def save_main_config():
                 if secrets_config:
                     if plugin_id not in current_secrets:
                         current_secrets[plugin_id] = {}
-                    current_secrets[plugin_id] = deep_merge(current_secrets[plugin_id], secrets_config)
+                    # Lists merge by replacement, so deep_merge here wrote a
+                    # blanked array straight over the stored credentials.
+                    current_secrets[plugin_id] = merge_secrets(
+                        current_secrets[plugin_id], secrets_config)
                     # Save secrets file
                     api_v3.config_manager.save_raw_file_content('secrets', current_secrets)
 
@@ -5118,7 +5122,7 @@ def save_plugin_config():
                                                                 converted_array.append(int(v))
                                                             else:
                                                                 converted_array.append(float(v))
-                                                        except (ValueError, TypeError):
+                                                        except (ValueError, TypeError, OverflowError):
                                                             converted_array.append(v)
                                                     else:
                                                         converted_array.append(v)
@@ -5143,7 +5147,7 @@ def save_plugin_config():
                                                         converted_array.append(int(v))
                                                     else:
                                                         converted_array.append(float(v))
-                                                except (ValueError, TypeError):
+                                                except (ValueError, TypeError, OverflowError):
                                                     converted_array.append(v)
                                             else:
                                                 converted_array.append(v)
@@ -5180,7 +5184,7 @@ def save_plugin_config():
                                                                 converted_array.append(int(v))
                                                             else:
                                                                 converted_array.append(float(v))
-                                                        except (ValueError, TypeError):
+                                                        except (ValueError, TypeError, OverflowError):
                                                             converted_array.append(v)
                                                     else:
                                                         converted_array.append(v)
@@ -5204,7 +5208,7 @@ def save_plugin_config():
                                                         converted_array.append(int(v))
                                                     else:
                                                         converted_array.append(float(v))
-                                                except (ValueError, TypeError):
+                                                except (ValueError, TypeError, OverflowError):
                                                     converted_array.append(v)
                                             else:
                                                 converted_array.append(v)
@@ -5371,7 +5375,7 @@ def save_plugin_config():
                                         if isinstance(v, str):
                                             try:
                                                 converted.append(int(v) if item_type == 'integer' else float(v))
-                                            except (ValueError, TypeError):
+                                            except (ValueError, TypeError, OverflowError):
                                                 converted.append(v)
                                         else:
                                             converted.append(v)
@@ -5496,7 +5500,7 @@ def save_plugin_config():
                             try:
                                 normalized[key] = int(value_stripped)
                                 continue
-                            except (ValueError, TypeError):
+                            except (ValueError, TypeError, OverflowError):
                                 pass
                         elif isinstance(value, (int, float)):
                             normalized[key] = int(value)
@@ -5514,7 +5518,7 @@ def save_plugin_config():
                             try:
                                 normalized[key] = float(value_stripped)
                                 continue
-                            except (ValueError, TypeError):
+                            except (ValueError, TypeError, OverflowError):
                                 pass
                         elif isinstance(value, (int, float)):
                             normalized[key] = float(value)
@@ -5569,7 +5573,7 @@ def save_plugin_config():
                                     try:
                                         normalized_array.append(int(v))
                                         continue
-                                    except (ValueError, TypeError):
+                                    except (ValueError, TypeError, OverflowError):
                                         pass
                                 elif isinstance(v, (int, float)):
                                     normalized_array.append(int(v))
@@ -5579,7 +5583,7 @@ def save_plugin_config():
                                     try:
                                         normalized_array.append(float(v))
                                         continue
-                                    except (ValueError, TypeError):
+                                    except (ValueError, TypeError, OverflowError):
                                         pass
                                 elif isinstance(v, (int, float)):
                                     normalized_array.append(float(v))
@@ -5595,7 +5599,7 @@ def save_plugin_config():
                             if isinstance(v, str):
                                 try:
                                     normalized_array.append(int(v))
-                                except (ValueError, TypeError):
+                                except (ValueError, TypeError, OverflowError):
                                     normalized_array.append(v)
                             elif isinstance(v, (int, float)):
                                 normalized_array.append(int(v))
@@ -5609,7 +5613,7 @@ def save_plugin_config():
                             if isinstance(v, str):
                                 try:
                                     normalized_array.append(float(v))
-                                except (ValueError, TypeError):
+                                except (ValueError, TypeError, OverflowError):
                                     normalized_array.append(v)
                             else:
                                 normalized_array.append(v)
@@ -5632,7 +5636,7 @@ def save_plugin_config():
                     if isinstance(value, str):
                         try:
                             normalized[key] = int(value)
-                        except (ValueError, TypeError):
+                        except (ValueError, TypeError, OverflowError):
                             normalized[key] = value
                     else:
                         normalized[key] = value
@@ -5641,7 +5645,7 @@ def save_plugin_config():
                     if isinstance(value, str):
                         try:
                             normalized[key] = float(value)
-                        except (ValueError, TypeError):
+                        except (ValueError, TypeError, OverflowError):
                             normalized[key] = value
                     else:
                         normalized[key] = value
@@ -5675,8 +5679,10 @@ def save_plugin_config():
         if schema:
             # Log what we're validating for debugging
             logger.info(f"Validating config for {plugin_id}")
+            # Only the shape. plugin_config still holds the submitted secret
+            # values at this point -- separate_secrets does not run until
+            # below -- so logging it wrote live credentials to the journal.
             logger.info(f"Config keys being validated: {list(plugin_config.keys())}")
-            logger.info(f"Full config: {plugin_config}")
 
             # Get enhanced schema keys (including injected core properties)
             # We need to create an enhanced schema to get the actual allowed keys
@@ -5699,7 +5705,8 @@ def save_plugin_config():
                 # Log validation errors for debugging
                 logger.error(f"Config validation failed for {plugin_id}")
                 logger.error(f"Validation errors: {validation_errors}")
-                logger.error(f"Config that failed: {plugin_config}")
+                # Keys only, for the same reason as above.
+                logger.error(f"Config keys that failed: {list(plugin_config.keys())}")
                 logger.error(f"Schema properties: {list(enhanced_schema.get('properties', {}).keys())}")
 
                 # Also print to console for immediate visibility
@@ -5750,7 +5757,9 @@ def save_plugin_config():
         if secrets_config:
             if plugin_id not in current_secrets:
                 current_secrets[plugin_id] = {}
-            current_secrets[plugin_id] = deep_merge(current_secrets[plugin_id], secrets_config)
+            # See above -- secrets lists must merge element-wise.
+            current_secrets[plugin_id] = merge_secrets(
+                current_secrets[plugin_id], secrets_config)
             # Save secrets file
             try:
                 api_v3.config_manager.save_raw_file_content('secrets', current_secrets)
@@ -6779,7 +6788,7 @@ def get_font_preview() -> tuple[Response, int] | Response:
         # Safe integer parsing for size
         try:
             size = int(request.args.get('size', 12))
-        except (ValueError, TypeError):
+        except (ValueError, TypeError, OverflowError):
             return jsonify({'status': 'error', 'message': 'Invalid font size'}), 400
 
         if not font_filename:
@@ -8360,7 +8369,7 @@ def clear_old_errors():
                     context={'provided_value': raw_max_age},
                     status_code=400
                 )
-        except (ValueError, TypeError):
+        except (ValueError, TypeError, OverflowError):
             return error_response(
                 error_code=ErrorCode.INVALID_INPUT,
                 message="max_age_hours must be a valid integer",

web_interface/static/v3/app.js

@@ -126,7 +126,17 @@ window.showRestartPending = function(message) {
     } catch { /* private browsing */ }
     const banner = document.getElementById('restart-pending-banner');
     const text = document.getElementById('restart-pending-text');
-    if (text && message) text.textContent = message;
+    if (text) {
+        // Without the else-branch a config save inherited whatever wording the
+        // previous update left in the DOM: showRestartPending() clears the
+        // stored text but used to leave the element itself alone. The default
+        // is read back from the server-rendered copy rather than duplicated
+        // here, so the template stays the one place that owns the string.
+        if (text.dataset.defaultText === undefined) {
+            text.dataset.defaultText = text.textContent.trim();
+        }
+        text.textContent = message || text.dataset.defaultText;
+    }
     if (banner) banner.style.display = 'block';
 };