chore: satisfy Codacy's subprocess checks on the new test runner

Codacy runs Bandit and Opengrep (its Semgrep fork). The new
subprocess.run in scripts/run_plugin_tests.py trips three patterns, on
two different lines:

  Bandit   B404 on the import, B603 on the call
  Opengrep dangerous-subprocess-use-audit          on the run( line
           dangerous-subprocess-use-tainted-env-args on the argv line

A nosemgrep applies only to its own line, so the call line and the argv
line each need one; a single comment on the call covered neither rule
fully. Suppression is the right answer here rather than a rewrite: the
interpreter is sys.executable, the arguments are a list, and no shell is
involved, so there is nothing to word-split or expand.

Matches the pair the rest of the repo already uses for this shape --
permission_utils.py, plugin_loader.py, install_dependencies_apt.py.

Codacy: 0 new issues, up to standards.

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

plugin-repos/starlark-apps/manager.py

@@ -681,12 +681,18 @@ class StarlarkAppsPlugin(BasePlugin):
                 if app.is_enabled() and app.should_render(current_time):
                     self._render_app(app, force=False)
 
-    def display(self, force_clear: bool = False) -> None:
+    def display(self, force_clear: bool = False) -> bool:
         """
         Display current Starlark app.
 
         This method is called during the display rotation.
         Displays frames from the currently active app.
+
+        Returns False when there is no app to show -- which is the state of
+        every install without Pixlet, and of a fresh one before any app is
+        added. The display controller only skips a mode on a boolean False
+        (it checks isinstance(result, bool)), so returning None held a black
+        panel for the full display_duration instead of rotating on.
         """
         try:
             if force_clear:
@@ -699,20 +705,24 @@ class StarlarkAppsPlugin(BasePlugin):
             if not self.current_app:
                 # No apps available
                 self.logger.debug("No Starlark apps to display")
-                return
+                return False
 
             # Render app if needed
             if not self.current_app.frames:
                 success = self._render_app(self.current_app, force=True)
                 if not success:
                     self.logger.error(f"Failed to render app: {self.current_app.app_id}")
-                    return
+                    return False
 
-            # Display current frame
-            self._display_frame()
+            # Display current frame. The result is propagated: a failed frame
+            # update is not a displayed frame, and returning True regardless
+            # told the controller the mode had rendered, so it held the dead
+            # frame for the whole display_duration instead of rotating on.
+            return self._display_frame()
 
         except Exception as e:
             self.logger.error(f"Error displaying Starlark app: {e}")
+            return False
 
     def _select_next_app(self) -> None:
         """Select the next enabled app for display."""
@@ -835,10 +845,13 @@ class StarlarkAppsPlugin(BasePlugin):
             self.logger.error(f"Error loading frames for {app.app_id}: {e}")
             return False
 
-    def _display_frame(self) -> None:
-        """Display the current frame of the current app."""
+    def _display_frame(self) -> bool:
+        """Display the current frame of the current app.
+
+        :returns: whether a frame actually reached the display manager.
+        """
         if not self.current_app or not self.current_app.frames:
-            return
+            return False
 
         try:
             current_time = time.time()
@@ -856,8 +869,11 @@ class StarlarkAppsPlugin(BasePlugin):
                 )
                 self.current_app.last_frame_time = current_time
 
+            return True
+
         except Exception as e:
             self.logger.error(f"Error displaying frame: {e}")
+            return False
 
     def install_app(self, app_id: str, star_file_path: str, metadata: Optional[Dict[str, Any]] = None, assets_dir: Optional[str] = None) -> bool:
         """

scripts/run_plugin_tests.py

@@ -6,6 +6,9 @@ Discovers and runs tests for LEDMatrix plugins.
 Supports both unittest and pytest.
 """
 
+import os
+import re
+import subprocess  # nosec B404 - list-form argv only, no shell  # nosemgrep
 import sys
 import argparse
 from pathlib import Path
@@ -68,6 +71,79 @@ def _find_tests_in_dir(directory: Path) -> list:
     return sorted(set(test_files))
 
 
