test(starlark): stop the pixlet-editor route tests leaking a fake Popen into the captive-portal check

TestPixletEditorHostDefaultsButDoesNotOverride patched Popen on the shared
subprocess module. The app's before_request captive-portal hook runs
subprocess.run (`with Popen(...) as p`) for `systemctl is-active hostapd`
whenever its 30s AP-mode cache is cold, so on a Linux host with systemctl -
the CI runner - the hook got the FakeProcess and the request 500'd with
"'FakeProcess' object does not support the context manager protocol".
Whether it happened depended on how long ago the previous request ran,
hence the intermittency; Windows has no systemctl and never reached it.

- Patch the starlark route module's own `subprocess` binding instead of
  the global Popen.
- Pin is_ap_mode_active to False in this file's client fixture, so no
  request here shells out to systemctl/nmcli at all.

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

CHANGELOG.md

@@ -19,6 +19,18 @@ accepts both, but the store flags the old spelling as deprecated
 
 ## Unreleased
 
+### A scrolling screen held by its plugin's update() is reported
+
+- While a plugin's `update()` runs it holds the plugin's lock, and that
+  plugin's frames are skipped: on a scroller, a frozen strip, with nothing
+  logged (and a freeze of 5 s or more is a gap, not a freeze, to the frame
+  stats). The high-FPS loop now times each run of skipped frames; one of
+  250 ms or more logs `Display of <plugin> held N ms by its update()`
+  (rate-limited per plugin) when it ends, and is recorded on the plugin's
+  health as a `display hold` busy skip, which never counts toward the
+  circuit breaker. The 1 Hz loop is left out: its frames are a second apart,
+  so one skipped frame there measures nothing and freezes nothing visible.
+
 ### Fixed
 
 - The web preview and `/api/v3/display/current` no longer stay black for a
@@ -1417,17 +1429,6 @@ read any of them:
 
 ### Fixes
 
-- Updating a plugin from the store no longer deletes the files it wrote
-  beside itself. A monorepo update replaces the plugin directory with the
-  fresh download and deletes the old copy, so calendar's Google OAuth files
-  (`token.pickle`, `credentials.json`) were lost on every update and the
-  calendar stopped until they were restored by hand. Before the old copy is
-  removed, the update now copies over anything the plugin's `.gitignore`
-  excludes plus known secret/state files (`*.pickle`, `token.json`,
-  `credentials.json`, `config_secrets.json`, `.pkce_code_verifier`); files the
-  new release ships are never overwritten, and byte code is not carried. A
-  plugin updated with `git pull` no longer sweeps an untracked token into the
-  auto-stash, which is never popped (`src/plugin_system/plugin_local_files.py`).
 - Quieter routine logging. Every rotation logged each mode twice
   ("Switching to mode", then "Processing mode"), and a mode with nothing to
   show added "display() returned False" and "No content to display". Those

src/display_controller.py

@@ -1164,6 +1164,55 @@ class DisplayController:
         except Exception:  # pylint: disable=broad-except
             logger.exception("Error running scheduled plugin updates")
 
+    #: A run of frames skipped because a plugin's update() held its lock is
+    #: reported once it has lasted this long.
+    DISPLAY_HOLD_REPORT_SECONDS = 0.25
+
+    #: (plugin_id, monotonic start) of the current run of skipped frames.
+    _display_hold: Optional[Tuple[str, float]] = None
+
+    def _note_display_hold(self, plugin_id: str, held: bool) -> None:
+        """Report how long a plugin's update() kept its display() from drawing.
+
+        While update() runs on the worker it holds the plugin's lock, and every
+        frame of that plugin's screen is skipped: the panel keeps showing the
+        last frame, which on a scroller is a frozen strip. Nothing said so --
+        the frames are not failures, and a scroll freeze of 5 s or more is a
+        gap to the frame stats, not a freeze. This times each such run and,
+        when it ends after DISPLAY_HOLD_REPORT_SECONDS or more, logs it
+        (rate-limited per plugin) and records it on the plugin's health as a
+        busy skip, which never touches the circuit breaker. Only frames of the
+        high-FPS loop are timed (see _display_once's ``report_hold``).
+        """
+        # The clock is read only when a run starts or ends: on a frame that
+        # draws with no run open, this is one attribute check.
+        current = self._display_hold
+        if held:
+            if current is None or current[0] != plugin_id:
+                self._display_hold = (plugin_id, time.monotonic())
+            return
+        if current is None:
+            return
+        self._display_hold = None
+        if current[0] != plugin_id:
+            return
+        seconds = time.monotonic() - current[1]
+        if seconds < self.DISPLAY_HOLD_REPORT_SECONDS:
+            return
+        pm = self.plugin_manager
+        warn = getattr(pm, '_warn_rate_limited', None)
+        if warn is not None:
+            warn(f"display-hold:{plugin_id}",
+                 "Display of %s held %.0f ms by its update()",
+                 plugin_id, seconds * 1000.0)
+        tracker = getattr(pm, 'health_tracker', None)
+        record = getattr(tracker, 'record_busy_skip', None)
+        if record is not None:
+            try:
+                record(plugin_id, "display hold", seconds)
+            except Exception:  # pylint: disable=broad-except
+                logger.debug("Could not record a display hold", exc_info=True)
+
     @contextmanager
     def _display_lock_or_skip(self, plugin_id):
         """Try-lock guard keeping a plugin's display() off its in-flight update().
@@ -1189,7 +1238,7 @@ class DisplayController:
             lock.release()
 
     def _display_once(self, plugin, mode: str, accepts_display_mode: bool,
-                      force_clear: bool = False):
+                      force_clear: bool = False, report_hold: bool = False):
         """Call ``plugin.display()`` directly for one frame of a render loop.
 
         Frames after a screen's first dispatch come through here rather than
