fix: keep low-memory boards reachable under load (#464)

* fix(service): survive corrupt health cache and clean exits

Three independent failure modes that each end with a dark panel and no
automatic recovery.

1. PluginHealthTracker._load_health_state returned the cached value
   verbatim. If that value is not a dict, every caller raises
   AttributeError: 'list' object has no attribute 'get' — during
   DisplayController.__init__, so the process dies before the display
   loop starts. systemd restarts it, the same bad entry is read back
   from disk, and it dies again: an unattended restart loop that
   survives reboots because the cause is persisted. Observed in the
   field with plugin_health:<id> holding an unrelated plugin's list
   payload. Now non-dict entries are discarded with a warning and the
   defaults are rebuilt.

2. ledmatrix.service used Restart=on-failure, so any exit with status 0
   left the unit stopped and the panel dark indefinitely — systemd
   treats it as success and never brings it back. Restart=always.

3. ledmatrix-wifi-monitor.service used StandardOutput=syslog, which
   systemd has marked obsolete; it warns and rewrites it to journal on
   every load.

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

* perf(memory): size the cache to the board and stop reinstalling deps

On a 1GB Pi 3B+ the display process settles around 600MB RSS of 905MB
total. When the remaining headroom runs out the failure is not a clean
crash: fork() starts returning ENOMEM, so sshd accepts connections and
closes them before its banner, timer jobs stop running, and the panel
goes dark, while already-resident processes keep serving normally. The
board looks healthy from outside and cannot be logged into. Only a power
cycle clears it.

Three contributing causes:

- MemoryCache had a fixed 1000-entry ceiling. Entries are parsed API
  payloads of tens of KB, so one ceiling cannot serve both a 512MB Zero
  2 W and an 8GB Pi 5. Now scaled from MemTotal (150 entries at <=1GB,
  1500 at >=8GB), overridable with LEDMATRIX_CACHE_MAX_ENTRIES.

- requirements_are_satisfied() returned False for any requirement with
  extras, so a plugin depending on python-socketio[client] re-ran pip on
  every single start: ~8s, a network dependency, and a 100-200MB spike
  at the least convenient moment. During a restart loop it repeats for
  each restart. Extras are now resolved one level deep against installed
  metadata, keeping the conservative "anything unverifiable falls
  through to pip" contract.

- ledmatrix.service had no memory ceiling. MemoryMax=85% expressed as a
  percentage so one unit file suits every board. Note this needs the
  memory cgroup controller, which Pi firmware disables by default;
  first_time_install.sh now adds cgroup_enable=memory to cmdline.txt,
  and the unit file documents how to verify it took effect.

first_time_install.sh also enables persistent journald storage (capped
at 64M). Default storage is volatile, so every reboot destroys the logs
that would explain why the board rebooted.

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

* docs: guidance for 512MB and 1GB boards

Documents the memory ceiling on small boards and, more usefully, what
running into it actually looks like: sshd accepting connections and
closing them before the banner, the web UI still responding normally,
clean ping, a dark panel, and a wrong clock after the next boot. None of
those read as "out of memory", which makes the failure hard to identify
from the symptoms.

Cross-referenced from SSH_UNAVAILABLE_AFTER_INSTALL.md, since "I can't
SSH in any more" is how most people will first meet this.

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

* fix: address review findings on the low-memory work

Nine CodeRabbit findings, five in code.

**Health state (the one that matters).** The non-dict guard did not cover a
dict missing fields the callers index directly, which is the shape actually
seen in the wild: a record carrying only circuit_state produced
`plugin clock-simple operation failed: 'circuit_state'` about fifty times a
minute with the panel frozen. The record is now completed against the
defaults per field rather than trusted or discarded wholesale. Per field
matters: a first pass rejected any incomplete record outright, which reset a
tripped breaker and real failure counts to healthy because one optional
field was absent -- an existing test caught it. Values of the wrong type
(a counter persisted as a string, an unknown circuit_state) fall back
individually, valid neighbours survive, and newer fields the schema has
grown since (degraded, degraded_reason) are carried through untouched.

**Cache ceiling.** MemoryCache.set() accepted entries without bound between
cleanup sweeps, which run every 300s by default, so a burst could take the
cache far past max_size -- the unbounded growth the limit exists to stop.
Eviction now runs under the same lock on every write, sharing one helper
with the periodic sweep so the two cannot drift.

**Installer, cgroups.** Only cgroup_enable=memory was checked, so a board
carrying that without cgroup_memory=1 reported success and got no change,
leaving MemoryMax= inert. Each parameter is now checked and appended
independently; verified against all four combinations, single line preserved.

**Installer, journald.** Persistence was inferred from /var/log/journal being
non-empty, which proves neither Storage=persistent nor a size cap -- the
directory survives a switch back to volatile. The effective configuration is
read instead (systemd-analyze cat-config, falling back to the conf files),
and an explicitly configured SystemMaxUse is preserved rather than
overwritten. Verified across volatile, persistent-without-cap,
persistent-with-user-cap, cap-without-storage, and commented-only configs.

**Dependency extras.** _extras_are_satisfied stopped at one level, so a
gated dependency that itself requests an extra (requests[socks]) passed on
the base distribution's version while the extra's own dependency was
missing, and pip was skipped. It now recurses, with a visited
(distribution, extras) set so a cycle terminates.

Docs: both kernel command-line paths documented (the installer falls back to
/boot/cmdline.txt), daemon-reload and restart added after the systemd
override example, memory exhaustion added to the SSH summary with its
power-cycle-only recovery, and a language on the fenced block for MD040.

Tests: five for the health-state repair including the exact wild shape and
that record_failure/record_success no longer raise against it, and one for
the cache ceiling. Both mutation-checked. Full suite 2927 passed, with the
one pre-existing tmpfs failure that also fails on main.

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

* fix: harden the health-state repair and confirm journald took effect

Second review round; all three findings were valid and two were bugs in the
repair added last commit.

The repair could raise out of itself. An unhashable circuit_state (a list or
dict on disk) hit `value in {...}` and raised TypeError -- from the code
whose whole job is to stop a malformed record crashing the caller. It now
requires a str before the membership test.

bool is a subclass of int, so True passed the timestamp check and then
compared as 1.0: enough to expire a cooldown the instant the breaker opened,
while False would stop the elapsed check firing at all. Timestamps now
exclude bool explicitly.

The regression test for the original crash was seeded with a record that
*contained* circuit_state, so it passed against the old raw-return behaviour
too -- the counters are read with .get(), so circuit_state is the only field
whose absence used to raise. Reseeded to omit it, and it now fails against
raw-return as intended.

journald: drop-ins apply in lexical order, so a local file sorting after
ledmatrix-persistent.conf still wins and writing ours proves nothing. The
effective Storage is re-read afterwards and a warning naming the diagnostic
command is printed if persistence is still not active, rather than reporting
a success that was not verified.

Full suite 2934 passed, same single pre-existing tmpfs failure.

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

src/cache/memory_cache.py

@@ -4,11 +4,58 @@ Memory Cache
 Handles in-memory caching with TTL support, size limits, and automatic cleanup.
 """
 
+import os
 import time
 import threading
 import logging
 from typing import Dict, Any, Optional
 
+# Historical fixed ceiling, kept as the fallback when RAM cannot be read.
+DEFAULT_MAX_SIZE = 1000
+
+
+def _total_memory_mb() -> Optional[float]:
+    """Physical RAM in MB, or None where /proc/meminfo is unavailable."""
+    try:
+        with open('/proc/meminfo', 'r', encoding='utf-8') as fh:
+            for line in fh:
+                if line.startswith('MemTotal:'):
+                    return int(line.split()[1]) / 1024
+    except (OSError, ValueError, IndexError):
+        return None
+    return None
+
+
+def default_max_size() -> int:
+    """Entry ceiling scaled to this machine's RAM.
+
+    One fixed ceiling cannot serve both a 512 MB Pi Zero 2 W and an 8 GB Pi 5.
+    Entries here are parsed API payloads that routinely run tens of kilobytes
+    each, so a thousand of them is a comfortable cache on a large board and a
+    substantial fraction of total RAM on a small one — where the process
+    competing for that RAM is also driving the panel. Set
+    LEDMATRIX_CACHE_MAX_ENTRIES to override.
+    """
+    override = os.environ.get('LEDMATRIX_CACHE_MAX_ENTRIES')
+    if override:
+        try:
+            value = int(override)
+            if value > 0:
+                return value
+        except ValueError:
+            pass
+
+    total_mb = _total_memory_mb()
+    if total_mb is None:
+        return DEFAULT_MAX_SIZE
+    if total_mb < 1536:      # 512 MB and 1 GB boards
+        return 150
+    if total_mb < 3072:      # 2 GB
+        return 400
+    if total_mb < 6144:      # 4 GB
+        return 800
+    return 1500              # 8 GB and up
+
 
 class MemoryCache:
     """Manages in-memory cache with TTL and size limits."""
@@ -87,6 +134,32 @@ class MemoryCache:
         with self._lock:
             self._cache[key] = value
             self._timestamps[key] = time.time()
+            # Enforce the ceiling here rather than leaving it to the periodic
+            # cleanup, which only runs every cleanup_interval seconds (300 by
+            # default). A burst of inserts between two sweeps could otherwise
+            # take the cache far past _max_size, which is the memory growth this
+            # limit exists to prevent -- and on a 1GB board that is the
+            # difference between a bounded cache and an unreachable Pi.
+            self._evict_over_limit_locked()
+
+    def _evict_over_limit_locked(self) -> int:
+        """Drop oldest entries until the cache is within _max_size.
+
+        Caller must hold self._lock. Returns the number of entries removed.
+        """
+        excess = len(self._cache) - self._max_size
+        if excess <= 0:
+            return 0
+        oldest = sorted(
+            self._timestamps.items(),
+            key=lambda item: float(item[1]) if isinstance(item[1], (int, float)) else 0.0
+        )
+        removed = 0
+        for key, _ in oldest[:excess]:
+            self._cache.pop(key, None)
+            self._timestamps.pop(key, None)
+            removed += 1
+        return removed
     
     def clear(self, key: Optional[str] = None) -> None:
         """
@@ -143,22 +216,8 @@ class MemoryCache:
                 self._timestamps.pop(key, None)
                 removed_count += 1
             
-            # Enforce size limit by removing oldest entries if cache is too large
-            if len(self._cache) > self._max_size:
-                # Sort by timestamp (oldest first)
-                sorted_entries = sorted(
-                    self._timestamps.items(),
-                    key=lambda x: float(x[1]) if isinstance(x[1], (int, float)) else 0
-                )
-                
-                # Remove oldest entries until we're under the limit
-                excess_count = len(self._cache) - self._max_size
-                for i in range(excess_count):
-                    if i < len(sorted_entries):
-                        key = sorted_entries[i][0]
-                        self._cache.pop(key, None)
-                        self._timestamps.pop(key, None)
-                        removed_count += 1
+            # Same ceiling enforcement set() uses, so the two cannot drift.
+            removed_count += self._evict_over_limit_locked()
             
             self._last_cleanup = current_time
             

src/cache_manager.py

@@ -33,7 +33,7 @@ import logging
 import threading
 import tempfile
 from src.exceptions import CacheError
-from src.cache.memory_cache import MemoryCache
+from src.cache.memory_cache import MemoryCache, default_max_size
 from src.cache.disk_cache import DiskCache
 from src.cache.cache_strategy import CacheStrategy
 from src.cache.cache_metrics import CacheMetrics
@@ -84,7 +84,9 @@ class CacheManager:
             self.logger.warning("ConfigManager not available, using default cache intervals")
         
         # Initialize cache components using composition
-        self._memory_cache_component = MemoryCache(max_size=1000, cleanup_interval=300.0)
+        self._memory_cache_component = MemoryCache(
+            max_size=default_max_size(), cleanup_interval=300.0
+        )
         self._disk_cache_component = DiskCache(cache_dir=self.cache_dir, logger=self.logger)
         self._strategy_component = CacheStrategy(config_manager=self.config_manager, logger=self.logger)
         self._metrics_component = CacheMetrics(logger=self.logger)

src/plugin_system/plugin_health.py

@@ -7,7 +7,7 @@ and circuit breaker state. Provides automatic recovery mechanisms.
 
 import time
 import logging
-from typing import Dict, Optional, Any
+from typing import Dict, Optional, Any, Tuple
 from enum import Enum
 
 
@@ -64,10 +64,48 @@ class PluginHealthTracker:
             cache_key, max_age=None, memory_ttl=0 if force_reload else None
         )
 