+def _is_script_style(path) -> bool:
+    """True when a test file is a standalone script, not a pytest module.
+
+    Most plugin tests are written as `def main()` plus an `if __name__ ==
+    "__main__"` guard and signal through an exit code. pytest collects zero
+    items from those, so handing them to pytest printed "no tests ran" and this
+    runner reported success over work it had not done -- 151 of 248 files on a
+    fully populated rig.
+    """
+    try:
+        src = Path(path).read_text(encoding="utf-8", errors="replace")
+    except OSError:
+        return False
+    has_pytest_items = re.search(r"^\s*(def test_|class Test|async def test_)", src, re.M)
+    has_main_guard = "__main__" in src and "__name__" in src
+    return bool(has_main_guard and not has_pytest_items)
+
+
+def run_script_tests(test_files: list, verbose: bool = False) -> int:
+    """Run standalone test scripts, honouring the 0 pass / 2 skip / 1 fail
+    convention that ledmatrix-plugins' own runner established.
+
+    Scripts opt into skipping by printing "SKIP: <reason>" and exiting 2 --
+    a script that needs a tty or an LED matrix is not a regression.
+    """
+    env = dict(os.environ)
+    # Prepend rather than setdefault. An inherited PYTHONPATH -- a developer's
+    # shell, a tox run, another checkout -- otherwise wins outright, and the
+    # subprocess imports a different copy of the core than the one under test.
+    # That is exactly the failure ledmatrix-plugins#467 describes, and it is
+    # invisible: the tests pass or fail against a tree nobody meant to test.
+    inherited = env.get("PYTHONPATH")
+    env["PYTHONPATH"] = (f"{PROJECT_ROOT}{os.pathsep}{inherited}"
+                         if inherited else str(PROJECT_ROOT))
+    env["LEDMATRIX_CORE"] = str(PROJECT_ROOT)
+
+    passed = skipped = failed = 0
+    failures = []
+    for path in test_files:
+        try:
+            # Fixed interpreter (sys.executable) plus a test path this script
+            # discovered by globbing the repo; argument list, no shell, so
+            # nothing is word-split or expanded. Same suppression pair the
+            # rest of the repo uses for this shape (see permission_utils.py).
+            proc = subprocess.run(  # nosec B603 - no shell invoked (list-form argv)  # nosemgrep
+                [sys.executable, str(path)],  # nosemgrep
+                cwd=str(Path(path).parent),
+                capture_output=True, text=True, env=env,
+                stdin=subprocess.DEVNULL, timeout=300,
+            )
+            rc = proc.returncode
+            tail = " | ".join((proc.stdout or proc.stderr or "").strip().splitlines()[-2:])[:200]
+        except subprocess.TimeoutExpired:
+            rc, tail = 1, "timed out after 300s"
+        if rc == 0:
+            passed += 1
+            label = "pass"
+        elif rc == 2:
+            skipped += 1
+            label = "SKIP"
+        else:
+            failed += 1
+            label = "FAIL"
+            failures.append(f"{Path(path).name}: exit {rc} | {tail}")
+        if verbose or rc != 0:
+            print(f"  [{label}] {Path(path).name}" + (f" -- {tail}" if rc != 0 else ""))
+
+    print(f"\n{passed} passed, {skipped} skipped, {failed} failed (scripts)")
+    for f in failures:
+        print(f"  - {f}", file=sys.stderr)
+    return 1 if failed else 0
+
+
 def run_unittest_tests(test_files: list, verbose: bool = False) -> int:
     """
     Run tests using unittest.
@@ -186,7 +262,12 @@ def main():
             print("No test files found in plugins directory")
         return 0
     
-    print(f"Found {len(test_files)} test file(s)")
+    scripts = [f for f in test_files if _is_script_style(f)]
+    modules = [f for f in test_files if f not in scripts]
+
+    print(f"Found {len(test_files)} test file(s)"
+          + (f" -- {len(modules)} collectable, {len(scripts)} standalone script(s)"
+             if scripts else ""))
     for test_file in test_files:
         print(f"  - {test_file}")
     print()
@@ -200,11 +281,20 @@ def main():
         except ImportError:
             runner = 'unittest'
 
-    # Run tests
+    # Standalone scripts cannot be collected by pytest or unittest -- run them
+    # as the scripts they are. Doing this rather than silently collecting zero
+    # items is the whole point: this runner used to report success having
+    # executed nothing.
+    rc = 0
+    if scripts:
+        rc |= run_script_tests(scripts, args.verbose)
+
+    if modules:
         if runner == 'pytest':
-        return run_pytest_tests(test_files, args.verbose, args.coverage)
+            rc |= run_pytest_tests(modules, args.verbose, args.coverage)
         else:
-        return run_unittest_tests(test_files, args.verbose)
+            rc |= run_unittest_tests(modules, args.verbose)
+    return rc
 
 
 if __name__ == '__main__':

src/display_controller.py

@@ -198,6 +198,10 @@ class DisplayController:
         # the main run loop reconciles (loads/unloads) on its own thread so
         # mutating available_modes never races with rendering.
         self._pending_plugin_reconcile = False
+        # Monotonic stamp of the last mailbox disk read; see
+        # _poll_on_demand_requests. None means "never polled", so the first
+        # call always goes through.
+        self._last_on_demand_poll: Optional[float] = None
         self.on_demand_active = False
         self.on_demand_mode: Optional[str] = None
         self.on_demand_modes: List[str] = []  # All modes for the on-demand plugin
@@ -1267,12 +1271,34 @@ class DisplayController:
         self.on_demand_schedule_override = False
         self._publish_on_demand_state()
 
+    #: Shortest gap between mailbox disk reads. This is called after every
+    #: frame -- about 125 times a second on a scrolling mode -- and the read
+    #: below is deliberately uncached, so without a floor it was 125 disk reads
+    #: per second to find nothing. An on-demand request comes from a person
+    #: clicking in the web UI, so a quarter second of latency is not
+    #: perceptible, and it cuts the read rate by 30x.
+    ON_DEMAND_POLL_INTERVAL = 0.25
+
     def _poll_on_demand_requests(self) -> None:
         """Poll cache for new on-demand requests from external controllers."""
+        now = time.monotonic()
+        if (self._last_on_demand_poll is not None
+                and now - self._last_on_demand_poll < self.ON_DEMAND_POLL_INTERVAL):
+            return
+        self._last_on_demand_poll = now
+
         try:
             # Use a long max_age (1 hour) to ensure requests aren't expired before processing
-            # The request_id check prevents duplicate processing
-            request = self.cache_manager.get('display_on_demand_request', max_age=3600)
+            # The request_id check prevents duplicate processing.
+            #
+            # memory_ttl=0 is required, not optional: this key is a mailbox the
+            # web process writes and this process reads. get() defaults the
+            # in-memory TTL to max_age, so without it the first request read was
+            # pinned in memory for the full hour and every later poll returned
+            # that stale copy -- meaning no second on-demand request was honoured
+            # for an hour, while the API still reported success.
+            request = self.cache_manager.get('display_on_demand_request',
+                                             max_age=3600, memory_ttl=0)
         except (OSError, RuntimeError, ValueError, TypeError) as err:
             logger.error("Failed to read on-demand request: %s", err, exc_info=True)
             return
@@ -1318,6 +1344,36 @@ class DisplayController:
         # Mark as processed BEFORE processing (to prevent duplicate processing)
         self.cache_manager.set('display_on_demand_processed_id', request_id, ttl=3600)
         self.on_demand_request_id = request_id
+        # Consume the mailbox entry. Leaving it on disk meant a restart replayed
+        # the previous request: the fresh controller read it, activated it and
+        # cached it, so the request the caller had just made was ignored and the
+        # panel silently showed the earlier plugin. processed_id still guards
+        # against double-processing if this delete fails.
+        try:
+            # Compare before deleting. The web process can post a newer request
+            # between the read above and this delete; an unconditional delete
+            # threw that one away and it was never processed -- the user's
+            # second click did nothing. Re-reading uncached and only deleting
+            # our own request_id means a newer request is left in the mailbox
+            # for the next poll instead.
+            #
+            # This narrows the window rather than closing it: a request landing
+            # between this re-read and the delete is still lost. Closing it
+            # properly needs an atomic claim (a rename, or a compare-and-delete
+            # primitive) that the cache layer does not currently offer, so the
+            # honest fix is a smaller window plus this note, not a bigger lock.
+            current = self.cache_manager.get('display_on_demand_request',
+                                             max_age=3600, memory_ttl=0)
+            if not current or current.get('request_id') == request_id:
+                self.cache_manager.delete('display_on_demand_request')
+            else:
+                logger.debug("Newer on-demand request %s arrived while processing "
+                             "%s; leaving it in the mailbox",
+                             current.get('request_id'), request_id)
+        except (OSError, AttributeError, KeyError) as err:
+            # Best-effort: processed_id still guards against reprocessing if the
+            # mailbox cannot be cleared.
+            logger.debug("Could not clear the on-demand request mailbox: %s", err)
         
         if action == 'start':
             logger.info("Processing on-demand start request for plugin: %s", request.get('plugin_id'))

src/display_manager.py

@@ -1453,12 +1453,30 @@ class DisplayManager:
                 if parent_dir and str(parent_dir) != '/tmp':  # nosec B108 - guard to skip /tmp for permission ops
                     ensure_directory_permissions(parent_dir, get_assets_dir_mode())
                 self._snapshot_dir_prepared = True
-            # Write atomically: temp then replace
-            tmp_path = f"{self._snapshot_path}.tmp"
-            self.image.save(tmp_path, format='PNG')
+            # Write atomically: temp then replace. The temp name must be
+            # unique, not "<snapshot>.tmp": /tmp is world-writable and sticky,
+            # and this file is written by whichever user the display service
+            # runs as while tests and tooling run as someone else. A leftover
+            # fixed-name temp owned by another user is then unopenable even by
+            # root (fs.protected_regular refuses O_CREAT on a foreign file in a
+            # sticky dir), which froze the preview and the health check's
+            # liveness proxy until somebody deleted it by hand. Same pattern as
+            # the hardware-status write above.
+            _fd, tmp_path = tempfile.mkstemp(
+                dir=str(snapshot_path_obj.parent),
+                prefix=f".{snapshot_path_obj.name}.", suffix=".tmp")
             try:
+                with os.fdopen(_fd, "wb") as _f:
+                    self.image.save(_f, format='PNG')
+                os.chmod(tmp_path, 0o644)
                 os.replace(tmp_path, self._snapshot_path)
             except Exception:
+                # Never leave the temp behind -- that is what made the failure
+                # permanent rather than transient.
+                try:
+                    os.unlink(tmp_path)
+                except OSError:
+                    pass
                 # Fallback to direct save if replace not supported
                 self.image.save(self._snapshot_path, format='PNG')
             # Set proper file permissions after saving

src/font_manager.py

@@ -96,7 +96,8 @@ class FontManager:
         self.common_fonts = {
             "press_start": "assets/fonts/PressStart2P-Regular.ttf",
             "four_by_six": "assets/fonts/4x6-font.ttf",
-            "five_by_seven": "assets/fonts/5x7.bdf"
+            "five_by_seven": "assets/fonts/5x7.bdf",
+            "tom_thumb": "assets/fonts/tom-thumb.bdf"
             # Note: cozette_bdf removed - font file not available
             # To re-enable: download cozette.bdf from https://github.com/the-moonwitch/Cozette
             # and add: "cozette_bdf": "assets/fonts/cozette.bdf"

src/plugin_system/testing/harness.py

@@ -14,6 +14,8 @@ via BoundsCheckingDisplayManager, and golden-image comparison.
 import contextlib
 import http.client
 import inspect
+import time
+from datetime import timedelta
 import socket
 import ssl
 import urllib.error
@@ -25,7 +27,7 @@ from PIL import Image, ImageChops
 
 from src.logging_config import get_logger
 from .bounds_display_manager import BoundsCheckingDisplayManager
-from .loading import load_config_defaults, load_manifest
+from .loading import load_config_defaults, load_manifest, merge_config
 from .sizes import DEFAULT_TEST_SIZES, safe_mode_filename, size_label
 
 logger = get_logger("[Plugin Harness]")
@@ -116,11 +118,20 @@ def list_modes(plugin_instance: Any, manifest: Dict[str, Any], plugin_id: str) -
 
 def _instantiate(plugin_id: str, manifest: Dict[str, Any], plugin_dir: Path,
                  config: Dict[str, Any], mock_data: Dict[str, Any],
-                 display_manager: Any) -> Any:
-    """Load and construct a plugin instance with mocked managers."""
+                 display_manager: Any, cache_manager: Any = None) -> Any:
+    """Load and construct a plugin instance with mocked managers.
+
+    Pass ``cache_manager`` to share one cache across the renders of a plugin.
+    Building a fresh one per (size, mode) made every render a cold start, so a
+    plugin that fetches per game or per player re-fetched everything N times --
+    baseball-scoreboard took 840s for nine renders where ~72s was the arithmetic
+    -- and the cache-hit path, which is what a running rig executes almost
+    always, was never exercised.
+    """
     from src.plugin_system.plugin_loader import PluginLoader
     from src.plugin_system.testing import MockCacheManager, MockPluginManager
 
+    if cache_manager is None:
         cache_manager = MockCacheManager()
         for key, value in (mock_data or {}).items():
             cache_manager.set(key, value)
@@ -160,6 +171,107 @@ def _render_mode(plugin_instance: Any, mode: str) -> Any:
     return plugin_instance.display(force_clear=False)
 
 
+# How many extra frames to drive before believing a mode really draws nothing.
+# A scroll starts with its content off-panel, so frame 1 is legitimately blank;
+# measured across the fleet, content appears by frame 2-4 (f1-scoreboard),
+# frame 4 (ledmatrix-elections) and frame 38 at 64px (ledmatrix-leaderboard).
+EMPTY_RECHECK_FRAMES = 48
+# Seconds to advance the clock between those frames. Scroll position is usually
+# driven by elapsed time, which a frozen clock never provides.
+EMPTY_RECHECK_STEP = 0.05
+
+
+def _has_content(image) -> bool:
+    """True when any pixel is lit above the threshold."""
+    if image is None:
+        return False
+    return image.convert("L").point(
+        lambda p: 255 if p > _LIT_THRESHOLD else 0).getbbox() is not None
+
+
+def _render_mode_again(plugin_instance: Any, mode: str) -> Any:
+    """Draw one more frame WITHOUT force_clear.
+
+    _render_mode passes force_clear=True, which for a scrolling plugin means
+    "reset the scroll to the start" -- so repeating it would redraw frame 1 for
+    ever. The re-check needs the plugin to advance.
+    """
+    sig = inspect.signature(plugin_instance.display)
+    if "display_mode" in sig.parameters:
+        return plugin_instance.display(force_clear=False, display_mode=mode)
+    return plugin_instance.display(force_clear=False)
+
+
+def _settle_empty_frame(inst, mode, dm, result, freezer) -> None:
+    """Give an apparently-empty mode a few frames to draw before believing it.
+
+    One frame is not evidence: a scroll's first frame is its blank scroll-in
+    buffer. Without this, every scrolling plugin was warned about -- 60 of 76
+    warnings on a 44-plugin rig were false, which is the rate at which people
+    stop reading a warning.
+    """
+    if result.error is not None or result.display_returned is False:
+        return
+    if _has_content(result.image):
+        return
+    # The frozen clock is shared by every render in the matrix, so any time this
+    # probe borrows has to be given back -- otherwise a mode that scrolls in
+    # leaves the clock advanced and every later mode renders at the wrong
+    # instant, drifting its golden. Seen as 5 spurious f1_upcoming drifts.
+    resume_at = None
+    if freezer is not None:
+        try:
+            resume_at = freezer()
+        except (AttributeError, TypeError, ValueError):
+            # Not a freezegun factory, or a version whose factory is not
+            # callable. Only used to restore the clock, never load-bearing.
+            resume_at = None
+    try:
+        _settle_loop(inst, mode, dm, result, freezer)
+    finally:
+        if resume_at is not None:
+            try:
+                freezer.move_to(resume_at)
+            except (AttributeError, TypeError, ValueError):
+                pass
+
+
+def _settle_loop(inst, mode, dm, result, freezer) -> None:
+    tick = getattr(freezer, "tick", None) if freezer is not None else None
+    for _ in range(EMPTY_RECHECK_FRAMES):
+        if tick is not None:
+            # timedelta rather than a bare float: freezegun has accepted a
+            # number only since 1.x, and a stale pin would raise here.
+            try:
+                tick(timedelta(seconds=EMPTY_RECHECK_STEP))
+            except (AttributeError, TypeError, ValueError):
+                # Pacing is best-effort; a freezegun that will not take a
+                # timedelta just means this probe runs without advancing time.
+                pass
+        else:
+            # No frozen clock, so the real one has to do the advancing. Without
+            # this the 48 frames run in microseconds, elapsed time stays ~0, and
+            # a scroll driven by elapsed time never moves -- which is exactly
+            # the plugin this check is trying not to slander.
+            time.sleep(EMPTY_RECHECK_STEP)
+        try:
+            result.display_returned = _render_mode_again(inst, mode)
+        except Exception as e:  # noqa: BLE001
+            # Deliberately broad: this calls a plugin's display(), which can
+            # raise anything. Recorded rather than swallowed -- a mode that
+            # renders one good frame and then crashes on the next is broken,
+            # and returning silently here reported it as passing. The frame
+            # already captured stays on the result so the failure is still
+            # inspectable.
+            result.error = repr(e)
+            return
+        image = dm.get_image()
+        if _has_content(image):
+            result.image = image
+            result.overflow = dm.check_overflow()
+            return
+
+
 def _freeze(freeze_time: Optional[str]):
     """Context manager that freezes wall-clock time when freeze_time is given,
     so time-dependent plugins (clocks, countdowns) render deterministic goldens."""
@@ -193,7 +305,9 @@ def render_plugin_matrix(
     manifest = load_manifest(plugin_dir)
     # Start from config_schema.json defaults so the plugin behaves like a real
     # install; explicit caller config still wins over a schema default.
-    config = {"enabled": True, **load_config_defaults(plugin_dir), **(config or {})}
+    config = merge_config(
+        merge_config({"enabled": True}, load_config_defaults(plugin_dir)),
+        config or {})
     sizes = sizes or DEFAULT_TEST_SIZES
     results: List[RenderResult] = []
 
@@ -202,25 +316,35 @@ def render_plugin_matrix(
     # rendering a smaller one, instead of being clipped into a false pass.
     extent = (max(w for w, _ in sizes), max(h for _, h in sizes))
 
-    with _freeze(freeze_time):
+    # One cache for the whole matrix: see _instantiate. The display manager
+    # stays per-render (the bounds checking depends on that); only fetched data
+    # is shared.
+    from src.plugin_system.testing import MockCacheManager
+    cache_manager = MockCacheManager()
+    for key, value in (mock_data or {}).items():
+        cache_manager.set(key, value)
+
+    with _freeze(freeze_time) as freezer:
         for width, height in sizes:
             results.extend(_render_size(
                 plugin_id, manifest, plugin_dir, config, mock_data or {},
-                width, height, run_update, extent,
+                width, height, run_update, extent, cache_manager, freezer,
             ))
 
     return results
 
 
 def _render_size(plugin_id, manifest, plugin_dir, config, mock_data,
-                 width, height, run_update, extent) -> List[RenderResult]:
+                 width, height, run_update, extent,
+                 cache_manager=None, freezer=None) -> List[RenderResult]:
     """Render every mode at one size. A fresh instance per mode avoids state leaks."""
     results: List[RenderResult] = []
 
     # Discover modes once per size (instance build can depend on config).
     try:
         probe_dm = BoundsCheckingDisplayManager(width=width, height=height, overflow_extent=extent)
-        probe = _instantiate(plugin_id, manifest, plugin_dir, config, mock_data, probe_dm)
+        probe = _instantiate(plugin_id, manifest, plugin_dir, config, mock_data, probe_dm,
+                             cache_manager)
         modes = list_modes(probe, manifest, plugin_id)
     except Exception as e:  # noqa: BLE001 — surface any load failure as a result
         return [RenderResult(plugin_id, width, height, "<load>", error=repr(e))]
@@ -229,7 +353,8 @@ def _render_size(plugin_id, manifest, plugin_dir, config, mock_data,
         result = RenderResult(plugin_id, width, height, mode)
         dm = BoundsCheckingDisplayManager(width=width, height=height, overflow_extent=extent)
         try:
-            inst = _instantiate(plugin_id, manifest, plugin_dir, config, mock_data, dm)
+            inst = _instantiate(plugin_id, manifest, plugin_dir, config, mock_data, dm,
+                                cache_manager)
             if run_update:
                 try:
                     inst.update()
@@ -248,6 +373,9 @@ def _render_size(plugin_id, manifest, plugin_dir, config, mock_data,
                 result.display_returned = _render_mode(inst, mode)
                 result.image = dm.get_image()
                 result.overflow = dm.check_overflow()
+                # A blank first frame is not proof of a blank mode; see
+                # _settle_empty_frame.
+                _settle_empty_frame(inst, mode, dm, result, freezer)
         except Exception as e:  # noqa: BLE001 — a display crash is a real failure
             result.error = repr(e)
         results.append(result)

src/plugin_system/testing/loading.py

@@ -33,6 +33,45 @@ def load_manifest(plugin_dir: Union[str, Path]) -> Dict[str, Any]:
         return json.load(f)
 
 
+def _defaults_from_properties(properties: Dict[str, Any]) -> Dict[str, Any]:
+    """Defaults for one `properties` block, recursing into nested objects.
+
+    An object property carries its defaults on its children, not on itself, so
+    reading only the top level dropped everything nested. That is most of the
+    fleet: config organised by league, or under customization/display_options,
+    lost 2,386 defaults across 37 of 44 plugins -- soccer-scoreboard alone lost
+    539 of 565 -- and the harness rendered them with a config no install would
+    ever have.
+    """
+    defaults: Dict[str, Any] = {}
+    for key, prop in (properties or {}).items():
+        if not isinstance(prop, dict):
+            continue
+        if prop.get('type') == 'object' and isinstance(prop.get('properties'), dict):
+            nested = _defaults_from_properties(prop['properties'])
+            if nested:
+                defaults[key] = nested
+        elif 'default' in prop:
+            defaults[key] = prop['default']
+    return defaults
+
+
+def merge_config(base: Dict[str, Any], override: Dict[str, Any]) -> Dict[str, Any]:
+    """Deep-merge override onto base, without dropping sibling defaults.
+
+    A shallow merge would let `-c '{"nhl": {"enabled": true}}'` replace the whole
+    nhl subtree and silently discard every other nhl default -- the same class of
+    bug this function exists to fix.
+    """
+    merged = dict(base)
+    for key, value in (override or {}).items():
+        if isinstance(value, dict) and isinstance(merged.get(key), dict):
+            merged[key] = merge_config(merged[key], value)
+        else:
+            merged[key] = value
+    return merged
+
+
 def load_config_defaults(plugin_dir: Union[str, Path]) -> Dict[str, Any]:
     """Extract default values from a plugin's config_schema.json (empty if none)."""
     schema_path = Path(plugin_dir) / 'config_schema.json'
