Merge branch 'perf/throttle-metrics-persistence' of https://github.com/ChuckBuilds/LEDMatrix into consolidate/perf

Two review findings on the throttle, both right.

The interval compared wall-clock timestamps. These devices have no RTC, so
the clock jumps by however far off boot-time was the moment NTP first syncs
-- a forward jump would allow an early write, a backward one would stall the
snapshot well past the interval. time.monotonic() is not subject to either.

The timestamp was also recorded before cache_manager.set(). A set() that
raised would buy the next interval's silence without leaving a snapshot
behind, which is the one case where skipping the write is least affordable.
Recorded after the write lands instead, so a failure is retried on the next
call.

Verified by restoring the original ordering: the new test then reports one
write where two are expected.

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

src/common/scroll_helper.py

@@ -328,7 +328,7 @@ 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
-            self.logger.info(
+            self.logger.debug(
                 "Scroll progress: elapsed=%.2fs, target=%.2fs, total_scrolled=%.0f/%d px (%.1f%%)",
                 elapsed_time,
                 self.calculated_duration,

src/logging_config.py

@@ -130,7 +130,12 @@ def setup_logging(
     # Console handler (always add)
     console_handler = logging.StreamHandler(sys.stdout)
     console_handler.setLevel(level)
-    console_handler.setFormatter(formatter)
+    # Under systemd, tag each line so the journal records the real severity
+    # rather than filing everything as informational. The file handler below
+    # keeps the plain formatter: the prefix is meaningful to journald and noise
+    # anywhere else.
+    console_handler.setFormatter(
+        JournalPriorityFormatter(formatter) if _under_systemd() else formatter)
     root_logger.addHandler(console_handler)
     
     # File handler (if specified)
@@ -145,6 +150,66 @@ def setup_logging(
             sys.stderr.write(f"Warning: Could not set up file logging to {log_file}: {e}\n")
 
 
+#: syslog priorities, which is what systemd parses from a "<N>" prefix on
+#: stdout. Mapped from Python's levels.
+_SYSLOG_PRIORITY = {
+    logging.CRITICAL: 2,   # LOG_CRIT
+    logging.ERROR: 3,      # LOG_ERR
+    logging.WARNING: 4,    # LOG_WARNING
+    logging.INFO: 6,       # LOG_INFO
+    logging.DEBUG: 7,      # LOG_DEBUG
+}
+
+
+class JournalPriorityFormatter(logging.Formatter):
+    """Wraps a formatter, prefixing each line with its syslog priority.
+
+    Under systemd everything this process writes to stdout lands in the journal
+    as PRIORITY=6, whatever the Python level was. Measured on a live rig: 55
+    ERROR lines and 13 WARNING lines in a day, every one of them recorded as
+    informational, so `journalctl -p err -u ledmatrix` returned nothing at all
+    while errors were being logged. Anyone triaging has to grep the message
+    text instead, which is both slower and wrong -- a search for "oom" matches
+    the radar logging "zoom=9".
+
+    systemd reads a leading "<N>" on each line and uses it as the priority
+    (sd-daemon(3)), so this needs no extra dependency. Multi-line records get
+    the prefix on every line, since the journal splits them and an unprefixed
+    continuation would fall back to the default.
+    """
+
+    def __init__(self, inner: logging.Formatter):
+        super().__init__()
+        self._inner = inner
+
+    @property
+    def inner(self) -> logging.Formatter:
+        """The formatter doing the actual work.
+
+        Whether journald tagging is applied depends on JOURNAL_STREAM, so it is
+        on under systemd and off in a terminal -- and anything asserting which
+        formatter setup_logging() selected would otherwise get a different
+        answer in CI than on a developer's machine. Exposing the inner one lets
+        those checks stay about format_type, which is what they mean.
+        """
+        return self._inner
+
+    def format(self, record: logging.LogRecord) -> str:
+        text = self._inner.format(record)
+        prefix = f"<{_SYSLOG_PRIORITY.get(record.levelno, 6)}>"
+        return "\n".join(prefix + line for line in text.split("\n"))
+
+
+def _under_systemd() -> bool:
+    """True when stdout is the journal.
+
+    systemd sets JOURNAL_STREAM for services whose output it captures. Without
+    this check the "<N>" prefixes would show up as literal noise when the
+    program is run from a terminal, in the emulator, or in tests.
+    """
+    return bool(os.environ.get("JOURNAL_STREAM"))
+
+
 class PluginLoggerAdapter(logging.LoggerAdapter):
     """LoggerAdapter that stamps every record with its plugin_id.
 

src/plugin_system/plugin_health.py

@@ -178,10 +178,20 @@ class PluginHealthTracker:
             )
         return self._health_state[plugin_id]
     
+    # Fields the circuit breaker is rebuilt from after a restart. Everything
+    # else in a health record is reporting, read only for display.
+    _DURABLE_FIELDS = ('consecutive_failures', 'circuit_state',
+                       'circuit_opened_time', 'half_open_start_time')
+
+    def _durable(self, state: Dict[str, Any]) -> tuple:
+        """The part of a health record whose loss would change behaviour."""
+        return tuple(state.get(field) for field in self._DURABLE_FIELDS)
+
     def record_success(self, plugin_id: str) -> None:
         """Record a successful plugin execution."""
         state = self.get_health_state(plugin_id)
         current_time = time.time()
+        durable_before = self._durable(state)
 
         # Reset consecutive failures
         state['consecutive_failures'] = 0
@@ -199,7 +209,18 @@ class PluginHealthTracker:
             state['circuit_state'] = CircuitState.CLOSED.value
             state['circuit_opened_time'] = None
 
-        self._save_health_state(plugin_id, state)
+        # A healthy plugin reports success every cycle, and in that steady state
+        # the only fields changed above are a counter and a timestamp that
+        # nothing reads back after a restart. Persisting them anyway rewrites a
+        # small file per plugin per cycle: on a rig running 24 plugins, a
+        # five-minute sample measured 22 rewrites, about 4.4 a minute or 6,300 a
+        # day. Those land on an SD card, where the cost is an erase-block cycle
+        # rather than the 400 bytes involved, and where wear is what eventually
+        # kills the card.
+        # In-memory state is still updated every time, so the health API and web
+        # UI show exactly what they did before; only the write is skipped.
+        if self._durable(state) != durable_before:
+            self._save_health_state(plugin_id, state)
 
     def record_failure(self, plugin_id: str, error: Optional[Exception] = None) -> None:
         """Record a failed plugin execution."""

src/plugin_system/resource_monitor.py

@@ -9,7 +9,7 @@ import time
 import logging
 import threading
 from typing import Dict, Optional, Any, Callable
-from dataclasses import dataclass, field
+from dataclasses import dataclass, field, fields
 
 try:
     import psutil
@@ -49,6 +49,20 @@ class ResourceMetrics:
             self.total_execution_time = self.total_execution_time / self.call_count
 
 
+#: How often a plugin's metrics are written to the cache, in seconds.
+#:
+#: Persisting on every call meant a small file rewritten roughly nine times a
+#: minute per plugin. On a rig with fourteen active plugins that was ~126
+#: writes a minute for metrics alone, and since each ~350-byte file costs a
+#: 4KB block plus an ext4 journal entry, it dominated the device's write
+#: volume -- on an SD card, which wears out.
+#:
+#: The in-memory copy stays authoritative and exact; only the cross-process
+#: snapshot the web UI reads is delayed, and telemetry up to half a minute old
+#: is still a fair description of a long-running plugin.
+_METRICS_PERSIST_INTERVAL = 30.0
+
+
 class PluginResourceMonitor:
     """
     Monitors resource usage for plugins.
@@ -75,6 +89,10 @@ class PluginResourceMonitor:
         # Resource metrics per plugin
         self._metrics: Dict[str, ResourceMetrics] = {}
         self._limits: Dict[str, ResourceLimits] = {}
+        # When each plugin's metrics last reached the cache. Metrics change on
+        # every call, so they cannot be de-duplicated the way health state can;
+        # they are rate-limited instead. See _METRICS_PERSIST_INTERVAL.
+        self._metrics_persisted_at: Dict[str, float] = {}
 
         # Thread-local storage for execution tracking
         self._local = threading.local()
@@ -102,6 +120,50 @@ class PluginResourceMonitor:
                 "psutil not available - resource monitoring will be limited to execution time only"
             )
     
+    def _metrics_from_cache(self, plugin_id: str, cached: Any) -> "ResourceMetrics":
+        """Build metrics from a cached record, ignoring anything unrecognised.
+
+        ResourceMetrics(**cached) raises TypeError on a single unexpected key,
+        and that exception escapes into plugin_manager, which reports it as
+        "plugin <id> operation failed". Every plugin fails, and the plugin
+        system never finishes initialising.
+
+        Seen on a live rig: every plugin failing with
+
+            ResourceMetrics.__init__() got an unexpected keyword argument
+            'consecutive_failures'
+
+        which is a plugin_health field, not a metrics one. How a health-shaped
+        record came to sit under a plugin_metrics key on that machine is not
+        established -- a restored backup that mixed two machines' caches is the
+        likeliest explanation -- but the loader should not be brittle enough for
+        it to matter. plugin_health already repairs its records field by field
+        rather than trusting whatever is on disk; this does the same.
+
+        Unknown keys are dropped and named once, so a genuine schema change is
+        visible in the log instead of silently discarded.
+        """
+        if not isinstance(cached, dict):
+            self.logger.warning(
+                "Ignoring cached metrics for %s: expected a mapping, got %s",
+                plugin_id, type(cached).__name__)
+            return ResourceMetrics()
+
+        known = {f.name for f in fields(ResourceMetrics)}
+        unknown = sorted(set(cached) - known)
+        if unknown:
+            self.logger.warning(
+                "Dropping unrecognised field(s) from cached metrics for %s: %s",
+                plugin_id, ", ".join(unknown))
+        usable = {k: v for k, v in cached.items() if k in known}
+        try:
+            return ResourceMetrics(**usable)
+        except (TypeError, ValueError) as e:
+            self.logger.warning(
+                "Cached metrics for %s unusable (%s); starting fresh",
+                plugin_id, e)
+            return ResourceMetrics()
+
     def _get_metrics_key(self, plugin_id: str) -> str:
         """Get cache key for plugin metrics."""
         return f"plugin_metrics:{plugin_id}"
@@ -126,7 +188,7 @@ class PluginResourceMonitor:
                     cache_key, max_age=None, memory_ttl=0 if force_reload else None
                 )
                 if cached:
-                    metrics = ResourceMetrics(**cached)
+                    metrics = self._metrics_from_cache(plugin_id, cached)
                 else:
                     metrics = ResourceMetrics()
                 self._metrics[plugin_id] = metrics
@@ -232,18 +294,8 @@ class PluginResourceMonitor:
                     # CPU is harder to measure per-call, so we track it separately
                     metrics.cpu_percent = self._get_process_cpu_percent()
                 
-                # Persist metrics
+                # Persist metrics, at most once per interval per plugin.
-                cache_key = self._get_metrics_key(plugin_id)
+                self._persist_metrics(plugin_id, metrics)
-                self.cache_manager.set(cache_key, {
-                    'memory_mb': metrics.memory_mb,
-                    'cpu_percent': metrics.cpu_percent,
-                    'execution_time': metrics.execution_time,
-                    'call_count': metrics.call_count,
-                    'total_execution_time': metrics.total_execution_time,
-                    'max_execution_time': metrics.max_execution_time,
-                    'min_execution_time': metrics.min_execution_time if metrics.min_execution_time != float('inf') else 0.0,
-                    'last_update_time': metrics.last_update_time
-                })
             
             # Check limits
             if limits:
@@ -363,6 +415,37 @@ class PluginResourceMonitor:
             summaries[plugin_id] = self.get_metrics_summary(plugin_id)
         return summaries
     
+    def _persist_metrics(self, plugin_id: str, metrics: ResourceMetrics,
+                         force: bool = False) -> None:
+        """Write a plugin's metrics to the cache, at most once per interval.
+
+        Caller must hold ``self._lock``.
+        """
+        # Monotonic, not wall clock: these devices have no RTC, so the clock
+        # jumps by however far off boot-time was the moment NTP first syncs.
+        # A forward jump would allow an early write, a backward one would
+        # stall the snapshot well past the interval.
+        now = time.monotonic()
+        if not force and now - self._metrics_persisted_at.get(plugin_id, 0.0) \
+                < _METRICS_PERSIST_INTERVAL:
+            return
+        cache_key = self._get_metrics_key(plugin_id)
+        self.cache_manager.set(cache_key, {
+            'memory_mb': metrics.memory_mb,
+            'cpu_percent': metrics.cpu_percent,
+            'execution_time': metrics.execution_time,
+            'call_count': metrics.call_count,
+            'total_execution_time': metrics.total_execution_time,
+            'max_execution_time': metrics.max_execution_time,
+            'min_execution_time': (metrics.min_execution_time
+                                   if metrics.min_execution_time != float('inf')
+                                   else 0.0),
+            'last_update_time': metrics.last_update_time,
+        })
+        # Only after the write lands. Marking it first would mean a failed
+        # set() bought the next interval's silence without leaving a snapshot.
+        self._metrics_persisted_at[plugin_id] = now
+
     def reset_metrics(self, plugin_id: str) -> None:
         """Reset metrics for a plugin."""
         with self._lock:
@@ -370,4 +453,7 @@ class PluginResourceMonitor:
                 self._metrics[plugin_id] = ResourceMetrics()
                 cache_key = self._get_metrics_key(plugin_id)
                 self.cache_manager.delete(cache_key)
+                # Let the next call persist immediately rather than leaving the
+                # deleted key absent for the rest of the interval.
+                self._metrics_persisted_at.pop(plugin_id, None)
 

src/vegas_mode/plugin_adapter.py

@@ -83,7 +83,7 @@ class PluginAdapter:
         # into unrelated headlines once the strip refreshed to 9,505px.
         self._offset_shapes: dict = {}
 
-        logger.info(
+        logger.debug(
             "PluginAdapter initialized: display=%dx%d",
             self.display_width, self.display_height
         )
@@ -109,7 +109,7 @@ class PluginAdapter:
         Returns:
             List of PIL Images representing plugin content, or None if no content
         """
-        logger.info(
+        logger.debug(
             "[%s] Getting content (class=%s)",
             plugin_id, plugin.__class__.__name__
         )
@@ -118,7 +118,7 @@ class PluginAdapter:
         cached = self._get_cached(plugin_id)
         if cached is not None:
             total_width = sum(img.width for img in cached)
-            logger.info(
+            logger.debug(
                 "[%s] Using cached content: %d images, %dpx total",
                 plugin_id, len(cached), total_width
             )
@@ -126,46 +126,46 @@ class PluginAdapter:
 
         # Try native Vegas content method first
         has_native = hasattr(plugin, 'get_vegas_content')
-        logger.info("[%s] Has get_vegas_content: %s", plugin_id, has_native)
+        logger.debug("[%s] Has get_vegas_content: %s", plugin_id, has_native)
         if has_native:
             content = self._get_native_content(plugin, plugin_id, offscreen_only)
             if content:
                 total_width = sum(img.width for img in content)
-                logger.info(
+                logger.debug(
                     "[%s] Native content SUCCESS: %d images, %dpx total",
                     plugin_id, len(content), total_width
                 )
                 return self._finalize(content, plugin_id, 'native', plugin)
-            logger.info("[%s] Native content returned None", plugin_id)
+            logger.debug("[%s] Native content returned None", plugin_id)
 
         # Try to get scroll_helper's cached image (for scrolling plugins like stocks/odds)
         has_scroll_helper = hasattr(plugin, 'scroll_helper')
-        logger.info("[%s] Has scroll_helper: %s", plugin_id, has_scroll_helper)
+        logger.debug("[%s] Has scroll_helper: %s", plugin_id, has_scroll_helper)
         content = self._get_scroll_helper_content(plugin, plugin_id, offscreen_only)
         if content:
             total_width = sum(img.width for img in content)
-            logger.info(
+            logger.debug(
                 "[%s] ScrollHelper content SUCCESS: %d images, %dpx total",
                 plugin_id, len(content), total_width
             )
             return self._finalize(content, plugin_id, 'scroll_helper', plugin)
         if has_scroll_helper:
-            logger.info("[%s] ScrollHelper content returned None", plugin_id)
+            logger.debug("[%s] ScrollHelper content returned None", plugin_id)
 
         if offscreen_only:
             # Display capture needs the shared canvas; leave it to the caller.
-            logger.info(
+            logger.debug(
                 "[%s] Needs display capture, deferring to the render thread",
                 plugin_id
             )
             return None
 
         # Fall back to display capture
-        logger.info("[%s] Trying fallback display capture...", plugin_id)
+        logger.debug("[%s] Trying fallback display capture...", plugin_id)
         content = self._capture_display_content(plugin, plugin_id)
         if content:
             total_width = sum(img.width for img in content)
-            logger.info(
+            logger.debug(
                 "[%s] Fallback capture SUCCESS: %d images, %dpx total",
                 plugin_id, len(content), total_width
             )
@@ -226,7 +226,7 @@ class PluginAdapter:
             kept.append(result.image)
 
         if not kept:
-            logger.info(
+            logger.debug(
                 "[%s] All %d image(s) from %s were blank — contributing nothing",
                 plugin_id, len(images), source
             )
@@ -235,14 +235,14 @@ class PluginAdapter:
         trimmed_width = sum(img.width for img in kept)
 
         if trimmed_width < self.config.min_plugin_width:
-            logger.info(
+            logger.debug(
                 "[%s] Trimmed content %dpx is below min_plugin_width %dpx — skipping",
                 plugin_id, trimmed_width, self.config.min_plugin_width
             )
             return None
 
         if trimmed_width != original_width or dropped_blank:
-            logger.info(
+            logger.debug(
                 "[%s] Trimmed %s content: %dpx -> %dpx (%.0f%% reclaimed), "
                 "%d image(s) kept, %d blank dropped",
                 plugin_id, source, original_width, trimmed_width,
@@ -431,7 +431,7 @@ class PluginAdapter:
         """
         if self._offset_shapes.get(plugin_id) != shape:
             if plugin_id in self._item_offsets:
-                logger.info(
+                logger.debug(
                     "[%s] Content is %s now, was %s — restarting the rotation "
                     "rather than resuming at a position that no longer means "
                     "anything", plugin_id, shape,
@@ -579,7 +579,7 @@ class PluginAdapter:
             consumed += 1
 
         if mode == 'truncate':
-            logger.info(
+            logger.debug(
                 "[%s] Width budget %dpx: showing the first %d of %d row(s) "
                 "(%dpx incl. gaps); the rest are not shown (overflow=truncate)",
                 plugin_id, budget, len(selected), len(images), used
@@ -587,7 +587,7 @@ class PluginAdapter:
         else:
             self._record_offset(
                 plugin_id, (start + consumed) % len(images), shape)
-            logger.info(
+            logger.debug(
                 "[%s] Width budget %dpx: showing %d of %d row(s) (%dpx incl. gaps) "
                 "from offset %d; remainder deferred to a later cycle",
                 plugin_id, budget, len(selected), len(images), used, start
@@ -636,7 +636,7 @@ class PluginAdapter:
             if mode != 'truncate':
                 self._record_offset(
                     plugin_id, 0 if end >= img.width else end, shape)
-            logger.info(
+            logger.debug(
                 "[%s] Width budget %dpx: cropped continuous %dpx image to "
                 "[%d:%d] (no item gaps of %dpx+ to align to)%s",
                 plugin_id, budget, img.width, offset, end, min_run,
@@ -674,7 +674,7 @@ class PluginAdapter:
             self._record_offset(
                 plugin_id, 0 if end >= img.width else end_index, shape)
 
-        logger.info(
+        logger.debug(
             "[%s] Width budget %dpx: cropped single %dpx image to [%d:%d] "
             "(%dpx) at item boundaries %d-%d of %d, %s",
             plugin_id, budget, img.width, start, end, end - start,
@@ -698,7 +698,7 @@ class PluginAdapter:
             List of images or None
         """
         try:
-            logger.info("[%s] Native: calling get_vegas_content()", plugin_id)
+            logger.debug("[%s] Native: calling get_vegas_content()", plugin_id)
 
             # Tell the plugin how much width the ticker wants it to use, and
             # narrow the canvas for the duration of the call. A plugin that
@@ -707,7 +707,7 @@ class PluginAdapter:
             # be explicit can read get_vegas_render_width().
             render_width = self.resolve_render_width(plugin, plugin_id)
             if render_width != self.display_width:
-                logger.info(
+                logger.debug(
                     "[%s] Native: requesting %dpx instead of %dpx",
                     plugin_id, render_width, self.display_width
                 )
@@ -735,19 +735,19 @@ class PluginAdapter:
                 plugin._vegas_render_width = None
 
             if result is None:
-                logger.info("[%s] Native: get_vegas_content() returned None", plugin_id)
+                logger.debug("[%s] Native: get_vegas_content() returned None", plugin_id)
                 return None
 
             # Normalize to list
             if isinstance(result, Image.Image):
                 images = [result]
-                logger.info(
+                logger.debug(
                     "[%s] Native: got single Image %dx%d",
                     plugin_id, result.width, result.height
                 )
             elif isinstance(result, (list, tuple)):
                 images = list(result)
-                logger.info(
+                logger.debug(
                     "[%s] Native: got %d items in list/tuple",
                     plugin_id, len(images)
                 )
@@ -768,14 +768,14 @@ class PluginAdapter:
                     )
                     continue
 
-                logger.info(
+                logger.debug(
                     "[%s] Native: item[%d] is %dx%d, mode=%s",
                     plugin_id, i, img.width, img.height, img.mode
                 )
 
                 # Ensure correct height
                 if img.height != self.display_height:
-                    logger.info(
+                    logger.debug(
                         "[%s] Native: resizing item[%d]: %dx%d -> %dx%d",
                         plugin_id, i, img.width, img.height,
                         img.width, self.display_height
@@ -793,13 +793,13 @@ class PluginAdapter:
 
             if valid_images:
                 total_width = sum(img.width for img in valid_images)
-                logger.info(
+                logger.debug(
                     "[%s] Native: SUCCESS - %d images, %dpx total width",
                     plugin_id, len(valid_images), total_width
                 )
                 return valid_images
 
-            logger.info("[%s] Native: no valid images after validation", plugin_id)
+            logger.debug("[%s] Native: no valid images after validation", plugin_id)
             return None
 
         except (AttributeError, TypeError, ValueError, OSError) as e:
@@ -833,20 +833,20 @@ class PluginAdapter:
                 logger.debug("[%s] No scroll_helper attribute", plugin_id)
                 return None
 
-            logger.info(
+            logger.debug(
                 "[%s] Found scroll_helper: %s",
                 plugin_id, type(scroll_helper).__name__
             )
 
             cached_image = getattr(scroll_helper, 'cached_image', None)
             if cached_image is None:
-                logger.info(
+                logger.debug(
                     "[%s] scroll_helper.cached_image is None, triggering content generation",
                     plugin_id
                 )
                 if offscreen_only:
                     # Generating it calls display(), which needs the canvas.
-                    logger.info(
+                    logger.debug(
                         "[%s] scroll_helper cache empty; deferring generation "
                         "to the render thread", plugin_id
                     )
@@ -859,13 +859,13 @@ class PluginAdapter:
                     return None
 
             if not isinstance(cached_image, Image.Image):
-                logger.info(
+                logger.debug(
                     "[%s] scroll_helper.cached_image is not an Image: %s",
                     plugin_id, type(cached_image).__name__
                 )
                 return None
 
-            logger.info(
+            logger.debug(
                 "[%s] scroll_helper.cached_image found: %dx%d, mode=%s",
                 plugin_id, cached_image.width, cached_image.height, cached_image.mode
             )
@@ -888,7 +888,7 @@ class PluginAdapter:
 
             # Ensure correct height
             if img.height != self.display_height:
-                logger.info(
+                logger.debug(
                     "[%s] Resizing scroll_helper content: %dx%d -> %dx%d",
                     plugin_id, img.width, img.height,
                     img.width, self.display_height
@@ -902,7 +902,7 @@ class PluginAdapter:
             if img.mode != 'RGB':
                 img = img.convert('RGB')
 
-            logger.info(
+            logger.debug(
                 "[%s] ScrollHelper content ready: %dx%d",
                 plugin_id, img.width, img.height
             )
@@ -1002,7 +1002,7 @@ class PluginAdapter:
             with self._capture():
                 # Method 1: Try _create_scrolling_display (stocks pattern)
                 if hasattr(plugin, '_create_scrolling_display'):
-                    logger.info(
+                    logger.debug(
                         "[%s] Triggering via _create_scrolling_display()",
                         plugin_id
                     )
@@ -1010,7 +1010,7 @@ class PluginAdapter:
                         plugin._create_scrolling_display()
                         cached_image = getattr(scroll_helper, 'cached_image', None)
                         if cached_image is not None and isinstance(cached_image, Image.Image):
-                            logger.info(
+                            logger.debug(
                                 "[%s] _create_scrolling_display() SUCCESS: %dx%d",
                                 plugin_id, cached_image.width, cached_image.height
                             )
@@ -1022,7 +1022,7 @@ class PluginAdapter:
 
                 # Method 2: Try display(force_clear=True) which typically builds scroll content
                 if hasattr(plugin, 'display'):
-                    logger.info(
+                    logger.debug(
                         "[%s] Triggering via display(force_clear=True)",
                         plugin_id
                     )
@@ -1031,12 +1031,12 @@ class PluginAdapter:
                         plugin.display(force_clear=True)
                         cached_image = getattr(scroll_helper, 'cached_image', None)
                         if cached_image is not None and isinstance(cached_image, Image.Image):
-                            logger.info(
+                            logger.debug(
                                 "[%s] display(force_clear=True) SUCCESS: %dx%d",
                                 plugin_id, cached_image.width, cached_image.height
                             )
                             return cached_image
-                        logger.info(
+                        logger.debug(
                             "[%s] display(force_clear=True) did not populate cached_image",
                             plugin_id
                         )
@@ -1045,7 +1045,7 @@ class PluginAdapter:
                             "[%s] display(force_clear=True) failed", plugin_id
                         )
 
-            logger.info(
+            logger.debug(
                 "[%s] Could not trigger scroll content generation",
                 plugin_id
             )
@@ -1077,15 +1077,15 @@ class PluginAdapter:
         try:
             # Save current display state
             original_image = self.display_manager.image.copy()
-            logger.info("[%s] Fallback: saved original display state", plugin_id)
+            logger.debug("[%s] Fallback: saved original display state", plugin_id)
 
             # Ensure plugin has fresh data before capturing
             has_update_data = hasattr(plugin, 'update_data')
-            logger.info("[%s] Fallback: has update_data=%s", plugin_id, has_update_data)
+            logger.debug("[%s] Fallback: has update_data=%s", plugin_id, has_update_data)
             if has_update_data:
                 try:
                     plugin.update_data()
-                    logger.info("[%s] Fallback: update_data() called", plugin_id)
+                    logger.debug("[%s] Fallback: update_data() called", plugin_id)
                 except (AttributeError, RuntimeError, OSError):
                     logger.exception("[%s] Fallback: update_data() failed", plugin_id)
 
@@ -1097,41 +1097,41 @@ class PluginAdapter:
             # arrangement rather than one that has to be cropped afterwards.
             render_width = self.resolve_render_width(plugin, plugin_id)
             if render_width != self.display_width:
-                logger.info(
+                logger.debug(
                     "[%s] Fallback: rendering at %dpx instead of %dpx",
                     plugin_id, render_width, self.display_width
                 )
 
             with self._capture(), self._render_at(render_width):
                 self.display_manager.clear()
-                logger.info("[%s] Fallback: display cleared, calling display()", plugin_id)
+                logger.debug("[%s] Fallback: display cleared, calling display()", plugin_id)
 
                 # First try without force_clear (some plugins behave better this way)
                 try:
                     plugin.display()
-                    logger.info("[%s] Fallback: display() called successfully", plugin_id)
+                    logger.debug("[%s] Fallback: display() called successfully", plugin_id)
                 except TypeError:
                     # Plugin may require force_clear argument
-                    logger.info("[%s] Fallback: display() failed, trying with force_clear=True", plugin_id)
+                    logger.debug("[%s] Fallback: display() failed, trying with force_clear=True", plugin_id)
                     plugin.display(force_clear=True)
 
                 # Capture the result
                 captured = self.display_manager.image.copy()
 
-            logger.info(
+            logger.debug(
                 "[%s] Fallback: captured frame %dx%d, mode=%s",
                 plugin_id, captured.width, captured.height, captured.mode
             )
 
             # Check if captured image has content (not all black)
             is_blank, bright_ratio = self._is_blank_image(captured, return_ratio=True)
-            logger.info(
+            logger.debug(
                 "[%s] Fallback: brightness check - %.3f%% bright pixels (threshold=0.5%%)",
                 plugin_id, bright_ratio * 100
             )
 
             if is_blank:
-                logger.info(
+                logger.debug(
                     "[%s] Fallback: first capture blank, retrying with force_clear",
                     plugin_id
                 )
@@ -1142,7 +1142,7 @@ class PluginAdapter:
                     captured = self.display_manager.image.copy()
 
                 is_blank, bright_ratio = self._is_blank_image(captured, return_ratio=True)
-                logger.info(
+                logger.debug(
                     "[%s] Fallback: retry brightness - %.3f%% bright pixels",
                     plugin_id, bright_ratio * 100
                 )
@@ -1159,7 +1159,7 @@ class PluginAdapter:
             if captured.mode != 'RGB':
                 captured = captured.convert('RGB')
 
-            logger.info(
+            logger.debug(
                 "[%s] Fallback: SUCCESS - captured %dx%d",
                 plugin_id, captured.width, captured.height
             )

src/web_interface/secret_helpers.py

@@ -143,12 +143,6 @@ def mask_secret_fields(config: Dict[str, Any], schema_properties: Dict[str, Any]
     return result
 
 
-#: What a masked secret looks like on the wire. Named because the write path
-#: has to recognise it coming back: a client that renders the mask and posts
-#: it unchanged must not store the mask as if it were the secret.
-SECRET_MASK = '\u2022' * 8
-
-
 def mask_all_secret_values(config: Dict[str, Any]) -> Dict[str, Any]:
     """Blanket-mask every non-empty value in a secrets config dict.
 
@@ -167,7 +161,7 @@ def mask_all_secret_values(config: Dict[str, Any]) -> Dict[str, Any]:
         if isinstance(v, dict):
             masked[k] = mask_all_secret_values(v)
         elif v not in (None, '') and not (isinstance(v, str) and v.startswith('YOUR_')):
-            masked[k] = SECRET_MASK
+            masked[k] = '••••••••'
         else:
             masked[k] = v
     return masked
@@ -195,30 +189,3 @@ def remove_empty_secrets(secrets: Dict[str, Any]) -> Dict[str, Any]:
         elif v is not None and not (isinstance(v, str) and v.strip() == ''):
             result[k] = v
     return result
-
-
-def strip_masked_values(secrets: Dict[str, Any]) -> Dict[str, Any]:
-    """Remove values a client echoed back rather than changed.
-
-    The counterpart to :func:`mask_all_secret_values`. A client that GETs the
-    masked secrets, edits one field and POSTs the whole object back is sending
-    ``SECRET_MASK`` for every field it did not touch. Storing those would
-    replace each untouched credential with eight bullet characters.
-
-    Drops the mask and, like :func:`remove_empty_secrets`, blank values -- so
-    the caller can merge the result onto what is already stored and have
-    "unchanged" mean unchanged. Empty nested dicts are pruned.
-    """
-    result: Dict[str, Any] = {}
-    for k, v in secrets.items():
-        if isinstance(v, dict):
-            nested = strip_masked_values(v)
-            if nested:
-                result[k] = nested
-        elif v is None:
-            continue
-        elif isinstance(v, str) and (v.strip() == '' or v == SECRET_MASK):
-            continue
-        else:
-            result[k] = v
-    return result

test/test_config_main_redacts_secrets.py

@@ -1,143 +0,0 @@
-"""GET /config/main must not hand out credentials.
-
-The endpoint returned the raw config to anyone who could reach the port, and
-this web interface has no authentication of any kind. Measured against a live
-rig, an unauthenticated request returned:
-
-    github.api_token                40 chars
-    incoming-packages.ha_token     183 chars
-    jellyfin-now-playing.api_key    32 chars
-    ledmatrix-weather.api_key       32 chars
-    on-air.mqtt_password             8 chars
-    youtube.api_key                 20 chars
-    youtube-stats.api_key           39 chars
-
-A GitHub token and a Home Assistant long-lived token among them.
-
-The x-secret masking the plugin config endpoints use does not apply here: this
-endpoint never consults a schema, and core keys such as github.api_token have
-no schema to carry the marker. Several of those fields *are* tagged x-secret in
-their plugin's schema and were still returned in full, which is what makes the
-schema route the wrong one to rely on for this endpoint.
-
-Matching on field name is blunt. For a whole-config dump it is the right
-default: anything named like a credential should not leave the process, and a
-new plugin that adds a differently-shaped secret is covered without anyone
-remembering to tag it.
-"""
-import pytest
-
-from web_interface.blueprints.api_v3 import (
-    _looks_like_a_credential,
-    _redact_credentials,
-)
-
-
-@pytest.mark.parametrize("name", [
-    "password", "mqtt_password", "opensky_password", "passwd",
-    "api_key", "apikey", "API_KEY", "flightaware_api_key",
-    "token", "ha_token", "api_token", "access_token",
-    "secret", "client_secret", "spotify_client_secret",
-    "access_key", "private_key",
-])
-def test_credential_names_are_recognised(name):
-    assert _looks_like_a_credential(name)
-
-
-@pytest.mark.parametrize("name", [
-    "timezone", "city", "brightness", "enabled", "update_interval",
-    "favorite_teams", "display_duration", "keyword",
-])
-def test_ordinary_names_are_left_alone(name):
-    assert not _looks_like_a_credential(name)
-
-
-def test_the_measured_leak_is_closed():
-    """The exact shape taken off the rig."""
-    config = {
-        "github": {"api_token": "ghp_" + "x" * 36},
-        "incoming-packages": {"ha_token": "y" * 183, "enabled": True},
-        "jellyfin-now-playing": {"api_key": "z" * 32},
-        "on-air": {"mqtt_password": "hunter22"},
-        "youtube": {"api_key": "k" * 20},
-        "timezone": "America/New_York",
-    }
-    out = _redact_credentials(config)
-    assert out["github"]["api_token"] == ""
-    assert out["incoming-packages"]["ha_token"] == ""
-    assert out["jellyfin-now-playing"]["api_key"] == ""
-    assert out["on-air"]["mqtt_password"] == ""
-    assert out["youtube"]["api_key"] == ""
-    # Everything else survives, or the config editor breaks.
-    assert out["timezone"] == "America/New_York"
-    assert out["incoming-packages"]["enabled"] is True
-
-
-def test_nested_and_listed_credentials_are_reached():
-    config = {"a": {"b": {"c": {"password": "p"}}},
-              "feeds": [{"name": "x", "api_key": "k"}, {"name": "y"}]}
-    out = _redact_credentials(config)
-    assert out["a"]["b"]["c"]["password"] == ""
-    assert out["feeds"][0]["api_key"] == ""
-    assert out["feeds"][0]["name"] == "x"
-
-
-def test_the_original_is_not_mutated():
-    """The caller holds the live config; redaction must not edit it in place."""
-    config = {"github": {"api_token": "keepme"}}
-    _redact_credentials(config)
-    assert config["github"]["api_token"] == "keepme"
-
-
-def test_a_credential_shaped_container_is_still_walked():
-    """`secrets: {...}` is a section name, not a value to blank."""
-    config = {"secrets": {"api_key": "k", "note": "keep"}}
-    out = _redact_credentials(config)
-    assert out["secrets"]["api_key"] == ""
-    assert out["secrets"]["note"] == "keep"
-
-
-def test_non_dict_input_passes_through():
-    assert _redact_credentials("plain") == "plain"
-    assert _redact_credentials(7) == 7
-    assert _redact_credentials(None) is None
-
-
-def test_the_endpoint_itself_redacts():
-    """Through the view function, not the helper.
-
-    The helper tests above all passed with the route still returning
-    `config` -- reverting the one line that calls the redactor changed
-    nothing, because nothing exercised the route. A property asserted on a
-    helper is not a property asserted on the endpoint, and it is the endpoint
-    that is exposed to the network.
-    """
-    import json as _json
-    from unittest.mock import MagicMock
-
-    import flask
-
-    from web_interface.blueprints import api_v3 as mod
-
-    raw = {"github": {"api_token": "ghp_secret_value"},
-           "timezone": "America/New_York"}
-
-    manager = MagicMock()
-    manager.load_config.return_value = raw
-    previous = getattr(mod.api_v3, "config_manager", None)
-    mod.api_v3.config_manager = manager
-
-    app = flask.Flask(__name__)
-    try:
-        with app.test_request_context("/config/main"):
-            response = mod.get_main_config()
-        payload = response.get_json() if hasattr(response, "get_json") else _json.loads(response[0].data)
-    finally:
-        mod.api_v3.config_manager = previous
-
-    data = payload["data"]
-    assert data["github"]["api_token"] == "", (
-        "the endpoint returned the token; the redactor is not wired in")
-    assert data["timezone"] == "America/New_York"
-    # And the config the manager handed over is untouched.
-    assert raw["github"]["api_token"] == "ghp_secret_value"

test/test_git_pull_resolution.py

@@ -58,7 +58,7 @@ def repos(tmp_path):
 def test_branch_with_upstream_uses_a_plain_pull(repos):
     args, note, error = resolve_pull_command(str(repos))
     assert error is None
-    assert args == ['git', 'pull', '--rebase', '--autostash']
+    assert args == ['git', 'pull', '--rebase']
     assert note == ''
 
 
@@ -73,7 +73,7 @@ def test_branch_without_upstream_falls_back_to_origin_branch(repos):
 
     args, note, error = resolve_pull_command(str(repos))
     assert error is None
-    assert args == ['git', 'pull', '--rebase', '--autostash', 'origin', 'audit']
+    assert args == ['git', 'pull', '--rebase', 'origin', 'audit']
     assert 'audit' in note
 
 
@@ -155,7 +155,7 @@ def test_switching_attaches_tracking_so_pull_needs_no_fallback(repos):
 
     args, note, error = resolve_pull_command(str(repos))
     assert error is None
-    assert args == ['git', 'pull', '--rebase', '--autostash']
+    assert args == ['git', 'pull', '--rebase']
     assert note == ''
 
 
@@ -200,37 +200,3 @@ def test_stash_option_lets_the_switch_through_and_keeps_the_work(repos):
     assert _git('branch', '--show-current', cwd=repos).stdout.strip() == 'other'
     # The edit is not lost — it is on the stash.
     assert 'switch to other' in _git('stash', 'list', cwd=repos).stdout
-
-
-class TestInstallerDoesNotBlockTheUpdateButton:
-    """first_time_install.sh chmods scripts that git tracked as 644.
-
-    With core.fileMode true -- the default on Linux -- that leaves five
-    permanently modified tracked files on every machine that ran the
-    installer, and `git pull --rebase` refuses to start:
-
-        error: cannot pull with rebase: You have unstaged changes.
-
-    Tracking them as executable makes the installer's chmod a no-op.
-    """
-
-    CHMODDED = [
-        'first_time_install.sh',
-        'start_display.sh',
-        'stop_display.sh',
-        'scripts/install/install_service.sh',
-        'scripts/install/install_web_service.sh',
-    ]
-
-    def test_scripts_the_installer_chmods_are_tracked_executable(self):
-        import subprocess
-        from pathlib import Path
-        root = Path(__file__).resolve().parent.parent
-        out = subprocess.run(['git', 'ls-files', '-s', *self.CHMODDED],
-                             capture_output=True, text=True, cwd=str(root)).stdout
-        modes = {line.split()[3]: line.split()[0] for line in out.strip().split('\n') if line}
-        non_exec = sorted(f for f, m in modes.items() if m != '100755')
-        assert not non_exec, (
-            f"{non_exec} are chmodded by the installer but tracked non-executable, "
-            "so every install leaves the working tree dirty and the update "
-            "button cannot pull")

test/test_health_write_churn.py

@@ -0,0 +1,110 @@
+"""A healthy plugin must not rewrite its health record every cycle.
+
+Every successful plugin update called record_success(), which persisted the
+record unconditionally. In steady state the only fields that had changed were
+total_successes and last_success_time -- a counter and a timestamp that
+health_monitor reads for display and that nothing reads back after a restart.
+
+Measured on a rig running 24 plugins: about 17 health-file rewrites a minute,
+roughly 25,000 a day. Each is ~400 bytes, but they land on an SD card where
+the unit of cost is an erase-block cycle, not the byte count, and where wear is
+what eventually kills the card.
+
+The circuit breaker still needs its own state to survive a restart, so the
+write is kept for exactly the fields it is rebuilt from -- and a failure, a
+circuit opening, or a recovery must still be written the moment it happens.
+"""
+import time
+
+import pytest
+
+from src.plugin_system.plugin_health import PluginHealthTracker, CircuitState
+
+
+class _Cache:
+    """Counts writes; serves back whatever was last written."""
+
+    def __init__(self):
+        self.store = {}
+        self.writes = 0
+
+    def set(self, key, data, ttl=None, **kwargs):
+        self.writes += 1
+        self.store[key] = data
+
+    def get(self, key, max_age=None, memory_ttl=None, **kwargs):
+        return self.store.get(key)
+
+
+@pytest.fixture
+def tracker():
+    cache = _Cache()
+    t = PluginHealthTracker(cache_manager=cache)
+    return t, cache
+
+
+def test_steady_state_success_stops_writing(tracker):
+    """The regression: 100 healthy cycles used to be 100 SD writes."""
+    t, cache = tracker
+    t.record_success("weather")
+    first = cache.writes
+    for _ in range(100):
+        t.record_success("weather")
+    assert cache.writes == first, (
+        f"{cache.writes - first} redundant writes across 100 healthy cycles"
+    )
+
+
+def test_the_counters_are_still_accurate_in_memory(tracker):
+    """Skipping the write must not skip the bookkeeping."""
+    t, _ = tracker
+    for _ in range(10):
+        t.record_success("weather")
+    state = t.get_health_state("weather")
+    assert state["total_successes"] == 10
+    assert state["last_success_time"] is not None
+    assert state["last_success_time"] <= time.time()
+
+
+def test_a_failure_is_written_immediately(tracker):
+    t, cache = tracker
+    t.record_success("weather")
+    before = cache.writes
+    t.record_failure("weather", RuntimeError("boom"))
+    assert cache.writes > before, "a failure must reach disk"
+
+
+def test_recovery_after_failure_is_written(tracker):
+    """consecutive_failures returning to 0 is durable state changing."""
+    t, cache = tracker
+    t.record_failure("weather", RuntimeError("boom"))
+    before = cache.writes
+    t.record_success("weather")
+    assert cache.writes > before, "recovery must reach disk"
+    assert t.get_health_state("weather")["consecutive_failures"] == 0
+
+
+def test_a_closing_circuit_is_written(tracker):
+    """Success in half-open closes the circuit -- that must survive a restart."""
+    t, cache = tracker
+    state = t.get_health_state("weather")
+    state["circuit_state"] = CircuitState.HALF_OPEN.value
+    state["half_open_start_time"] = time.time()
+    before = cache.writes
+    t.record_success("weather")
+    assert cache.writes > before, "a circuit transition must reach disk"
+    assert t.get_health_state("weather")["circuit_state"] == CircuitState.CLOSED.value
+
+
+def test_durable_state_survives_a_restart(tracker):
+    """What is skipped must genuinely not matter to the breaker."""
+    t, cache = tracker
+    for _ in range(3):
+        t.record_failure("weather", RuntimeError("boom"))
+    for _ in range(50):
+        t.record_success("weather")
+
+    revived = PluginHealthTracker(cache_manager=cache)
+    state = revived.get_health_state("weather")
+    assert state["consecutive_failures"] == 0
+    assert state["circuit_state"] == CircuitState.CLOSED.value

test/test_journald_log_priority.py

@@ -0,0 +1,101 @@
+"""Log lines must reach the journal with their real severity.
+
+Everything this process writes to stdout lands in the journal as PRIORITY=6,
+whatever the Python level was, because journald has no other signal. Measured
+on a live rig over 24 hours: 55 lines containing " - ERROR - " and 13
+containing " - WARNING - ", every one of them recorded as informational. So
+
+    journalctl -p err -u ledmatrix
+
+returned nothing while errors were being logged, and anyone triaging has to
+grep the message text instead. That is slower and it is wrong: a search for
+"oom" also matches the radar logging "zoom=9", which is exactly the false
+positive it produced during this audit.
+
+systemd reads a leading "<N>" on each stdout line and uses it as the priority
+(sd-daemon(3)), so this needs no extra dependency -- and it must only be
+applied when systemd is actually reading, or the prefixes become literal noise
+in a terminal, the emulator, and test output.
+"""
+import logging
+import os
+from unittest.mock import patch
+
+import pytest
+
+from src.logging_config import JournalPriorityFormatter, _SYSLOG_PRIORITY, _under_systemd
+
+
+class _Plain(logging.Formatter):
+    def format(self, record):
+        return record.getMessage()
+
+
+def _record(level, msg="hello"):
+    return logging.LogRecord("t", level, "f.py", 1, msg, None, None)
+
+
+@pytest.mark.parametrize("level,expected", [
+    (logging.CRITICAL, 2),
+    (logging.ERROR, 3),
+    (logging.WARNING, 4),
+    (logging.INFO, 6),
+    (logging.DEBUG, 7),
+])
+def test_each_level_maps_to_its_syslog_priority(level, expected):
+    out = JournalPriorityFormatter(_Plain()).format(_record(level))
+    assert out.startswith(f"<{expected}>"), out
+    assert _SYSLOG_PRIORITY[level] == expected
+
+
+def test_error_and_info_are_distinguishable():
+    """The whole point: journalctl -p err must be able to tell them apart."""
+    fmt = JournalPriorityFormatter(_Plain())
+    assert fmt.format(_record(logging.ERROR))[:3] != fmt.format(_record(logging.INFO))[:3]
+
+
+def test_every_line_of_a_multiline_record_is_tagged():
+    """The journal splits them, and an untagged continuation loses its level.
+
+    A traceback is the case that matters -- it is the most important thing in
+    the log and the longest.
+    """
+    out = JournalPriorityFormatter(_Plain()).format(
+        _record(logging.ERROR, "Traceback:\nline one\nline two"))
+    lines = out.split("\n")
+    assert len(lines) == 3
+    assert all(line.startswith("<3>") for line in lines), lines
+
+
+def test_the_message_survives_intact():
+    out = JournalPriorityFormatter(_Plain()).format(_record(logging.WARNING, "disk full"))
+    assert out == "<4>disk full"
+
+
+def test_an_unknown_level_falls_back_to_info():
+    out = JournalPriorityFormatter(_Plain()).format(_record(25))
+    assert out.startswith("<6>")
+
+
+def test_prefixing_is_off_outside_systemd():
+    """Otherwise a terminal run, the emulator and pytest all show `<6>`."""
+    with patch.dict(os.environ, {}, clear=True):
+        assert not _under_systemd()
+    with patch.dict(os.environ, {"JOURNAL_STREAM": "8:12345"}):
+        assert _under_systemd()
+
+
+def test_setup_uses_the_wrapper_only_under_systemd():
+    from src.logging_config import setup_logging
+
+    for env, expect_wrapped in (({}, False), ({"JOURNAL_STREAM": "8:1"}, True)):
+        with patch.dict(os.environ, env, clear=True):
+            setup_logging()
+            handlers = [h for h in logging.getLogger().handlers
+                        if isinstance(h, logging.StreamHandler)]
+            assert handlers, "no stream handler installed"
+            wrapped = any(isinstance(h.formatter, JournalPriorityFormatter)
+                          for h in handlers)
+            assert wrapped is expect_wrapped, (
+                f"JOURNAL_STREAM={env}: wrapped={wrapped}, expected {expect_wrapped}")
+    logging.getLogger().handlers.clear()

test/test_logging_config.py

@@ -183,15 +183,27 @@ class TestSetupLogging:
         setup_logging()
         assert len(logging.getLogger().handlers) == 1
 
+    @staticmethod
+    def _selected_formatter():
+        """The formatter setup_logging() chose, past any journald wrapper.
+
+        Under systemd the console handler's formatter is wrapped so each line
+        carries its syslog priority. That wrapper is applied only when
+        JOURNAL_STREAM is set, which is true in CI and false in a terminal, so
+        asserting on the handler's formatter directly passes locally and fails
+        on the runner. These tests are about which formatter format_type
+        selects, so they look through the wrapper.
+        """
+        formatter = logging.getLogger().handlers[0].formatter
+        return getattr(formatter, "inner", formatter)
+
     def test_json_format_selects_structured_formatter(self):
         setup_logging(format_type="json")
-        assert isinstance(
+        assert isinstance(self._selected_formatter(), StructuredFormatter)
-            logging.getLogger().handlers[0].formatter, StructuredFormatter)
 
     def test_readable_format_selects_contextual_formatter(self):
         setup_logging(format_type="readable")
-        assert isinstance(
+        assert isinstance(self._selected_formatter(), ContextualFormatter)
-            logging.getLogger().handlers[0].formatter, ContextualFormatter)
 
     def test_log_file_adds_file_handler(self, tmp_path):
         log_file = tmp_path / "test.log"

test/test_metrics_cache_unknown_fields.py

@@ -0,0 +1,100 @@
+"""A malformed metrics cache entry must not take every plugin down with it.
+
+`ResourceMetrics(**cached)` raises TypeError on a single unexpected key, and
+that exception escapes into plugin_manager, which reports it per plugin as
+"plugin <id> operation failed". Every plugin fails and the plugin system never
+finishes initialising -- the health endpoint reports
+`plugin_system: not_initialized` while the display itself keeps running.
+
+Seen on a live rig, once per plugin, continuously:
+
+    ERROR - src.plugin_system.plugin_manager - plugin geochron operation failed:
+    ResourceMetrics.__init__() got an unexpected keyword argument
+    'consecutive_failures'
+
+`consecutive_failures` belongs to plugin_health, not to metrics. How a
+health-shaped record came to sit under a plugin_metrics key on that machine is
+not established -- a restored backup that mixed two machines' caches is the
+likeliest explanation, and the same rig had one restored onto it -- but a
+loader that turns one bad cache entry into a total outage is the part worth
+fixing. plugin_health already repairs its own records field by field rather
+than trusting what is on disk.
+"""
+import logging
+from dataclasses import fields
+from unittest.mock import MagicMock
+
+import pytest
+
+from src.plugin_system.resource_monitor import PluginResourceMonitor, ResourceMetrics
+
+
+class _Cache:
+    def __init__(self, payload=None):
+        self.payload = payload
+
+    def get(self, key, max_age=None, memory_ttl=None, **kwargs):
+        return self.payload
+
+    def set(self, key, data, ttl=None, **kwargs):
+        pass
+
+
+def _monitor(payload):
+    m = PluginResourceMonitor(cache_manager=_Cache(payload))
+    m.logger = logging.getLogger("test")
+    return m
+
+
+#: What the rig actually had under the metrics key.
+HEALTH_SHAPED = {
+    "consecutive_failures": 0, "circuit_state": "closed",
+    "circuit_opened_time": None, "half_open_start_time": None,
+    "last_error": None, "last_failure_time": None,
+    "last_success_time": 1_700_000_000.0, "total_failures": 0,
+    "total_successes": 42,
+}
+
+
+def test_a_health_record_under_the_metrics_key_does_not_raise():
+    """The exact failure: it must degrade, not take the plugin system down."""
+    monitor = _monitor(HEALTH_SHAPED)
+    metrics = monitor.get_metrics(" plugin-a".strip())
+    assert isinstance(metrics, ResourceMetrics)
+
+
+def test_recognised_fields_in_a_mixed_record_are_kept():
+    """Dropping the record wholesale would lose real history unnecessarily."""
+    mixed = dict(HEALTH_SHAPED, call_count=7, memory_mb=12.5)
+    metrics = _monitor(mixed).get_metrics("plugin-b")
+    assert metrics.call_count == 7
+    assert metrics.memory_mb == 12.5
+
+
+def test_a_clean_record_still_loads_unchanged():
+    clean = {f.name: 3 for f in fields(ResourceMetrics)}
+    metrics = _monitor(clean).get_metrics("plugin-c")
+    for name in (f.name for f in fields(ResourceMetrics)):
+        assert getattr(metrics, name) == 3
+
+
+def test_unknown_fields_are_named_in_the_log(caplog):
+    """Silently discarding them would hide a real schema change."""
+    with caplog.at_level(logging.WARNING):
+        _monitor(HEALTH_SHAPED).get_metrics("plugin-d")
+    # getMessage(), not .message: the latter is only populated once a handler
+    # formats the record, so the obvious spelling silently never matches.
+    assert any("consecutive_failures" in r.getMessage() for r in caplog.records), \
+        caplog.text
+
+
+@pytest.mark.parametrize("payload", ["a string", 42, ["a", "list"]])
+def test_a_non_mapping_cache_entry_does_not_raise(payload):
+    metrics = _monitor(payload).get_metrics("plugin-e")
+    assert isinstance(metrics, ResourceMetrics)
+
+
+def test_values_of_the_wrong_type_do_not_raise():
+    """A dataclass will accept these, but a later float() on them would not."""
+    metrics = _monitor({"call_count": "not a number"}).get_metrics("plugin-f")
+    assert isinstance(metrics, ResourceMetrics)

test/test_resource_monitor.py

@@ -127,3 +127,67 @@ class TestForceReload:
         fresh = mon.get_metrics_summary("p", force_reload=True)
         assert fresh["call_count"] == 7
         assert any(c.kwargs.get("memory_ttl") == 0 for c in cache.get.call_args_list)
+
+
+class TestMetricsPersistenceChurn:
+    """Metrics are telemetry; writing them on every call wore the SD card.
+
+    Each write is a ~350-byte file, which on ext4 costs a 4KB block plus a
+    journal entry. At roughly nine calls a minute per plugin across fourteen
+    plugins it dominated the device's write volume.
+    """
+
+    def test_repeated_calls_persist_once_per_interval(self):
+        cache = _cache()
+        mon = PluginResourceMonitor(cache, enable_monitoring=False)
+        for _ in range(50):
+            mon.monitor_call("p", lambda: None)
+        writes = [c for c in cache.set.call_args_list
+                  if c.args and str(c.args[0]).startswith("plugin_metrics:")]
+        assert len(writes) == 1, (
+            f"50 calls produced {len(writes)} metric writes; expected 1")
+
+    def test_the_interval_elapsing_allows_the_next_write(self, monkeypatch):
+        import src.plugin_system.resource_monitor as rm
+        cache = _cache()
+        mon = PluginResourceMonitor(cache, enable_monitoring=False)
+        mon.monitor_call("p", lambda: None)
+        # pretend the interval has passed
+        mon._metrics_persisted_at["p"] -= rm._METRICS_PERSIST_INTERVAL + 1
+        mon.monitor_call("p", lambda: None)
+        writes = [c for c in cache.set.call_args_list
+                  if c.args and str(c.args[0]).startswith("plugin_metrics:")]
+        assert len(writes) == 2
+
+    def test_in_memory_metrics_stay_exact_while_writes_are_skipped(self):
+        mon = PluginResourceMonitor(_cache(), enable_monitoring=False)
+        for _ in range(20):
+            mon.monitor_call("p", lambda: None)
+        assert mon.get_metrics("p").call_count == 20
+
+    def test_reset_lets_the_next_call_persist_immediately(self):
+        cache = _cache()
+        mon = PluginResourceMonitor(cache, enable_monitoring=False)
+        mon.monitor_call("p", lambda: None)
+        mon.reset_metrics("p")
+        mon.monitor_call("p", lambda: None)
+        writes = [c for c in cache.set.call_args_list
+                  if c.args and str(c.args[0]).startswith("plugin_metrics:")]
+        assert len(writes) == 2, "reset should clear the throttle timestamp"
+
+    def test_a_failed_write_does_not_buy_the_next_interval_of_silence(self):
+        """A set() that raises must not count as having persisted.
+
+        Marking the timestamp before the write would leave no snapshot in the
+        cache and still suppress the next 30 seconds of attempts.
+        """
+        cache = _cache()
+        cache.set.side_effect = [OSError("disk full"), None]
+        mon = PluginResourceMonitor(cache, enable_monitoring=False)
+        with pytest.raises(OSError):
+            mon.monitor_call("p", lambda: None)
+        # the very next call must try again rather than skip the interval
+        mon.monitor_call("p", lambda: None)
+        writes = [c for c in cache.set.call_args_list
+                  if c.args and str(c.args[0]).startswith("plugin_metrics:")]
+        assert len(writes) == 2, "a failed write should be retried, not skipped"

test/test_update_check_reports_failure.py

@@ -1,85 +0,0 @@
-"""A check that could not run must not be reported as "up to date".
-
-check-update returned update_available=False whenever git failed. The banner
-is the only route to the update button, so a checkout git refuses to touch
-looked exactly like a current one -- permanently, and with nothing for the
-user to act on. The usual cause is an install performed as root, after which
-every git command fails with "detected dubious ownership".
-"""
-import subprocess
-import sys
-from pathlib import Path
-from unittest.mock import patch
-
-import pytest
-from flask import Flask
-
-sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
-
-from web_interface.blueprints import api_v3 as mod  # noqa: E402
-from web_interface.blueprints.api_v3 import api_v3  # noqa: E402
-
-DUBIOUS = ("fatal: detected dubious ownership in repository at "
-           "'/home/pi/LEDMatrix'\nTo add an exception for this directory, call:\n"
-           "\tgit config --global --add safe.directory /home/pi/LEDMatrix\n")
-
-
-@pytest.fixture
-def client():
-    app = Flask(__name__)
-    app.config['TESTING'] = True
-    app.register_blueprint(api_v3, url_prefix='/api/v3')
-    mod._update_check_cache['result'] = None
-    mod._update_check_cache['ts'] = 0
-    return app.test_client()
-
-
-def _fetch_fails(stderr: bytes):
-    def fake_run(args, **kwargs):
-        if args[:2] == ['git', 'fetch']:
-            return subprocess.CompletedProcess(args, 1, stdout=b'', stderr=stderr)
-        return subprocess.CompletedProcess(args, 0, stdout='', stderr='')
-    return fake_run
-
-
-class TestFailedCheckIsNotSilence:
-    def test_dubious_ownership_is_reported_not_swallowed(self, client):
-        with patch.object(mod.subprocess, 'run', _fetch_fails(DUBIOUS.encode())):
-            data = client.get('/api/v3/system/check-update').get_json()
-        assert data['check_failed'] is True, (
-            "a git failure was reported as a successful 'no update' check")
-        assert data['update_available'] is False
-
-    def test_the_message_tells_the_user_what_to_do(self, client):
-        with patch.object(mod.subprocess, 'run', _fetch_fails(DUBIOUS.encode())):
-            data = client.get('/api/v3/system/check-update').get_json()
-        assert 'chown' in data['error'], (
-            "dubious ownership is unactionable without the fix command")
-        assert 'root' in data['error']
-
-    def test_an_ordinary_git_failure_still_surfaces(self, client):
-        with patch.object(mod.subprocess, 'run',
-                          _fetch_fails(b'fatal: some other git problem\n')):
-            data = client.get('/api/v3/system/check-update').get_json()
-        assert data['check_failed'] is True
-        assert 'some other git problem' in data['error']
-
-    def test_offline_reads_as_offline(self, client):
-        with patch.object(mod.subprocess, 'run',
-                          _fetch_fails(b'fatal: could not resolve host: github.com\n')):
-            data = client.get('/api/v3/system/check-update').get_json()
-        assert 'Could not reach GitHub' in data['error']
-
-
-class TestSuccessPathUnchanged:
-    def test_up_to_date_carries_no_failure_flag(self, client):
-        def fake_run(args, **kwargs):
-            if args[:2] == ['git', 'fetch']:
-                return subprocess.CompletedProcess(args, 0, stdout=b'', stderr=b'')
-            if args[:2] == ['git', 'rev-parse']:
-                return subprocess.CompletedProcess(args, 0, stdout='abc123\n', stderr='')
-            return subprocess.CompletedProcess(args, 0, stdout='0\n', stderr='')
-        with patch.object(mod.subprocess, 'run', fake_run):
-            data = client.get('/api/v3/system/check-update').get_json()
-        assert data['update_available'] is False
-        assert not data.get('check_failed'), "a healthy check must not look like a failure"

test/test_update_prompts_restart.py

@@ -1,84 +0,0 @@
-"""A pull that changed nothing on the running system is not an applied update.
-
-git_pull replaces files on disk and restarts nothing -- there is no systemctl
-call anywhere in the handler. The display and web services keep running the
-code they loaded at boot, so the user is told "Code updated successfully" and
-sees no change until they happen to reboot. The response now says whether a
-restart is owed, and the UI raises the existing restart-pending banner.
-"""
-import subprocess
-import sys
-from pathlib import Path
-from unittest.mock import patch
-
-import pytest
-from flask import Flask
-
-sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
-
-from web_interface.blueprints import api_v3 as mod  # noqa: E402
-from web_interface.blueprints.api_v3 import api_v3  # noqa: E402
-
-
-@pytest.fixture
-def client():
-    app = Flask(__name__)
-    app.config['TESTING'] = True
-    app.register_blueprint(api_v3, url_prefix='/api/v3')
-    # The handler consults these after a successful pull; None is the
-    # "not wired up" case it already guards for.
-    api_v3.plugin_store_manager = None
-    api_v3.config_manager = None
-    return app.test_client()
-
-
-def _git(heads, pull_rc=0, pull_out='Updating a1b2c3..d4e5f6\n'):
-    """Fake git. `heads` are the successive answers to rev-parse HEAD."""
-    seq = list(heads)
-
-    def run(args, **kwargs):
-        def ok(stdout='', rc=0, b=False):
-            return subprocess.CompletedProcess(
-                args, rc, stdout=(stdout.encode() if b else stdout),
-                stderr=(b'' if b else ''))
-        if args[:2] == ['git', 'rev-parse'] and args[-1] == 'HEAD':
-            return ok(seq.pop(0) + '\n' if seq else 'deadbeef\n')
-        if 'symbolic-full-name' in args or '@{u}' in args:
-            return ok('origin/main\n')
-        if args[:2] == ['git', 'status']:
-            return ok('')
-        if args[:2] == ['git', 'diff']:
-            return ok('')
-        if args[:2] == ['git', 'pull']:
-            return ok(pull_out, pull_rc)
-        return ok('')
-    return run
-
-
-def _pull(client):
-    return client.post('/api/v3/system/action',
-                       json={'action': 'git_pull'}).get_json()
-
-
-class TestRestartIsRequestedWhenCodeChanged:
-    def test_a_pull_that_moved_head_asks_for_a_restart(self, client):
-        with patch.object(mod.subprocess, 'run', _git(['aaa111', 'bbb222'])):
-            data = _pull(client)
-        assert data['status'] == 'success'
-        assert data['restart_required'] is True, (
-            "new code on disk, services still running the old code, and "
-            "nothing told the user to restart")
-
-    def test_already_up_to_date_does_not(self, client):
-        with patch.object(mod.subprocess, 'run',
-                          _git(['aaa111', 'aaa111'], pull_out='Already up to date.\n')):
-            data = _pull(client)
-        assert data['status'] == 'success'
-        assert data['restart_required'] is False, (
-            "prompting after a no-op update trains users to ignore the prompt")
-
-    def test_a_failed_pull_does_not(self, client):
-        with patch.object(mod.subprocess, 'run', _git(['aaa111'], pull_rc=1)):
-            data = _pull(client)
-        assert data['status'] == 'error'
-        assert data['restart_required'] is False

test/test_vegas_log_volume.py

@@ -0,0 +1,63 @@
+"""The Vegas content path must trace at DEBUG, not INFO.
+
+plugin_adapter narrates every step of acquiring content from every plugin --
+"Has get_vegas_content", "Native: calling get_vegas_content()", "Native content
+returned None", "Has scroll_helper", the per-item sizes -- and it does that for
+each plugin on each cycle.
+
+Measured on a live rig: 13,408 log lines an hour, of which 13,366 were INFO and
+35 were WARNING. plugin_adapter alone produced 2,457 of them. That is ~223
+lines a minute of string formatting on a Pi that is also driving the panel, all
+of it written through journald to the SD card, and it buries the 35 lines that
+actually indicate a problem.
+
+Nothing is lost by moving it to DEBUG: the 19 warning/error/exception calls in
+the module are untouched, so real failures still surface at their own level.
+
+One INFO call is deliberate and stays -- the padding-strip message chooses its
+level at runtime (`logger.warning if (left and right) else logger.info`) and
+test_vegas_plugin_adapter.py pins it.
+"""
+import ast
+from pathlib import Path
+
+import pytest
+
+ADAPTER = (Path(__file__).resolve().parent.parent / "src" / "vegas_mode"
+           / "plugin_adapter.py")
+
+
+def _info_calls(path):
+    """Direct logger.info(...) call sites in a module."""
+    tree = ast.parse(path.read_text(encoding="utf-8"))
+    found = []
+    for node in ast.walk(tree):
+        if (isinstance(node, ast.Call)
+                and isinstance(node.func, ast.Attribute)
+                and node.func.attr == "info"
+                and getattr(node.func.value, "id", None) == "logger"):
+            found.append(node.lineno)
+    return found
+
+
+def test_the_content_path_does_not_trace_at_info():
+    calls = _info_calls(ADAPTER)
+    assert not calls, (
+        "plugin_adapter should trace at DEBUG; found logger.info at lines "
+        f"{calls}. This path runs per plugin per cycle and its output goes to "
+        "the SD card via journald."
+    )
+
+
+def test_real_failures_still_have_a_level_of_their_own():
+    """Demoting the trace must not have swept up the error reporting."""
+    source = ADAPTER.read_text(encoding="utf-8")
+    loud = sum(source.count(f"logger.{level}(")
+               for level in ("warning", "error", "exception"))
+    assert loud >= 15, f"only {loud} warning/error/exception calls remain"
+
+
+def test_the_deliberate_runtime_chosen_level_survives():
+    """The padding-strip message picks its level at runtime; leave it alone."""
+    source = ADAPTER.read_text(encoding="utf-8")
+    assert "logger.warning if (left and right) else logger.info" in source

test/web_interface/test_api_v3_secret_roundtrip.py

@@ -194,46 +194,15 @@ class TestSavePluginConfig:
 
     def test_secret_count_message_counts_top_level_keys(self, env):
         # Pinned: the "(N secret field(s))" message counts TOP-LEVEL keys of
-        # the separated secrets dict. Here that is 1: the posted accounts
+        # the separated secrets dict. Here that is 2: the posted accounts
-        # array, whose item tokens all count as ONE key.
+        # array (all its item tokens count as ONE key) plus the schema's
-        #
+        # api_key default ("") that merge_with_defaults adds before
-        # It was 2 before blank secrets were dropped, the second being the
+        # separation.
-        # schema's api_key default (""), which merge_with_defaults adds to
-        # every save. Counting it was the visible edge of a real bug: that
-        # injected blank was merged over the stored api_key, so saving any
-        # unrelated field destroyed the credential. See
-        # test_an_unrelated_edit_does_not_erase_a_stored_secret.
         resp = self._save(env, {
             "accounts": [{"name": "a", "token": "t"}],
         })
         message = resp.get_json()["message"]
-        assert "(1 secret field(s) saved to config_secrets.json)" in message
+        assert "(2 secret field(s) saved to config_secrets.json)" in message
-
-    def test_an_unrelated_edit_does_not_erase_a_stored_secret(self, env):
-        """Editing one field must not wipe the plugin's API key.
-
-        The config form renders secrets masked, so the browser posts them
-        back blank; merge_with_defaults injects a blank api_key even when
-        the client omits it entirely. Either way a "" reached the secrets
-        file and deep_merge wrote it over the stored credential.
-        """
-        assert self._save(env, {"api_key": "REAL-KEY-0123456789",
-                                "city": "Austin"}).status_code == 200
-        assert _on_disk(env.secrets_file)[PLUGIN_ID]["api_key"] == \
-            "REAL-KEY-0123456789"
-
-        # the user changes the city; the masked api_key rides along blank
-        assert self._save(env, {"api_key": "", "city": "Dallas"}).status_code == 200
-
-        assert _on_disk(env.secrets_file)[PLUGIN_ID]["api_key"] == \
-            "REAL-KEY-0123456789", "an unrelated edit destroyed the API key"
-        assert env.fresh_load()[PLUGIN_ID]["city"] == "Dallas"
-
-    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"})
-        self._save(env, {"api_key": "second-key"})
-        assert _on_disk(env.secrets_file)[PLUGIN_ID]["api_key"] == "second-key"
 
     def test_resave_replaces_stored_secrets_list_wholesale(self, env):
         # Characterized: api_v3's deep_merge intentionally replaces lists,

test/web_interface/test_config_secrets_masking.py

@@ -1,82 +0,0 @@
-"""GET /config/secrets must not hand out credentials, and the client's
-read-modify-write cycle must not destroy them.
-
-This interface has no authentication. The endpoint returned the whole
-config_secrets.json to anyone who could reach the port; on one rig that was a
-40-character GitHub token, a 183-character Home Assistant token and three API
-keys. Masking it alone is not enough: the only client fetches every secret,
-edits one field and posts all of them back, so the write path has to treat an
-echoed mask as "unchanged".
-"""
-import json
-import sys
-from pathlib import Path
-
-sys.path.insert(0, str(Path(__file__).parent))
-
-from test_api_v3_secret_roundtrip import env, _on_disk  # noqa: F401,E402
-from src.web_interface.secret_helpers import SECRET_MASK  # noqa: E402
-
-STORED = {
-    "github": {"api_token": "ghp_" + "x" * 36},
-    "ledmatrix-weather": {"api_key": "w" * 32},
-    "incoming-packages": {"ha_token": "h" * 183},
-    "unset-plugin": {"api_key": ""},
-    "placeholder-plugin": {"api_key": "YOUR_API_KEY_HERE"},
-}
-
-
-def _seed(env):
-    env.secrets_file.write_text(json.dumps(STORED))
-
-
-def _get(env):
-    r = env.client.get("/api/v3/config/secrets")
-    assert r.status_code == 200, r.get_data(as_text=True)[:200]
-    return r.get_json()["data"]
-
-
-def test_no_credential_leaves_the_process(env):
-    _seed(env)
-    body = json.dumps(_get(env))
-    for secret in ("ghp_" + "x" * 36, "w" * 32, "h" * 183):
-        assert secret not in body, "endpoint returned a stored credential"
-
-
-def test_set_and_unset_remain_distinguishable(env):
-    _seed(env)
-    data = _get(env)
-    assert data["github"]["api_token"] == SECRET_MASK
-    assert data["unset-plugin"]["api_key"] == ""
-    assert data["placeholder-plugin"]["api_key"] == "YOUR_API_KEY_HERE"
-
-
-def test_the_clients_read_modify_write_preserves_every_other_secret(env):
-    """What the GitHub-token save button actually does."""
-    _seed(env)
-    secrets = _get(env)                      # everything arrives masked
-    secrets["github"]["api_token"] = "ghp_" + "n" * 36   # user changes one
-    r = env.client.post("/api/v3/config/raw/secrets", json=secrets)
-    assert r.status_code == 200, r.get_data(as_text=True)[:200]
-
-    on_disk = _on_disk(env.secrets_file)
-    assert on_disk["github"]["api_token"] == "ghp_" + "n" * 36, "new token not saved"
-    assert on_disk["ledmatrix-weather"]["api_key"] == "w" * 32
-    assert on_disk["incoming-packages"]["ha_token"] == "h" * 183
-
-
-def test_a_mask_echoed_back_is_never_stored(env):
-    _seed(env)
-    env.client.post("/api/v3/config/raw/secrets", json=_get(env))
-    on_disk = _on_disk(env.secrets_file)
-    assert SECRET_MASK not in json.dumps(on_disk), "the mask was stored as a secret"
-    assert on_disk["github"]["api_token"] == "ghp_" + "x" * 36
-
-
-def test_a_brand_new_secret_can_still_be_added(env):
-    _seed(env)
-    env.client.post("/api/v3/config/raw/secrets",
-                    json={"new-plugin": {"api_key": "brand-new"}})
-    on_disk = _on_disk(env.secrets_file)
-    assert on_disk["new-plugin"]["api_key"] == "brand-new"
-    assert on_disk["github"]["api_token"] == "ghp_" + "x" * 36

web_interface/blueprints/api_v3.py

@@ -21,9 +21,7 @@ logger = logging.getLogger(__name__)
 # Import new infrastructure
 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,
+from src.web_interface.secret_helpers import find_secret_fields, separate_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
 from src.web_interface.validators import (
@@ -264,54 +262,15 @@ def _stop_display_service():
     result['status'] = status
     return result
 
-#: Field names whose value is a credential. Matched by name because this
-#: endpoint returns the whole config, core keys included, and core config has
-#: no schema to carry x-secret markers.
-_CREDENTIAL_NAME_PARTS = ("password", "passwd", "secret", "token", "api_key",
-                          "apikey", "access_key", "private_key", "client_secret")
-
-
-def _looks_like_a_credential(name: str) -> bool:
-    lowered = name.lower()
-    return any(part in lowered for part in _CREDENTIAL_NAME_PARTS)
-
-
-def _redact_credentials(value):
-    """A copy of `value` with credential-named fields blanked.
-
-    /config/main returned the raw config to anyone who could reach the port,
-    and this interface has no authentication. On one rig that meant a 40-char
-    GitHub token, a 183-char Home Assistant token and five API keys were
-    readable by anything on the LAN.
-
-    The x-secret masking used by the plugin config endpoints does not help
-    here: this endpoint never consults a schema, and core keys such as
-    github.api_token have no schema to mark. Matching on the field name is
-    blunt, but for a whole-config dump the right default is that anything
-    named like a credential does not leave the process.
-
-    Blanked rather than removed, and safe to blank: POST /config/main merges
-    into the loaded config and only writes the keys it was given, so a client
-    that round-trips this response cannot erase a secret it never saw.
-    """
-    if isinstance(value, dict):
-        return {k: ("" if _looks_like_a_credential(k) and not isinstance(v, (dict, list))
-                    else _redact_credentials(v))
-                for k, v in value.items()}
-    if isinstance(value, list):
-        return [_redact_credentials(item) for item in value]
-    return value
-
-
 @api_v3.route('/config/main', methods=['GET'])
 def get_main_config():
-    """Get main configuration, with credentials redacted."""
+    """Get main configuration"""
     try:
         if not api_v3.config_manager:
             return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500
 
         config = api_v3.config_manager.load_config()
-        return jsonify({'status': 'success', 'data': _redact_credentials(config)})
+        return jsonify({'status': 'success', 'data': config})
     except Exception as e:
         logger.error('Unhandled exception', exc_info=True)
         return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@@ -756,12 +715,10 @@ def save_main_config():
         if not data:
             return jsonify({'status': 'error', 'message': 'No data provided'}), 400
 
-        # What arrives here is the config itself, and the headers carry the
+        import logging
-        # session cookie -- neither belongs in the journal, least of all at
+        logging.error(f"DEBUG: save_main_config received data: {data}")
-        # ERROR on every save. The shape of the request is the part with
+        logging.error(f"DEBUG: Content-Type header: {request.content_type}")
-        # diagnostic value, so log that, at the level it deserves.
+        logging.error(f"DEBUG: Headers: {dict(request.headers)}")
-        logger.debug("save_main_config: %s, %d top-level key(s)",
-                     request.content_type or 'no content-type', len(data))
 
         # Merge with existing config (similar to original implementation)
         current_config = api_v3.config_manager.load_config()
@@ -1259,11 +1216,6 @@ def save_main_config():
 
                 # Separate secrets from regular config (same logic as save_plugin_config)
                 regular_config, secrets_config = separate_secrets(plugin_config, secret_fields)
-                # The config form renders secrets masked, so every save posts
-                # them back blank. Without this the blank is merged over the
-                # stored value and the credential is destroyed by the act of
-                # changing an unrelated setting. A blank means "unchanged".
-                secrets_config = remove_empty_secrets(secrets_config)
 
                 # PRE-PROCESSING: Preserve 'enabled' state if not in regular_config
                 # This prevents overwriting the enabled state when saving config from a form that doesn't include the toggle
@@ -1381,12 +1333,7 @@ def get_secrets_config():
             return jsonify({'status': 'error', 'message': 'Config manager not initialized'}), 500
 
         config = api_v3.config_manager.get_raw_file_content('secrets')
-        # This interface has no authentication, and this file is nothing but
+        return jsonify({'status': 'success', 'data': config})
-        # credentials. It was handing all of them to anyone who could reach
-        # the port. Values are masked; empty and YOUR_* placeholders are left
-        # alone so a client can still tell "set" from "not set".
-        return jsonify({'status': 'success',
-                        'data': mask_all_secret_values(config)})
     except Exception as e:
         logger.error('Unhandled exception', exc_info=True)
         return jsonify({'status': 'error', 'message': 'An error occurred; see logs for details', 'details': describe_exception(e)}), 500
@@ -1448,19 +1395,8 @@ def save_raw_secrets_config():
         if not data:
             return jsonify({'status': 'error', 'message': 'No data provided'}), 400
 
-        # The GET above masks what it returns, and this endpoint's only client
+        # Save the secrets config
-        # reads the whole file, edits one field and posts all of it back. So
+        api_v3.config_manager.save_raw_file_content('secrets', data)
-        # most of what arrives here is the mask, echoed rather than changed --
-        # storing it verbatim would replace every untouched credential with
-        # eight bullets. Strip those, then merge onto what is already stored,
-        # which makes "unchanged" mean unchanged.
-        #
-        # The cost is that a secret can no longer be cleared by blanking it.
-        # That needs its own affordance; a control that erases credentials as
-        # a side effect of saving an unrelated one is not it.
-        current = api_v3.config_manager.get_raw_file_content('secrets') or {}
-        merged = deep_merge(current, strip_masked_values(data))
-        api_v3.config_manager.save_raw_file_content('secrets', merged)
 
         # Reload GitHub token in plugin store manager if it exists
         if api_v3.plugin_store_manager:
@@ -1721,22 +1657,13 @@ def resolve_pull_command(project_dir):
     backup, or following an install guide that names one. The update button
     then reports a failure the user cannot act on.
 
-    ``--autostash`` is passed for the same reason. Rebase refuses to start
-    when any tracked file is modified, and on these installs something always
-    is: first_time_install.sh chmods five scripts that git tracked as 644, so
-    every machine that ran the installer carries five permanent mode changes
-    and the update button reports "cannot pull with rebase: You have unstaged
-    changes". Those modes are corrected in this commit, but a user cannot pull
-    the correction while the pull is what is blocked, and any other local edit
-    would reproduce it anyway. Autostash reapplies the changes afterwards.
-
     Returns ``(args, note, error)``. When ``origin/<branch>`` exists the pull
     is made explicit against it, so the update proceeds and the branch is
     given tracking information afterwards.
     """
     upstream = _git_upstream(project_dir)
     if upstream:
-        return ['git', 'pull', '--rebase', '--autostash'], '', None
+        return ['git', 'pull', '--rebase'], '', None
 
     branch = _git_current_branch(project_dir)
     if not branch:
@@ -1746,7 +1673,7 @@ def resolve_pull_command(project_dir):
         )
     if _git_remote_branch_exists(project_dir, branch):
         return (
-            ['git', 'pull', '--rebase', '--autostash', 'origin', branch],
+            ['git', 'pull', '--rebase', 'origin', branch],
             f"Branch '{branch}' had no upstream; pulled from origin/{branch} and set it as the upstream.",
             None,
         )
@@ -1894,33 +1821,6 @@ def get_system_version():
 _update_check_cache: Dict[str, Any] = {'result': None, 'ts': 0.0}
 _UPDATE_CHECK_TTL = 300  # 5 minutes — avoids a git fetch on every page load
 
-def _update_check_failed(detail: str) -> Dict[str, Any]:
-    """A check that could not run is not the same as being up to date.
-
-    Reporting update_available=False on a git failure hides the banner, and
-    the banner is the only route to the update button -- so a checkout git
-    refuses to touch looks exactly like a current one, permanently. The most
-    common cause is an install performed as root: git then reports "dubious
-    ownership" and every command fails, including the fetch here.
-    """
-    return {'update_available': False, 'remote_sha': 'unknown',
-            'commits_behind': 0, 'check_failed': True, 'error': detail}
-
-
-def _describe_git_failure(stderr: str) -> str:
-    """Turn git's stderr into something the user can act on."""
-    text = (stderr or '').strip()
-    if 'dubious ownership' in text or 'detected dubious ownership' in text:
-        return ("This checkout is owned by a different user than the one "
-                "running the web interface, so git refuses to use it. It is "
-                "usually the result of installing as root. Fix the ownership "
-                "and the update will work: sudo chown -R $USER:$USER "
-                + str(PROJECT_ROOT))
-    if 'could not resolve host' in text.lower() or 'network is unreachable' in text.lower():
-        return "Could not reach GitHub to check for updates."
-    return "Could not check for updates: " + (text.splitlines()[0] if text else "git failed")
-
-
 @api_v3.route('/system/check-update', methods=['GET'])
 def check_for_update():
     """Check whether a newer LEDMatrix commit is available on origin/main."""
@@ -1936,13 +1836,12 @@ def check_for_update():
             capture_output=True, timeout=10, cwd=cwd,
         )
         if fetch_result.returncode != 0:
-            stderr = fetch_result.stderr.decode(errors='replace').strip()
             logger.warning("check-update: git fetch failed (rc=%d): %s",
-                           fetch_result.returncode, stderr)
+                           fetch_result.returncode,
-            failed = _update_check_failed(_describe_git_failure(stderr))
+                           fetch_result.stderr.decode(errors='replace').strip())
-            _update_check_cache['result'] = failed
+            _update_check_cache['result'] = _safe
             _update_check_cache['ts'] = now
-            return jsonify(failed)
+            return jsonify(_safe)
         local = subprocess.run(
             ['git', 'rev-parse', 'HEAD'],
             capture_output=True, text=True, timeout=5, cwd=cwd,
@@ -1970,8 +1869,7 @@ def check_for_update():
         return jsonify(result)
     except Exception as e:
         logger.warning("check-update failed: %s", e)
-        return jsonify(_update_check_failed(
+        return jsonify(_safe)
-            "Could not check for updates; see logs for details."))
 
 @api_v3.route('/system/action', methods=['POST'])
 def execute_system_action():
@@ -2098,11 +1996,6 @@ def execute_system_action():
             except subprocess.TimeoutExpired:
                 logger.warning("git rev-parse timed out before pull")
 
-            # Whether the pull actually brought new code in. "Already up to
-            # date" is a success too, and prompting for a restart then would
-            # train users to ignore the prompt.
-            code_changed = False
-
             # Perform the git pull. Branches without an upstream were given
             # an explicit "origin <branch>" above so the update still works.
             result = subprocess.run(
@@ -2146,7 +2039,6 @@ def execute_system_action():
                                            capture_output=True, text=True, timeout=10, cwd=project_dir)
                     new_head = _post.stdout.strip() if _post.returncode == 0 else None
                     if old_head and new_head and old_head != new_head:
-                        code_changed = True
                         diff = subprocess.run(
                             ['git', 'diff', '--name-only', f'{old_head}..{new_head}'],
                             capture_output=True, text=True, timeout=15, cwd=project_dir)
@@ -2206,14 +2098,9 @@ def execute_system_action():
                                if ln.strip()), '')
                 pull_message = f"Update failed: {detail}" if detail else "Update failed; check logs for details"
 
-            # Nothing here restarts anything: the pull replaces files on
-            # disk while the display and web services keep running the code
-            # they loaded at boot. Without this the user is told the update
-            # succeeded and sees no change until they happen to reboot.
             return jsonify({
                 'status': 'success' if result.returncode == 0 else 'error',
                 'message': pull_message,
-                'restart_required': bool(result.returncode == 0 and code_changed),
             })
         elif action == 'checkout_branch':
             # Switch branches from the Tools tab. Needed because a checkout
@@ -5712,11 +5599,6 @@ def save_plugin_config():
         # Separate secrets from regular config (handles nested configs and
         # array-item secrets — see src/web_interface/secret_helpers.py)
         regular_config, secrets_config = separate_secrets(plugin_config, secret_fields)
-        # The config form renders secrets masked, so every save posts
-        # them back blank. Without this the blank is merged over the
-        # stored value and the credential is destroyed by the act of
-        # changing an unrelated setting. A blank means "unchanged".
-        secrets_config = remove_empty_secrets(secrets_config)
 
         # Get current configs
         current_config = api_v3.config_manager.load_config()

web_interface/static/v3/app.js

@@ -116,25 +116,14 @@ document.body.addEventListener('htmx:afterRequest', function(event) {
 // ===== Restart-pending banner =====
 // Shown after restart-requiring saves; persists across tab switches (and
 // reloads, via sessionStorage) until the display restarts or it's dismissed.
-window.showRestartPending = function(message) {
+window.showRestartPending = function() {
-    try {
+    try { sessionStorage.setItem('ledmatrix-restart-pending', '1'); } catch { /* private browsing */ }
-        sessionStorage.setItem('ledmatrix-restart-pending', '1');
-        // Persisted alongside the flag: a code update and a config save want
-        // different wording, and the banner outlives the page that raised it.
-        if (message) sessionStorage.setItem('ledmatrix-restart-pending-text', message);
-        else sessionStorage.removeItem('ledmatrix-restart-pending-text');
-    } catch { /* private browsing */ }
     const banner = document.getElementById('restart-pending-banner');
-    const text = document.getElementById('restart-pending-text');
-    if (text && message) text.textContent = message;
     if (banner) banner.style.display = 'block';
 };
 
 window.dismissRestartPending = function() {
-    try {
+    try { sessionStorage.removeItem('ledmatrix-restart-pending'); } catch { /* no-op */ }
-        sessionStorage.removeItem('ledmatrix-restart-pending');
-        sessionStorage.removeItem('ledmatrix-restart-pending-text');
-    } catch { /* no-op */ }
     const banner = document.getElementById('restart-pending-banner');
     if (banner) banner.style.display = 'none';
 };
@@ -162,9 +151,6 @@ document.addEventListener('DOMContentLoaded', function() {
     try {
         if (sessionStorage.getItem('ledmatrix-restart-pending') === '1') {
             const banner = document.getElementById('restart-pending-banner');
-            const saved = sessionStorage.getItem('ledmatrix-restart-pending-text');
-            const text = document.getElementById('restart-pending-text');
-            if (text && saved) text.textContent = saved;
             if (banner) banner.style.display = 'block';
         }
     } catch { /* no-op */ }

web_interface/static/v3/plugins_manager.js

@@ -4622,17 +4622,15 @@ window.loadGithubToken = function() {
                 // Handle empty data (secrets file doesn't exist) - API returns {} in this case
                 const secrets = data.data || {};
                 const token = secrets.github?.api_token || '';
-                const configured = token && token !== 'YOUR_GITHUB_PERSONAL_ACCESS_TOKEN';
 
                 if (input) {
-                    // The endpoint masks what it returns, so this never holds
+                    if (token && token !== 'YOUR_GITHUB_PERSONAL_ACCESS_TOKEN') {
-                    // the real token -- and the field is deliberately left
+                        // Token exists and is valid
-                    // empty rather than filled with the mask, which would be
+                        input.value = token;
-                    // saved verbatim the next time the user pressed Save.
+                        showNotification('GitHub token loaded successfully', 'success');
-                    input.value = '';
-                    if (configured) {
-                        showNotification('A GitHub token is saved. Enter a new one to replace it.', 'success');
                     } else {
+                        // No token configured or placeholder value
+                        input.value = '';
                         showNotification('No GitHub token configured. Enter a new token to save.', 'info');
                     }
                 }

web_interface/templates/v3/base.html

@@ -413,8 +413,7 @@
             <div class="flex items-center justify-between">
                 <div class="flex items-center space-x-3">
                     <i class="fas fa-rotate text-lg"></i>
-                    <span class="text-sm font-medium" aria-live="polite"
+                    <span class="text-sm font-medium" aria-live="polite">
-                          id="restart-pending-text">
                         Configuration saved &mdash; restart the display to apply the changes
                     </span>
                 </div>
@@ -1108,29 +1107,15 @@
         fetch('/api/v3/system/check-update')
             .then(function(r) { return r.json(); })
             .then(function(data) {
-                var banner = document.getElementById('update-banner');
-                var btn = document.getElementById('update-banner-btn');
-                if (data.check_failed) {
-                    // A check that could not run is not the same as being up
-                    // to date. Hiding the banner here made a checkout git
-                    // refuses to touch look permanently current, with no
-                    // route to the update button and nothing to act on.
-                    document.getElementById('update-banner-text').textContent =
-                        data.error || 'Could not check for updates.';
-                    if (btn) btn.style.display = 'none';
-                    banner.style.display = '';
-                    return;
-                }
-                if (btn) btn.style.display = '';
                 if (data.update_available && getDismissedSha() !== data.remote_sha) {
                     var n = data.commits_behind || 0;
                     var msg = 'A new LEDMatrix update is available';
                     if (n > 0) msg += ' (' + n + ' commit' + (n > 1 ? 's' : '') + ')';
                     document.getElementById('update-banner-text').textContent = msg;
-                    banner.style.display = '';
+                    document.getElementById('update-banner').style.display = '';
                     try { sessionStorage.setItem('update-sha', data.remote_sha); } catch(e) {}
                 } else {
-                    banner.style.display = 'none';
+                    document.getElementById('update-banner').style.display = 'none';
                 }
             })
             .catch(function() {});
@@ -1161,13 +1146,6 @@
             if (data.status === 'success') {
                 document.getElementById('update-banner').style.display = 'none';
                 try { sessionStorage.removeItem('update-sha-dismissed'); } catch(e) {}
-                // The pull replaced files on disk; the running services still
-                // hold the code they loaded at boot. Ask for the restart that
-                // makes the update actually take effect.
-                if (data.restart_required && typeof window.showRestartPending === 'function') {
-                    window.showRestartPending(
-                        'Update installed \u2014 restart the display to run the new code');
-                }
             }
             if (typeof showNotification === 'function') {
                 showNotification(data.message || 'Update complete', data.status || 'success');