-        if cached:
-            return cached
-        
-        # Default state
+        if isinstance(cached, dict) and cached:
+            # Complete it rather than trusting it: a persisted record can be
+            # missing fields the callers index directly (a partial write, a
+            # restored backup, an older schema), and returning it verbatim makes
+            # record_success / record_failure raise KeyError, which takes the
+            # display down in a restart loop that survives reboots because the
+            # bad entry is on disk.
+            state, repaired = self._repair_health_state(cached)
+            if repaired:
+                self.logger.warning(
+                    f"Repaired health state for {plugin_id}: "
+                    f"{sorted(repaired)} missing or invalid, using defaults for those."
+                )
+            return state
+
+        # Not a dict at all: written by something other than
+        # _save_health_state (a key collision, a corrupted entry). Nothing to
+        # salvage.
+        if cached is not None and not isinstance(cached, dict):
+            self.logger.warning(
+                f"Discarding malformed health state for {plugin_id}: expected "
+                f"dict, got {type(cached).__name__}. Falling back to defaults."
+            )
+
+        return self._default_health_state()
+    
+    def _save_health_state(self, plugin_id: str, state: Dict[str, Any]) -> None:
+        """Save health state to cache."""
+        cache_key = self._get_health_key(plugin_id)
+        self.cache_manager.set(cache_key, state)  # Persist indefinitely
+        self._health_state[plugin_id] = state
+    
+    # The fields callers index directly (state['circuit_state'] and friends).
+    # A cached dict missing any of them raises KeyError deep in record_success /
+    # record_failure, so the value is completed before it is handed out.
+    _COUNTER_FIELDS = ('consecutive_failures', 'total_failures', 'total_successes')
+    _TIMESTAMP_FIELDS = ('last_success_time', 'last_failure_time',
+                         'circuit_opened_time', 'half_open_start_time')
+
+    @staticmethod
+    def _default_health_state() -> Dict[str, Any]:
+        """A fresh state with every field the callers expect."""
         return {
             'consecutive_failures': 0,
             'total_failures': 0,
@@ -77,15 +115,56 @@ class PluginHealthTracker:
             'circuit_state': CircuitState.CLOSED.value,
             'circuit_opened_time': None,
             'half_open_start_time': None,
-            'last_error': None
+            'last_error': None,
         }