@@ -40,11 +79,7 @@ def load_config_defaults(plugin_dir: Union[str, Path]) -> Dict[str, Any]:
         return {}
     with open(schema_path, 'r') as f:
         schema = json.load(f)
-    defaults: Dict[str, Any] = {}
-    for key, prop in schema.get('properties', {}).items():
-        if isinstance(prop, dict) and 'default' in prop:
-            defaults[key] = prop['default']
-    return defaults
+    return _defaults_from_properties(schema.get('properties', {}))
 
 
 def load_harness_spec(plugin_dir: Union[str, Path]) -> Dict[str, Any]:

src/plugin_system/testing/visual_display_manager.py

@@ -507,7 +507,15 @@ class VisualTestDisplayManager:
     # ------------------------------------------------------------------
 
     def set_scrolling_state(self, is_scrolling: bool):
-        """Set the current scrolling state (no-op for testing)."""
+        """Set the current scrolling state (no-op for testing).
+
+        Deliberately mirrors DisplayManager.set_scrolling_state exactly. A
+        double that accepts arguments production does not lets a call pass
+        every harness run and then raise TypeError on the panel, which is the
+        one failure a safety harness exists to prevent. ``frame_hold`` arrives
+        here in the same change that adds it to DisplayManager (#523), not
+        before.
+        """
         self._scrolling_state['is_scrolling'] = is_scrolling
         if is_scrolling:
             self._scrolling_state['last_scroll_activity'] = time.time()