@@ -1205,6 +1254,12 @@ class DisplayController:
                 ``display_mode`` so plugins with several modes stay on it.
             accepts_display_mode: Whether display() takes ``display_mode``.
             force_clear: Passed through to display().
+            report_hold: Time runs of frames skipped because update() holds
+                the plugin's lock (see _note_display_hold). Only the high-FPS
+                loop asks: its frames are ~8 ms apart, so a run measures the
+                hold, and a held scroller is a frozen strip. The 1 Hz loop's
+                frames are a second apart, so one skipped frame there would
+                read as a 1 s hold of a screen that did not visibly change.
 
         Each call is timed (two monotonic reads) and handed to
         PluginManager.note_display_duration, which logs and records slow
@@ -1219,6 +1274,12 @@ class DisplayController:
         display_watchdog.watchdog.beat()
         plugin_id = getattr(plugin, 'plugin_id', None)
         with self._display_lock_or_skip(plugin_id) as can_display:
+            if report_hold and plugin_id:
+                self._note_display_hold(plugin_id, held=not can_display)
+            elif can_display and self._display_hold is not None:
+                # A drawn frame outside the high-FPS loop: whatever run was
+                # open is over, unreported.
+                self._display_hold = None
             if not can_display:
                 return True
             started = time.monotonic()
@@ -3987,7 +4048,8 @@ class DisplayController:
                             _frame_start = time.perf_counter()
                             try:
                                 result = self._display_once(
-                                    manager_to_display, active_mode, _accepts_display_mode)
+                                    manager_to_display, active_mode, _accepts_display_mode,
+                                    report_hold=True)
                                 if isinstance(result, bool) and not result:
                                     logger.debug("Display returned False, breaking early")
                                     break

src/plugin_system/plugin_local_files.py