-    
-    def _save_health_state(self, plugin_id: str, state: Dict[str, Any]) -> None:
-        """Save health state to cache."""
-        cache_key = self._get_health_key(plugin_id)
-        self.cache_manager.set(cache_key, state)  # Persist indefinitely
-        self._health_state[plugin_id] = state
-    
+
+    @classmethod
+    def _repair_health_state(cls, cached: Dict[str, Any]) -> Tuple[Dict[str, Any], list]:
+        """Return `cached` completed against the defaults, plus what was repaired.
+
+        Per-field rather than all-or-nothing: a record that has real failure
+        counts but is missing `last_error` should keep the counts, not be reset
+        to healthy. Only values that are absent or the wrong type fall back to
+        the default, so a partial or older-schema record survives with whatever
+        it does carry, while every field the callers index is guaranteed present
+        and of a usable type.
+        """
+        state = cls._default_health_state()
+        repaired = []
+        for field, default in state.items():
+            if field not in cached:
+                repaired.append(field)
+                continue
+            value = cached[field]
+            if field in cls._COUNTER_FIELDS:
+                ok = isinstance(value, int) and not isinstance(value, bool) and value >= 0
+            elif field in cls._TIMESTAMP_FIELDS:
+                # bool is a subclass of int, so True would pass as a timestamp
+                # and then compare as 1.0 -- expiring a cooldown the instant it
+                # opens, or (False) making the elapsed check never fire.
+                ok = value is None or (
+                    isinstance(value, (int, float)) and not isinstance(value, bool)
+                )
+            elif field == 'circuit_state':
+                # Membership first requires the value to be hashable: a list or
+                # dict here would raise TypeError out of the repair itself,
+                # which is the crash this whole path exists to prevent.
+                ok = isinstance(value, str) and value in {
+                    member.value for member in CircuitState
+                }
+            else:  # last_error
+                ok = value is None or isinstance(value, str)
+            if ok:
+                state[field] = value
+            else:
+                repaired.append(field)
+        # Anything the schema has since grown (degraded, degraded_reason) is
+        # read with .get() by its callers, so carry it through untouched.
+        for field, value in cached.items():
+            if field not in state:
+                state[field] = value
+        return state, repaired
+
     def get_health_state(self, plugin_id: str, force_reload: bool = False) -> Dict[str, Any]:
         """Get current health state for a plugin.
 

src/plugin_system/plugin_loader.py

@@ -14,7 +14,7 @@ import sys
 import subprocess
 import threading
 from pathlib import Path
-from typing import Dict, Any, Optional, Tuple, Type
+from typing import Dict, Any, List, Optional, Tuple, Type
 import logging
 
 from packaging.requirements import InvalidRequirement, Requirement
@@ -45,6 +45,76 @@ def requirements_has_real_deps(requirements_file: str) -> bool:
     return False
 
 
+def _extra_dependencies(dist_name: str, extras) -> Optional[List[Requirement]]:
+    """Dependencies a distribution declares *only* behind the given extras.
+
+    Returns None when the installed metadata cannot be read or parsed, so the
+    caller can fall back to running pip rather than assuming anything.
+    """
+    try:
+        meta = importlib.metadata.metadata(dist_name)
+    except importlib.metadata.PackageNotFoundError:
+        return None
+
+    gated: List[Requirement] = []
+    for raw in meta.get_all('Requires-Dist') or []:
+        try:
+            dep = Requirement(raw)
+        except InvalidRequirement:
+            return None
+        if dep.marker is None:
+            continue
+        # Keep only what the distribution gates behind an extra we asked for:
+        # satisfied when `extra` is that name, but not when no extra is
+        # requested. A marker that holds either way (python_version, sys_platform)
+        # belongs to the base install and is already covered by the version check.
+        if dep.marker.evaluate({'extra': ''}):
+            continue
+        if any(dep.marker.evaluate({'extra': extra}) for extra in extras):
+            gated.append(dep)
+    return gated
+
+
+def _extras_are_satisfied(req: Requirement, _visited: Optional[set] = None) -> bool:
+    """Check the dependencies pulled in by req's extras are installed.
+
+    Follows extras through nested extras. A gated dependency can itself request
+    one (`requests[socks]`), and checking only that `requests` is installed at
+    an acceptable version says nothing about whether the socks extra's own
+    dependency is there -- so the caller would skip pip and the plugin would
+    fail at import instead. Plain dependencies are still checked one level
+    deep, which is all that is needed to tell "the extra was installed" from
+    "the extra was never installed".
+
+    `_visited` carries the (distribution, extras) pairs already seen, so a
+    dependency cycle between extras terminates instead of recursing forever.
+    Anything unreadable returns False, so the caller still falls through to pip.
+    """
+    if _visited is None:
+        _visited = set()
+    marker = (req.name.lower(), frozenset(e.lower() for e in req.extras))
+    if marker in _visited:
+        # Already accounted for higher up the chain; treating a cycle as
+        # satisfied here is safe because the outer frame still has to pass.
+        return True
+    _visited.add(marker)
+
+    gated = _extra_dependencies(req.name, req.extras)
+    if gated is None:
+        return False
+
+    for dep in gated:
+        try:
+            dep_version = importlib.metadata.version(dep.name)
+        except importlib.metadata.PackageNotFoundError:
+            return False
+        if dep.specifier and not dep.specifier.contains(dep_version, prereleases=True):
+            return False
+        if dep.extras and not _extras_are_satisfied(dep, _visited):
+            return False
+    return True
+
+
 def requirements_are_satisfied(requirements_file: str) -> bool:
     """
     Check whether every real requirement line in requirements.txt is already
@@ -76,9 +146,6 @@ def requirements_are_satisfied(requirements_file: str) -> bool:
         except InvalidRequirement:
             return False
 
-        if req.extras:
-            return False  # verifying extras' sub-dependencies isn't worth it here
-
         if req.marker is not None and not req.marker.evaluate():
             continue  # not applicable on this platform/interpreter
 
@@ -90,6 +157,9 @@ def requirements_are_satisfied(requirements_file: str) -> bool:
         if req.specifier and not req.specifier.contains(installed_version, prereleases=True):
             return False
 
+        if req.extras and not _extras_are_satisfied(req):
+            return False
+
     return True