test/test_harness_empty_claimed.py

@@ -101,3 +101,61 @@ class TestSkippedResults:
         r = _result(None, returned=None)
         check_empty_claimed([r], strict=True)
         assert r.empty_claimed is None
+
+
+class TestSettleRecordsLaterFailures:
+    """A mode that renders one good frame and then crashes is broken.
+
+    _settle_loop re-renders a mode that came back blank, to give a scroll or an
+    animation time to put something on the panel. Swallowing an exception from
+    those later frames meant the harness reported a passing result for a mode
+    that crashes as soon as it is asked for a second frame -- exactly the kind
+    of defect the harness exists to catch.
+    """
+
+    class Boom:
+        """Renders once, then raises."""
+
+        def __init__(self):
+            self.calls = 0
+
+        def display(self, force_clear=False):
+            self.calls += 1
+            raise RuntimeError("second frame exploded")
+
+    def _settle(self, inst, dm, result):
+        from src.plugin_system.testing import harness
+        harness._settle_loop(inst, "mode", dm, result, None)
+
+    def test_the_exception_is_recorded_on_the_result(self, monkeypatch):
+        from src.plugin_system.testing import harness
+        # Keep the probe short; this test is about the error, not the pacing.
+        monkeypatch.setattr(harness, "EMPTY_RECHECK_FRAMES", 1)
+        monkeypatch.setattr(harness, "EMPTY_RECHECK_STEP", 0)
+
+        result = _result(_blank())
+        assert result.error is None
+        self._settle(self.Boom(), _FakeDM(), result)
+
+        assert result.error is not None, "a crash on a later frame was swallowed"
+        assert "second frame exploded" in result.error
+
+    def test_the_already_captured_frame_is_kept(self, monkeypatch):
+        from src.plugin_system.testing import harness
+        monkeypatch.setattr(harness, "EMPTY_RECHECK_FRAMES", 1)
+        monkeypatch.setattr(harness, "EMPTY_RECHECK_STEP", 0)
+
+        image = _blank()
+        result = _result(image)
+        self._settle(self.Boom(), _FakeDM(), result)
+        assert result.image is image, "the good frame was discarded along with the error"
+
+
+class _FakeDM:
+    """Minimal display-manager double for _settle_loop."""
+
+    def get_image(self):
+        return _blank()
+
+    def check_overflow(self):
+        return None