@@ -1,204 +0,0 @@
-"""
-Files a plugin writes beside itself at runtime, which an update must keep.
-
-A store update replaces a plugin's directory with a fresh download and then
-deletes the old copy. Anything the plugin created there -- OAuth tokens, a
-client-secrets file, a PKCE verifier, cached state -- is in no release, so the
-fresh download does not contain it and deleting the old copy destroys it. On
-2026-10-04 updating calendar 1.2.9 -> 1.2.12 that way deleted its
-``token.pickle`` and ``credentials.json``, and the calendar stopped until they
-were restored from a backup.
-
-What counts as "the plugin's own local file" is the union of:
-
-* :data:`KNOWN_STATE_PATTERNS` -- secret and state files plugins are known to
-  write, kept even when a plugin forgot to gitignore them; and
-* whatever the plugin's own ``.gitignore`` (old copy or new) excludes. A file
-  the author ignores is by definition not part of a release.
-
-A file the new release ships is never overwritten: tracked content wins. Byte
-code (``__pycache__``, ``*.pyc``) and ``.git`` are never carried, since they
-belong to the old code rather than to the user.
-"""
-
-from __future__ import annotations
-
-import fnmatch
-import os
-import re
-import shutil
-from pathlib import Path
-from typing import Iterable, List, Optional, Pattern, Tuple
-
-__all__ = [
-    'KNOWN_STATE_PATTERNS',
-    'carry_over_local_files',
-    'is_known_state_file',
-    'local_files_to_keep',
-]
-
-# Basename globs. Kept even when the plugin's .gitignore does not list them.
-KNOWN_STATE_PATTERNS: Tuple[str, ...] = (
-    'token.pickle',
-    '*.pickle',
-    'token.json',
-    'credentials.json',
-    'config_secrets.json',
-    '.pkce_code_verifier',
-)
-
-_NEVER_CARRY_DIRS = frozenset({'.git', '__pycache__'})
-_NEVER_CARRY_SUFFIXES = ('.pyc', '.pyo')
-
-
-def is_known_state_file(rel_path: str) -> bool:
-    """True when ``rel_path``'s basename is a known secret/state file."""
-    name = rel_path.replace('\\', '/').rsplit('/', 1)[-1]
-    return any(fnmatch.fnmatchcase(name, p) for p in KNOWN_STATE_PATTERNS)
-
-
-class _GitIgnore:
-    """The subset of gitignore semantics plugin .gitignore files use.
-
-    Supports comments, ``!`` negation (last match wins), a trailing ``/`` for
-    directory-only patterns, anchoring by a leading or embedded ``/``, ``*``,
-    ``?``, ``[...]`` and ``**``. As in git, a file under an ignored directory
-    is ignored regardless of later negations.
-    """
-
-    def __init__(self, lines: Iterable[str]):
-        self._rules: List[Tuple[Pattern[str], bool, bool]] = []
-        for raw in lines:
-            line = raw.rstrip('\n').rstrip()
-            if not line or line.startswith('#'):
-                continue
-            negate = line.startswith('!')
-            if negate:
-                line = line[1:]
-            elif line.startswith('\\'):
-                line = line[1:]
-            dir_only = line.endswith('/')
-            line = line.rstrip('/')
-            if not line:
-                continue
-            anchored = '/' in line
-            line = line.lstrip('/')
-            body = self._translate(line)
-            regex = body if anchored else r'(?:.*/)?' + body
-            self._rules.append((re.compile(r'\A' + regex + r'\Z'), negate, dir_only))
-
-    @staticmethod
-    def _translate(pattern: str) -> str:
-        out, i, n = [], 0, len(pattern)
-        while i < n:
-            if pattern.startswith('**/', i):
-                out.append(r'(?:.*/)?')
-                i += 3
-            elif pattern.startswith('/**', i) and i + 3 == n:
-                out.append(r'/.*')
-                i += 3
-            elif pattern.startswith('**', i):
-                out.append(r'.*')
-                i += 2
-            elif pattern[i] == '*':
-                out.append(r'[^/]*')
-                i += 1
-            elif pattern[i] == '?':
-                out.append(r'[^/]')
-                i += 1
-            elif pattern[i] == '[':
-                end = pattern.find(']', i + 1)
-                if end == -1:
-                    out.append(re.escape('['))
-                    i += 1
-                else:
-                    cls = pattern[i + 1:end]
-                    if cls.startswith('!'):
-                        cls = '^' + cls[1:]
-                    out.append('[' + cls.replace('\\', '\\\\') + ']')
-                    i = end + 1
-            else:
-                out.append(re.escape(pattern[i]))
-                i += 1
-        return ''.join(out)
-
-    def _decide(self, rel: str, is_dir: bool) -> Optional[bool]:
-        verdict = None
-        for regex, negate, dir_only in self._rules:
-            if dir_only and not is_dir:
-                continue
-            if regex.match(rel):
-                verdict = not negate
-        return verdict
-
-    def ignores(self, rel_path: str) -> bool:
-        if not self._rules:
-            return False
-        parts = rel_path.replace('\\', '/').split('/')
-        for depth in range(1, len(parts)):
-            if self._decide('/'.join(parts[:depth]), True):
-                return True
-        return bool(self._decide('/'.join(parts), False))
-
-
-def _read_gitignore(plugin_dir: Path) -> List[str]:
-    try:
-        return (plugin_dir / '.gitignore').read_text(
-            encoding='utf-8', errors='replace').splitlines()
-    except OSError:
-        return []
-
-
-def local_files_to_keep(old_dir: Path, new_dir: Path) -> List[str]:
-    """Relative paths (``/``-separated) in ``old_dir`` to copy into ``new_dir``.
-
-    Regular files only; symlinks and anything the new release already ships
-    are skipped.
-    """
-    old_dir, new_dir = Path(old_dir), Path(new_dir)
-    ignore = _GitIgnore(_read_gitignore(old_dir) + _read_gitignore(new_dir))
-    keep: List[str] = []
-    for root, dirs, files in os.walk(old_dir):
-        dirs[:] = sorted(d for d in dirs if d not in _NEVER_CARRY_DIRS
-                         and not os.path.islink(os.path.join(root, d)))
-        rel_root = os.path.relpath(root, old_dir)
-        for name in sorted(files):
-            if name.endswith(_NEVER_CARRY_SUFFIXES):
-                continue
-            full = os.path.join(root, name)
-            if os.path.islink(full) or not os.path.isfile(full):
-                continue
-            rel = name if rel_root == '.' else f"{rel_root}/{name}".replace('\\', '/')
-            if not (is_known_state_file(rel) or ignore.ignores(rel)):
-                continue
-            if os.path.lexists(new_dir / rel):
-                continue
-            keep.append(rel)
-    return keep
-
-
-def carry_over_local_files(
-    old_dir: Path, new_dir: Path
-) -> Tuple[List[str], List[Tuple[str, str]]]:
-    """Copy the plugin's local files from ``old_dir`` into ``new_dir``.
-
-    Copies rather than moves, so ``old_dir`` stays a complete copy until the
-    caller deletes it. Returns ``(copied, failed)`` where ``failed`` pairs a
-    relative path with the error; the caller should keep ``old_dir`` when
-    anything failed.
-    """
-    copied: List[str] = []
-    failed: List[Tuple[str, str]] = []
-    try:
-        candidates = local_files_to_keep(old_dir, new_dir)
-    except OSError as e:
-        return copied, [('.', str(e))]
-    for rel in candidates:
-        dest = Path(new_dir) / rel
-        try:
-            dest.parent.mkdir(parents=True, exist_ok=True)
-            shutil.copy2(Path(old_dir) / rel, dest)
-            copied.append(rel)
-        except OSError as e:
-            failed.append((rel, str(e)))
-    return copied, failed

src/plugin_system/store_install.py

@@ -22,7 +22,6 @@ from src.plugin_system.plugin_loader import (
     contained_plugin_dir, requirements_to_install,
 )
 from src.plugin_system.plugin_dirs import BACKUP_MARKER
-from src.plugin_system.plugin_local_files import carry_over_local_files
 from src.plugin_system.repo_urls import (
     USER_AGENT, github_api_headers, github_owner_repo, normalize_repo_url,
 )
@@ -93,9 +92,7 @@ class _InstallMixin:
                 raise
 
             if installed:
-                self._discard_backup(
-                    plugin_id, backup_path, "install",
-                    new_path=self._existing_install(plugin_id) or plugin_path)
+                self._discard_backup(plugin_id, backup_path, "install")
                 return True
 
             self._restore_backup(plugin_id, plugin_path, backup_path, "Install")
@@ -136,33 +133,8 @@ class _InstallMixin:
             return f"could not set aside {plugin_path}: {e}"
         return None
 
-    def _discard_backup(
-        self, plugin_id: str, backup_path: Path, action: str,
-        new_path: Optional[Path] = None,
-    ) -> None:
-        """Remove the set-aside copy after a successful (re)install.
-
-        With ``new_path`` (where the new copy landed), first carries the
-        plugin's own runtime files -- OAuth tokens, client secrets, anything
-        its .gitignore excludes -- from the old copy into the new one: no
-        release contains them, so deleting the old copy would destroy them.
-        See src/plugin_system/plugin_local_files.py. If any could not be
-        copied the old copy is kept, so nothing is lost.
-        """
-        if new_path is not None and new_path.is_dir():
-            copied, failed = carry_over_local_files(backup_path, new_path)
-            if copied:
-                self.logger.info(
-                    "Kept %d local file(s) of %s across the %s: %s",
-                    len(copied), plugin_id, action, ", ".join(copied))
-            if failed:
-                self.logger.error(
-                    "Could not carry %s's local files into the new copy (%s); "
-                    "the previous copy is kept at %s -- copy them back by hand",
-                    plugin_id,
-                    "; ".join(f"{rel}: {err}" for rel, err in failed),
-                    backup_path)
-                return
+    def _discard_backup(self, plugin_id: str, backup_path: Path, action: str) -> None:
+        """Remove the set-aside copy after a successful (re)install."""
         if not self._safe_remove_directory(backup_path):
             self.logger.warning(
                 "%s of %s succeeded but the previous copy at %s could not be "
@@ -570,8 +542,7 @@ class _InstallMixin:
                     raise
                 temp_dir = None  # Prevent cleanup since we moved it
                 if backup_path is not None:
-                    self._discard_backup(
-                        plugin_id, backup_path, "install", new_path=final_path)
+                    self._discard_backup(plugin_id, backup_path, "install")
 
             # Install dependencies
             self._install_dependencies(final_path)

src/plugin_system/store_update.py

@@ -10,9 +10,6 @@ import subprocess  # nosec B404 - list-form argv only, no shell  # nosemgrep
 from pathlib import Path
 from typing import Dict, Optional, Tuple
 from src.plugin_system.plugin_dirs import BACKUP_MARKER
-from src.plugin_system.plugin_local_files import (
-    KNOWN_STATE_PATTERNS, is_known_state_file,
-)
 from src.plugin_system.repo_urls import same_repo
 
 
@@ -305,11 +302,7 @@ class _UpdateMixin:
                 installed = False
 
             if installed:
-                # install_plugin may land the new copy under the manifest id
-                # rather than the old directory name.
-                self._discard_backup(
-                    plugin_id, backup_path, "update",
-                    new_path=self._existing_install(plugin_id) or plugin_path)
+                self._discard_backup(plugin_id, backup_path, "update")
                 return True
 
             # Bad network, registry error...: the user keeps a working plugin.
@@ -516,12 +509,8 @@ class _UpdateMixin:
                             for line in untracked_result.stdout.strip().split('\n'):
                                 if line.startswith('??'):
                                     # Untracked file
-                                    file_path = line[3:].strip().strip('"')
-                                    # Tokens and secrets stay out of the
-                                    # stash (see below), so they alone are
-                                    # not a reason to stash.
-                                    if not is_known_state_file(file_path):
-                                        untracked_files.append(file_path)
+                                    file_path = line[3:].strip()
+                                    untracked_files.append(file_path)
                         
                         # Check for tracked file changes
                         status_result = subprocess.run(
@@ -548,17 +537,9 @@ class _UpdateMixin:
                     if has_changes:
                         self.logger.info(f"Stashing local changes in {plugin_id} before update")
                         try:
-                            # Use -u to include untracked files in stash --
-                            # except the plugin's tokens and secrets, which a
-                            # repo may have forgotten to gitignore. The stash
-                            # is never popped, so a stashed token.pickle would
-                            # vanish from the plugin and break it.
-                            stash_cmd = (
-                                ['git', '-C', str(plugin_path), 'stash', 'push', '-u',
-                                 '-m', f'LEDMatrix auto-stash before update {plugin_id}', '--', '.']
-                                + [f':(exclude,glob)**/{p}' for p in KNOWN_STATE_PATTERNS])
+                            # Use -u to include untracked files in stash
                             stash_result = subprocess.run(
-                                stash_cmd,
+                                ['git', '-C', str(plugin_path), 'stash', 'push', '-u', '-m', f'LEDMatrix auto-stash before update {plugin_id}'],
                                 capture_output=True,
                                 text=True,
                                 timeout=30,

test/test_display_hold_report.py

@@ -0,0 +1,142 @@
+"""The report of a scrolling screen held by its plugin's update().
+
+While a plugin's update() runs it holds the plugin's lock, and its screen's
+frames are skipped -- on a scroller, a frozen strip -- with nothing logged.
+_note_display_hold times each such run and reports one of
+DISPLAY_HOLD_REPORT_SECONDS or more.
+"""
+
+import threading
+import types
+from unittest.mock import MagicMock
+
+import pytest
+
+
+class _Clock:
+    """display_controller's clock: moves only when run() sleeps or a test says."""
+
+    def __init__(self, start=10_000.0):
+        self.t = start
+
+    def now(self):
+        return self.t
+
+    def sleep(self, seconds):
+        self.t += max(seconds, 0.0005)
+
+    def module(self):
+        return types.SimpleNamespace(time=self.now, monotonic=self.now,
+                                     perf_counter=self.now, sleep=self.sleep)
+
+
+@pytest.fixture
+def clock(monkeypatch):
+    c = _Clock()
+    monkeypatch.setattr("src.display_controller.time", c.module())
+    return c
+
+
+class _Locks:
+    """get_plugin_lock for one plugin, whose lock the test can hold."""
+
+    def __init__(self):
+        self.lock = threading.Lock()
+
+    def __call__(self, plugin_id):
+        return self.lock
+
+
+@pytest.fixture
+def held(test_display_controller):
+    c = test_display_controller
+    locks = _Locks()
+    c.plugin_manager.get_plugin_lock = locks
+    c.plugin_manager._warn_rate_limited = MagicMock()
+    c.plugin_manager.health_tracker = MagicMock()
+    c._display_hold = None
+    return c, locks.lock
+
+
+def _plugin(plugin_id):
+    p = MagicMock()
+    p.plugin_id = plugin_id
+    p.display.return_value = True
+    return p
+
+
+class TestTheDisplayHoldReport:
+    def test_a_long_hold_is_reported_when_it_ends(self, held, clock):
+        c, lock = held
+        ticker = _plugin("ticker")
+        lock.acquire()                         # update() running
+        assert c._display_once(ticker, "ticker", False, report_hold=True) is True
+        clock.t += 0.2
+        assert c._display_once(ticker, "ticker", False, report_hold=True) is True
+        assert ticker.display.call_count == 0
+        c.plugin_manager._warn_rate_limited.assert_not_called()
+        clock.t += 0.2
+        lock.release()                         # update() done
+        c._display_once(ticker, "ticker", False, report_hold=True)
+        assert ticker.display.call_count == 1
+        key, message, plugin_id, ms = c.plugin_manager._warn_rate_limited.call_args[0]
+        assert key == "display-hold:ticker" and plugin_id == "ticker"
+        assert "held" in message and ms == pytest.approx(400.0)
+        c.plugin_manager.health_tracker.record_busy_skip.assert_called_once_with(
+            "ticker", "display hold", pytest.approx(0.4))
+
+    def test_a_short_hold_is_not(self, held, clock):
+        c, lock = held
+        ticker = _plugin("ticker")
+        lock.acquire()
+        c._display_once(ticker, "ticker", False, report_hold=True)
+        clock.t += 0.1
+        lock.release()
+        c._display_once(ticker, "ticker", False, report_hold=True)
+        c.plugin_manager._warn_rate_limited.assert_not_called()
+        c.plugin_manager.health_tracker.record_busy_skip.assert_not_called()
+
+    def test_a_hold_that_ends_on_another_plugins_screen_is_not_blamed_on_it(
+            self, held, clock):
+        c, lock = held
+        lock.acquire()
+        c._display_once(_plugin("ticker"), "ticker", False, report_hold=True)
+        clock.t += 1.0
+        lock.release()
+        c._display_once(_plugin("clock"), "clock", False, report_hold=True)
+        c.plugin_manager._warn_rate_limited.assert_not_called()
+        assert c._display_hold is None
+
+    def test_frames_that_draw_report_nothing(self, held, clock):
+        c, _lock = held
+        ticker = _plugin("ticker")
+        for _ in range(5):
+            c._display_once(ticker, "ticker", False, report_hold=True)
+            clock.t += 0.5
+        c.plugin_manager._warn_rate_limited.assert_not_called()
+
+    def test_the_1hz_loop_reports_no_holds(self, held, clock):
+        # A static screen's frames are a second apart: one skipped frame is
+        # not a measured hold, and nothing on the panel froze. (On ledpi the
+        # first version reported every such skip as "held 1000 ms".)
+        c, lock = held
+        board = _plugin("board")
+        lock.acquire()
+        c._display_once(board, "board", False)
+        clock.t += 1.0
+        lock.release()
+        c._display_once(board, "board", False)
+        c.plugin_manager._warn_rate_limited.assert_not_called()
+        c.plugin_manager.health_tracker.record_busy_skip.assert_not_called()
+
+    def test_a_run_left_open_is_dropped_by_a_1hz_frame(self, held, clock):
+        c, lock = held
+        ticker = _plugin("ticker")
+        lock.acquire()
+        c._display_once(ticker, "ticker", False, report_hold=True)
+        lock.release()
+        c._display_once(ticker, "ticker", False)        # the 1 Hz loop draws
+        assert c._display_hold is None
+        clock.t += 5.0
+        c._display_once(ticker, "ticker", False, report_hold=True)
+        c.plugin_manager._warn_rate_limited.assert_not_called()

test/test_on_demand_live_and_restore.py

@@ -21,18 +21,6 @@ SPORTS_MODES = ['nfl_live', 'nfl_recent', 'nfl_upcoming',
                 'ncaa_fb_live', 'ncaa_fb_recent', 'ncaa_fb_upcoming']
 
 
-def _last_write(cache_manager, key):
-    """The last ``cache_manager.set(key, ...)`` call.
-
-    Not simply the last ``set`` call: the controller's font-usage publisher
-    thread writes ``font_usage_snapshot`` to the same cache manager whenever
-    it wakes, so on a slow runner it can land after the write under test.
-    """
-    writes = [c for c in cache_manager.set.call_args_list if c.args and c.args[0] == key]
-    assert writes, f"nothing was written to {key!r}"
-    return writes[-1]
-
-
 def _sports_plugin(has_live_content=False):
     plugin = MagicMock(spec=['display', 'has_live_content', 'has_live_priority',
                              'get_live_modes'])
@@ -99,7 +87,8 @@ class TestANamedLiveModeIsShown:
     def test_the_named_mode_survives_a_restart(self, football):
         football._activate_on_demand({'plugin_id': 'football-scoreboard',
                                       'mode': 'ncaa_fb_live'})
-        saved = _last_write(football.cache_manager, 'display_on_demand_config')
+        saved = football.cache_manager.set.call_args_list[-1]
+        assert saved.args[0] == 'display_on_demand_config'
         config = saved.args[1]
         assert config['named_mode'] == 'ncaa_fb_live'
 
@@ -131,7 +120,10 @@ class TestARestoreWithNothingToResume:
     def test_it_is_reported_as_an_error(self, restored):
         assert restored.on_demand_status == 'error'
         assert restored.on_demand_last_error == 'restore-failed'
-        published = _last_write(restored.cache_manager, 'display_on_demand_state')
+        # The last on-demand state write, not the last write of any key: the
+        # font-usage publisher thread writes its own key at its own pace.
+        published = [c for c in restored.cache_manager.set.call_args_list
+                     if c.args and c.args[0] == 'display_on_demand_state'][-1]
         assert published.args[1]['status'] == 'error'
         assert published.args[1]['error'] == 'restore-failed'
 

test/test_store_update_keeps_local_files.py

@@ -1,244 +0,0 @@
-"""A plugin update must keep the files the plugin wrote beside itself.
-
-Field incident, 2026-10-04: updating calendar 1.2.9 -> 1.2.12 from the web UI
-replaced plugin-repos/calendar/ with the fresh download and deleted the old
-copy -- and with it token.pickle and credentials.json, the plugin's Google
-OAuth files. No release contains them (the repo gitignores them), so the hot
-reload logged "Credentials file not found" and the calendar stayed broken
-until the files were restored by hand.
-
-Both update routes are covered: a monorepo plugin (registry ``plugin_path``),
-which is reinstalled into a fresh directory, and a plugin installed from its
-own git repository, which is updated with ``git pull`` after an auto-stash.
-"""
-
-import json
-import shutil
-import subprocess
-
-import pytest
-
-from src.plugin_system.plugin_local_files import (
-    is_known_state_file, local_files_to_keep,
-)
-from src.plugin_system.store_manager import PluginStoreManager
-
-PLUGIN_ID = "calendar"
-
-
-def _manifest(version):
-    return {"id": PLUGIN_ID, "name": "Calendar", "class_name": "CalendarPlugin",
-            "display_modes": ["calendar"], "version": version}
-
-
-def _write_release(target, version):
-    """What a download of ``version`` puts on disk."""
-    target.mkdir(parents=True, exist_ok=True)
-    (target / "manifest.json").write_text(json.dumps(_manifest(version)))
-    (target / "manager.py").write_text(f"VERSION = {version!r}\n")
-    (target / ".gitignore").write_text("credentials.json\ntoken.pickle\ncache/\n")
-
-
-def _drop_local_files(plugin_dir):
-    """What the plugin writes at runtime: OAuth files plus cached state."""
-    (plugin_dir / "token.pickle").write_bytes(b"\x80\x04oauth-token")
-    (plugin_dir / "credentials.json").write_text('{"installed": {}}')
-    (plugin_dir / "cache").mkdir()
-    (plugin_dir / "cache" / "events.json").write_text("[]")
-
-
-def _assert_local_files_kept(plugin_dir):
-    assert (plugin_dir / "token.pickle").read_bytes() == b"\x80\x04oauth-token"
-    assert (plugin_dir / "credentials.json").read_text() == '{"installed": {}}'
-    assert (plugin_dir / "cache" / "events.json").read_text() == "[]"
-
-
-def _leftover_backups(plugins_dir):
-    return [p.name for p in plugins_dir.iterdir() if "standalone-backup" in p.name]
-
-
-@pytest.fixture
-def store(tmp_path, monkeypatch):
-    mgr = PluginStoreManager(
-        plugins_dir=str(tmp_path / "plugin-repos"),
-        uninstalled_registry_path=str(tmp_path / "uninstalled.json"))
-    mgr.plugins_dir.mkdir(parents=True, exist_ok=True)
-    monkeypatch.setattr(mgr, "_install_dependencies", lambda *a, **k: True)
-    monkeypatch.setattr(mgr, "fetch_registry", lambda *a, **k: {"plugins": []})
-    return mgr
-
-
-class TestMonorepoUpdate:
-    @pytest.fixture
-    def installed(self, store, monkeypatch):
-        registry_entry = {
-            "id": PLUGIN_ID, "repo": "https://github.com/ChuckBuilds/ledmatrix-plugins",
-            "plugin_path": "plugins/calendar", "branch": "main",
-            "latest_version": "1.2.9",
-        }
-        monkeypatch.setattr(store, "get_plugin_info", lambda *a, **k: registry_entry)
-        release = {"version": "1.2.9"}
-
-        def fake_monorepo_download(download_url, plugin_subpath, target):
-            assert plugin_subpath == "plugins/calendar"
-            _write_release(target, release["version"])
-            return True
-
-        monkeypatch.setattr(store, "_install_from_monorepo", fake_monorepo_download)
-        assert store.install_plugin(PLUGIN_ID) is True
-
-        def publish(version):
-            registry_entry["latest_version"] = release["version"] = version
-        return store, store.plugins_dir / PLUGIN_ID, publish
-
-    def test_update_keeps_token_and_gitignored_files(self, installed):
-        store, plugin_dir, publish = installed
-        _drop_local_files(plugin_dir)
-
-        publish("1.2.12")
-        assert store.update_plugin(PLUGIN_ID) is True
-
-        assert json.loads((plugin_dir / "manifest.json").read_text())["version"] == "1.2.12"
-        _assert_local_files_kept(plugin_dir)
-        assert _leftover_backups(store.plugins_dir) == []
-
-    def test_token_is_kept_even_when_the_release_does_not_gitignore_it(self, installed):
-        store, plugin_dir, publish = installed
-        (plugin_dir / ".gitignore").unlink()
-        (plugin_dir / "token.pickle").write_bytes(b"tok")
-        (plugin_dir / "config_secrets.json").write_text("{}")
-
-        publish("1.2.12")
-        assert store.update_plugin(PLUGIN_ID) is True
-
-        assert (plugin_dir / "token.pickle").read_bytes() == b"tok"
-        assert (plugin_dir / "config_secrets.json").read_text() == "{}"
-
-    def test_release_content_wins_and_old_code_is_not_carried(self, installed):
-        store, plugin_dir, publish = installed
-        # A file the old copy had that the new release dropped, byte code, and
-        # an old copy of a file the new release also ships.
-        (plugin_dir / "removed_module.py").write_text("OLD = True\n")
-        (plugin_dir / "__pycache__").mkdir()
-        (plugin_dir / "__pycache__" / "manager.cpython-313.pyc").write_bytes(b"pyc")
-
-        publish("1.2.12")
-        assert store.update_plugin(PLUGIN_ID) is True
-
-        assert not (plugin_dir / "removed_module.py").exists()
-        assert not (plugin_dir / "__pycache__").exists()
-        assert "1.2.12" in (plugin_dir / "manager.py").read_text()
-
-    def test_reinstall_over_an_existing_copy_keeps_them_too(self, installed):
-        store, plugin_dir, publish = installed
-        _drop_local_files(plugin_dir)
-
-        assert store.install_plugin(PLUGIN_ID) is True
-
-        _assert_local_files_kept(plugin_dir)
-        assert _leftover_backups(store.plugins_dir) == []
-
-
-class TestInstallFromUrlReplace:
-    def test_replacing_an_installed_copy_keeps_the_token(self, store, monkeypatch):
-        plugin_dir = store.plugins_dir / PLUGIN_ID
-        _write_release(plugin_dir, "1.0.0")
-        _drop_local_files(plugin_dir)
-
-        def fake_clone(repo_url, target, branches):
-            _write_release(target, "2.0.0")
-            return "main"
-
-        monkeypatch.setattr(store, "_install_via_git", fake_clone)
-        result = store.install_from_url(
-            "https://github.com/example/ledmatrix-calendar", plugin_id=PLUGIN_ID)
-
-        assert result["success"] is True
-        assert json.loads((plugin_dir / "manifest.json").read_text())["version"] == "2.0.0"
-        _assert_local_files_kept(plugin_dir)
-
-
-def _git(*args, cwd):
-    subprocess.run(["git", "-c", "user.email=t@example.com", "-c", "user.name=t",
-                    "-c", "core.autocrlf=false", *args],
-                   cwd=cwd, check=True, capture_output=True)
-
-
-@pytest.mark.skipif(shutil.which("git") is None, reason="git not installed")
-class TestGitRepoUpdate:
-    @pytest.fixture
-    def cloned(self, store, tmp_path, monkeypatch):
-        monkeypatch.setattr(store, "get_plugin_info", lambda *a, **k: None)
-        upstream = tmp_path / "upstream"
-        _write_release(upstream, "1.0.0")
-        # This repo does NOT gitignore the token: an untracked, non-ignored
-        # file is exactly what `git stash push -u` used to sweep away.
-        (upstream / ".gitignore").write_text("cache/\n")
-        _git("init", "-q", "-b", "main", cwd=upstream)
-        _git("add", ".", cwd=upstream)
-        _git("commit", "-qm", "1.0.0", cwd=upstream)
-
-        plugin_dir = store.plugins_dir / PLUGIN_ID
-        _git("clone", "-q", str(upstream), str(plugin_dir), cwd=tmp_path)
-
-        def publish(version):
-            (upstream / "manifest.json").write_text(json.dumps(_manifest(version)))
-            _git("commit", "-qam", version, cwd=upstream)
-        return store, plugin_dir, publish
-
-    def test_pull_update_keeps_untracked_token(self, cloned):
-        store, plugin_dir, publish = cloned
-        _drop_local_files(plugin_dir)
-        # An unrelated untracked file, so the update really does stash.
-        (plugin_dir / "notes.txt").write_text("scratch")
-
-        publish("1.1.0")
-        assert store.update_plugin(PLUGIN_ID) is True
-
-        assert json.loads((plugin_dir / "manifest.json").read_text())["version"] == "1.1.0"
-        _assert_local_files_kept(plugin_dir)
-
-    def test_token_alone_does_not_trigger_a_stash(self, cloned):
-        store, plugin_dir, publish = cloned
-        (plugin_dir / "token.pickle").write_bytes(b"tok")
-
-        publish("1.1.0")
-        assert store.update_plugin(PLUGIN_ID) is True
-
-        assert (plugin_dir / "token.pickle").read_bytes() == b"tok"
-        stashes = subprocess.run(["git", "-C", str(plugin_dir), "stash", "list"],
-                                 capture_output=True, text=True, check=True)
-        assert stashes.stdout.strip() == ""
-
-
-class TestWhatIsKept:
-    @pytest.mark.parametrize("path,expected", [
-        ("token.pickle", True),
-        ("data/session.pickle", True),
-        ("credentials.json", True),
-        ("token.json", True),
-        ("config_secrets.json", True),
-        (".pkce_code_verifier", True),
-        ("manager.py", False),
-        ("config.json", False),
-    ])
-    def test_known_state_files(self, path, expected):
-        assert is_known_state_file(path) is expected
-
-    def test_gitignore_rules(self, tmp_path):
-        old, new = tmp_path / "old", tmp_path / "new"
-        new.mkdir()
-        for rel in ["a.log", "logs/x.txt", "sub/deep/b.log", "keep.log",
-                    "anchored.txt", "sub/anchored.txt", "assets/x/y_backup/z.png",
-                    "manager.py", "shipped.log"]:
-            (old / rel).parent.mkdir(parents=True, exist_ok=True)
-            (old / rel).write_text("x")
-        (new / "shipped.log").write_text("new")
-        (old / ".gitignore").write_text(
-            "# comment\n*.log\n!keep.log\nlogs/\n/anchored.txt\n"
-            "assets/**/*_backup/\n")
-
-        assert local_files_to_keep(old, new) == [
-            "a.log", "anchored.txt", "assets/x/y_backup/z.png",
-            "logs/x.txt", "sub/deep/b.log",
-        ]

test/web_interface/test_starlark_pixlet_routes.py

@@ -26,8 +26,15 @@ import pytest
 
 
 @pytest.fixture
-def client():
+def client(monkeypatch):
+    from web_interface import app as web_app
     from web_interface.app import app
+    # The captive-portal before_request hook shells out to systemctl/nmcli
+    # whenever its 30s cache is cold, so on a Linux host whether a request
+    # here runs subprocess depended on how long ago the previous one was --
+    # and several tests below stub subprocess. Pin it: no test in this file
+    # is about AP mode.
+    monkeypatch.setattr(web_app, 'is_ap_mode_active', lambda: False)
     app.config['TESTING'] = True
     with app.test_client() as c:
         yield c
@@ -1131,25 +1138,28 @@ class TestPixletEditorHostDefaultsButDoesNotOverride:
         class FakeProcess:
             pid = 424242
 
-        real_popen = mod.subprocess.Popen
-
         def fake_popen(cmd, *args, env=None, **kwargs):
-            # Only the editor launch is faked. Patching subprocess.Popen
-            # patches it for the whole request, and the captive-portal
-            # before_request hook runs `systemctl is-active hostapd` through
-            # subprocess.run whenever its 30s cache has expired -- which
-            # needs a real process (run() uses it as a context manager).
-            if str(script) not in cmd:
-                return real_popen(cmd, *args, env=env, **kwargs)
-            captured['env'] = env
+            if env is not None:
+                captured['env'] = env
             return FakeProcess()
 
+        # Swap the route module's own ``subprocess`` binding, not the shared
+        # ``subprocess.Popen``: patching the attribute on the real module is
+        # process-wide, and the app's before_request hook (the captive-portal
+        # check) runs ``subprocess.run`` -- ``with Popen(...)`` -- whenever its
+        # 30s AP-mode cache is cold on a host with systemctl. On the Linux CI
+        # runner that handed it this FakeProcess and 500'd the request, but
+        # only when the previous request was more than 30s earlier.
+        fake_subprocess = types.ModuleType('subprocess')
+        fake_subprocess.__dict__.update(mod.subprocess.__dict__)
+        fake_subprocess.Popen = fake_popen
+
         with patch.object(mod, '_validate_starlark_app_path',
                           return_value=(app_dir, None)), \
              patch.object(mod, '_PIXLET_EDITOR_SCRIPT', script), \
              patch.object(mod, '_PIXLET_EDITOR_STATE', state_file), \
              patch.object(mod, '_find_pixlet_binary', return_value='/usr/bin/pixlet'), \
-             patch.object(mod.subprocess, 'Popen', side_effect=fake_popen), \
+             patch.object(mod, 'subprocess', fake_subprocess), \
              patch.dict(os.environ):
             if operator_host is None:
                 os.environ.pop('PIXLET_EDITOR_HOST', None)