test/test_on_demand_mailbox.py

@@ -0,0 +1,106 @@
+"""The on-demand request mailbox: how often it is read, and how it is consumed.
+
+The mailbox is a cache key the web process writes and the display process
+reads. Two properties matter and neither is obvious from the call site:
+
+  * it is polled after every rendered frame, so an uncached read here is a
+    disk read at frame rate;
+  * consuming it must not throw away a request that arrived while the previous
+    one was being processed.
+"""
+
+from unittest.mock import MagicMock
+
+import pytest
+
+
+class TestPollingIsBounded:
+    """_poll_on_demand_requests runs ~125x/second on a scrolling mode.
+
+    The read is deliberately uncached (memory_ttl=0) because a cached one
+    pinned the first request for an hour. That makes the call a real disk read,
+    so it needs a floor -- without one it was ~125 reads per second to find
+    nothing at all.
+    """
+
+    def test_first_call_always_reads(self, test_display_controller):
+        c = test_display_controller
+        c.cache_manager.get = MagicMock(return_value=None)
+        c._poll_on_demand_requests()
+        assert c.cache_manager.get.call_count == 1
+
+    def test_immediate_second_call_does_not_read(self, test_display_controller):
+        c = test_display_controller
+        c.cache_manager.get = MagicMock(return_value=None)
+        c._poll_on_demand_requests()
+        for _ in range(50):
+            c._poll_on_demand_requests()
+        assert c.cache_manager.get.call_count == 1, "polling was not bounded"
+
+    def test_reads_again_once_the_interval_has_passed(self, test_display_controller, monkeypatch):
+        c = test_display_controller
+        c.cache_manager.get = MagicMock(return_value=None)
+        clock = {"t": 1000.0}
+        monkeypatch.setattr("src.display_controller.time.monotonic", lambda: clock["t"])
+
+        c._poll_on_demand_requests()
+        clock["t"] += c.ON_DEMAND_POLL_INTERVAL / 2
+        c._poll_on_demand_requests()
+        assert c.cache_manager.get.call_count == 1, "read before the interval elapsed"
+
+        clock["t"] += c.ON_DEMAND_POLL_INTERVAL
+        c._poll_on_demand_requests()
+        assert c.cache_manager.get.call_count == 2
+
+    def test_the_interval_is_short_enough_to_feel_instant(self, test_display_controller):
+        # A person clicking in the web UI must not notice the floor.
+        assert test_display_controller.ON_DEMAND_POLL_INTERVAL <= 0.5
+
+
+class TestMailboxIsConsumedByIdentity:
+    """Deleting whatever is in the mailbox loses a request that raced in."""
+
+    def _arrange(self, controller, first, later):
+        """Mailbox returns `first`, then `later` on the pre-delete re-read."""
+        controller.on_demand_active = False
+        controller.on_demand_request_id = None
+        controller._last_on_demand_poll = None
+        reads = iter([first, later])
+
+        def fake_get(key, *a, **kw):
+            if key == 'display_on_demand_request':
+                return next(reads, later)
+            return None  # processed-id lookup
+
+        controller.cache_manager.get = MagicMock(side_effect=fake_get)
+        controller.cache_manager.set = MagicMock()
+        controller.cache_manager.delete = MagicMock()
+        controller._activate_on_demand = MagicMock()
+
+    REQ_A = {'request_id': 'A', 'action': 'start', 'plugin_id': 'p', 'mode': 'm'}
+    REQ_B = {'request_id': 'B', 'action': 'start', 'plugin_id': 'p', 'mode': 'm'}
+
+    def test_own_request_is_deleted(self, test_display_controller):
+        c = test_display_controller
+        self._arrange(c, self.REQ_A, self.REQ_A)
+        c._poll_on_demand_requests()
+        c.cache_manager.delete.assert_called_once_with('display_on_demand_request')
+
+    def test_a_newer_request_is_left_for_the_next_poll(self, test_display_controller):
+        c = test_display_controller
+        self._arrange(c, self.REQ_A, self.REQ_B)
+        c._poll_on_demand_requests()
+        assert c.cache_manager.delete.call_count == 0, \
+            "request B was deleted without ever being processed"
+
+    def test_an_already_empty_mailbox_is_still_cleared(self, test_display_controller):
+        c = test_display_controller
+        self._arrange(c, self.REQ_A, None)
+        c._poll_on_demand_requests()
+        c.cache_manager.delete.assert_called_once_with('display_on_demand_request')
+
+    def test_the_request_is_still_processed(self, test_display_controller):
+        c = test_display_controller
+        self._arrange(c, self.REQ_A, self.REQ_B)
+        c._poll_on_demand_requests()
+        c._activate_on_demand.assert_called_once()

test/test_starlark_display_contract.py

@@ -0,0 +1,82 @@
+"""starlark-apps: display() must report what actually reached the panel.
+
+The display controller skips a mode only on a boolean False. Returning True
+after the frame update failed told it the mode had rendered, so it held a dead
+frame for the whole display_duration instead of rotating on -- the same class
+of defect as a display() that returns None.
+"""
+
+import sys
+from pathlib import Path
+from unittest.mock import MagicMock
+
+import pytest
+
+PLUGIN_DIR = Path(__file__).resolve().parent.parent / "plugin-repos" / "starlark-apps"
+
+
+@pytest.fixture(scope="module")
+def manager_module():
+    if not PLUGIN_DIR.exists():
+        pytest.skip("starlark-apps plugin is not checked out")
+    sys.path.insert(0, str(PLUGIN_DIR))
+    try:
+        import importlib
+        spec = importlib.util.spec_from_file_location(
+            "starlark_manager_under_test", PLUGIN_DIR / "manager.py")
+        module = importlib.util.module_from_spec(spec)
+        spec.loader.exec_module(module)
+        return module
+    except Exception as e:  # noqa: BLE001 - optional deps (pixlet, fcntl) may be absent
+        pytest.skip(f"starlark-apps manager is not importable here: {e}")
+    finally:
+        sys.path.remove(str(PLUGIN_DIR))
+
+
+def _plugin(manager_module):
+    """A manager with __init__ bypassed -- only display paths are under test."""
+    cls = manager_module.StarlarkAppsPlugin
+    inst = cls.__new__(cls)
+    inst.logger = MagicMock()
+    inst.display_manager = MagicMock()
+    inst.current_app = None
+    return inst
+
+
+class _App:
+    def __init__(self, frames):
+        self.frames = frames
+        self.current_frame_index = 0
+        self.last_frame_time = 0.0
+        self.app_id = "app"
+
+
+class TestDisplayFramePropagates:
+    def test_a_failed_update_returns_false(self, manager_module):
+        p = _plugin(manager_module)
+        p.current_app = _App([("frame", 100)])
+        p.display_manager.update_display.side_effect = RuntimeError("panel gone")
+
+        assert p._display_frame() is False
+
+    def test_a_good_update_returns_true(self, manager_module):
+        p = _plugin(manager_module)
+        p.current_app = _App([("frame", 100)])
+
+        assert p._display_frame() is True
+
+    def test_no_frames_returns_false(self, manager_module):
+        p = _plugin(manager_module)
+        p.current_app = _App([])
+
+        assert p._display_frame() is False
+
+    def test_display_reports_the_frame_failure(self, manager_module):
+        p = _plugin(manager_module)
+        p.current_app = _App([("frame", 100)])
+        p.display_manager.update_display.side_effect = RuntimeError("panel gone")
+
+        result = p.display()
+
+        assert result is False, "display() claimed success over a failed frame update"
+        assert isinstance(result, bool), "the controller only skips on a real bool"

web_interface/blueprints/api_v3.py

@@ -78,8 +78,11 @@ def _scrub_git_remote_url(url: str) -> str:
     return url
 
 # Will be initialized when blueprint is registered
-config_manager = None
-plugin_manager = None
+# NOTE: the managers live on the blueprint object (app.py sets
+# api_v3.config_manager / api_v3.plugin_manager). Deliberately not
+# mirrored as module globals: a bare `config_manager` used to resolve to
+# a None that was never assigned, which silently disabled the /health
+# checks and made /display/current fall back to a hardcoded 128x64.
 plugin_store_manager = None
 saved_repositories_manager = None
 cache_manager = None
@@ -1598,11 +1601,16 @@ def get_health():
         }
 
         # Check web interface service
+        # Stamp the start time before measuring against it -- reading it with a
+        # fallback of time.time() and only assigning afterwards made the very
+        # first call subtract two separate clock reads, reporting a small
+        # negative uptime.
+        if not hasattr(get_health, '_start_time'):
+            get_health._start_time = time.time()
         health_status['services']['web_interface'] = {
             'status': 'running',
-            'uptime_seconds': time.time() - (getattr(get_health, '_start_time', time.time()))
+            'uptime_seconds': time.time() - get_health._start_time
         }
-        get_health._start_time = getattr(get_health, '_start_time', time.time())
 
         # Check display service
         display_service_status = _get_display_service_status()
@@ -1613,8 +1621,8 @@ def get_health():
 
         # Check config file accessibility
         try:
-            if config_manager:
-                test_config = config_manager.load_config()
+            if api_v3.config_manager:
+                test_config = api_v3.config_manager.load_config()
                 health_status['checks']['config_file'] = {
                     'status': 'accessible',
                     'readable': True
@@ -1633,9 +1641,9 @@ def get_health():
 
         # Check plugin system
         try:
-            if plugin_manager:
+            if api_v3.plugin_manager:
                 # Try to discover plugins (lightweight check)
-                plugin_count = len(plugin_manager.get_available_plugins()) if hasattr(plugin_manager, 'get_available_plugins') else 0
+                plugin_count = len(api_v3.plugin_manager.get_available_plugins()) if hasattr(api_v3.plugin_manager, 'get_available_plugins') else 0
                 health_status['checks']['plugin_system'] = {
                     'status': 'operational',
                     'plugin_count': plugin_count
@@ -2468,8 +2476,8 @@ def get_display_current():
 
         # Get display dimensions from config
         try:
-            if config_manager:
-                main_config = config_manager.load_config()
+            if api_v3.config_manager:
+                main_config = api_v3.config_manager.load_config()
                 hardware_config = main_config.get('display', {}).get('hardware', {})
                 cols = hardware_config.get('cols', 64)
                 chain_length = hardware_config.get('chain_length', 2)