fix(store): update_plugin silently did nothing for four installed plugins (#536)

install_plugin() deliberately renames a plugin's directory to the MANIFEST id
when it differs from the REGISTRY id, so registry `stocks` lands in
`ledmatrix-stocks/`. Every lookup in _find_plugin_path() is by directory name,
so update_plugin("stocks") found nothing, logged "Plugin not installed", and
returned False.

Nothing surfaced that to the user. Clicking update in the web UI was a no-op
with no error, and the plugin stayed on a stale version indefinitely. Four
installed plugins hit this on a real device -- leaderboard, music, stocks and
weather -- found because a scripted update of eleven plugins failed on exactly
those four.

Adds a manifest-id scan as the LAST step of the resolution chain, so the two
documented lookups above it (configured dir, then the sibling plugins/
fallback) keep their exact meaning and ordering. That ordering is pinned by
test_discovery_path_contract.py, which characterises the divergence between
the three resolvers on purpose; this extends the chain rather than reordering
it. Directories renamed aside with '.standalone-backup-' during an install or
rollback are skipped, since matching one would report a half-finished install
as a live plugin.

Also adds scripts/audit_render_path.py, which walks the call graph from
display() and reports blocking calls reachable from it. display() runs on the
render thread, so anything slow there stalls the panel; on a vsync-paced loop
a single 15ms call drops a frame and a network round trip freezes the marquee.
Two instances were already found the slow way, by reading frame-time
histograms -- odds-ticker reading the scoreboard cache per frame, and
soccer-scoreboard timing out inside update(). The audit finds that shape in
the source instead. It is a heuristic and says so: a hit behind an interval
check may be fine.

It currently flags 23 calls across six plugins. The clearest is
ledmatrix-music, whose display() falls back to an inline
requests.get(timeout=5) when album art has not been prefetched -- a deliberate
"show the art rather than go blank" tradeoff by its author, but up to five
seconds of frozen panel. Reported, not changed; that is its owner's call.

185 store tests pass. Three of the six new tests fail without the fix.

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

scripts/audit_render_path.py

@@ -0,0 +1,159 @@
+#!/usr/bin/env python3
+"""Find blocking work reachable from a plugin's render path.
+
+`display()` runs on the render thread. Anything slow reached from it stalls the
+panel for its whole duration, and on a vsync-paced loop that is immediately
+visible: a single 15ms call on a 100Hz panel drops a frame, and a network round
+trip freezes the marquee outright.
+
+This has bitten twice already. odds-ticker called `_has_live_games()` every
+frame, whose slow path read the scoreboard cache from disk and parsed JSON per
+enabled league -- one stalled frame every few minutes. soccer-scoreboard timed
+out inside `update()` during a cache refresh. Both were found by staring at
+frame-time histograms, which is a slow way to find a bug that is visible in the
+source.
+
+The audit walks the call graph from `display()` through same-class `self.*`
+methods and reports anything that reaches a known-blocking API. It is a
+heuristic, not a proof: it cannot see through indirection, and a hit is not
+automatically a bug -- a call guarded by an interval check may be fine. It is a
+list of places worth a human look.
+
+    python3 scripts/audit_render_path.py                    # all plugins
+    python3 scripts/audit_render_path.py --dir plugin-repos # a specific tree
+    python3 scripts/audit_render_path.py --plugin odds-ticker
+"""
+from __future__ import annotations
+
+import argparse
+import ast
+import sys
+from pathlib import Path
+
+#: Calls that can block for longer than a frame. Matched on the attribute or
+#: function name, so `requests.get`, `self.session.get` and a bare `get` on a
+#: requests-ish object all register.
+BLOCKING = {
+    "get": "network or cache read",
+    "post": "network",
+    "put": "network",
+    "request": "network",
+    "urlopen": "network",
+    "read": "I/O",
+    "open": "file I/O",
+    "load": "JSON/file parse",
+    "loads": "JSON parse",
+    "dump": "file write",
+    "dumps": "serialise",
+    "sleep": "sleep on the render thread",
+    "run": "subprocess",
+    "check_output": "subprocess",
+    "connect": "network",
+    "download_logo": "network",
+    "_fetch": "fetch",
+}
+
+#: Names that make a hit far more likely to matter.
+HIGH_SIGNAL = ("requests", "urllib", "session", "cache_manager", "subprocess",
+               "socket", "http")
+
+RENDER_ENTRY = "display"
+
+
+class Analyzer:
+    def __init__(self, tree: ast.AST):
+        self.methods: dict[str, ast.FunctionDef] = {}
+        for node in ast.walk(tree):
+            if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
+                self.methods.setdefault(node.name, node)
+
+    def calls_in(self, fn: ast.AST):
+        """(self-method names called, blocking hits) inside one function."""
+        self_calls, hits = set(), []
+        for node in ast.walk(fn):
+            if not isinstance(node, ast.Call):
+                continue
+            func = node.func
+            if isinstance(func, ast.Attribute):
+                name = func.attr
+                base = ast.unparse(func.value) if hasattr(ast, "unparse") else ""
+                if base == "self" and name in self.methods:
+                    self_calls.add(name)
+                    continue
+                if name in BLOCKING:
+                    hits.append((name, base, BLOCKING[name], node.lineno))
+            elif isinstance(func, ast.Name) and func.id in BLOCKING:
+                hits.append((func.id, "", BLOCKING[func.id], node.lineno))
+        return self_calls, hits
+
+    def reachable_from(self, entry: str, max_depth: int = 3):
+        """Blocking hits reachable from `entry`, with the path that reaches them."""
+        if entry not in self.methods:
+            return []
+        found, seen = [], set()
+        stack = [(entry, [entry], 0)]
+        while stack:
+            name, path, depth = stack.pop()
+            if name in seen or depth > max_depth:
+                continue
+            seen.add(name)
+            self_calls, hits = self.calls_in(self.methods[name])
+            for hit in hits:
+                found.append((path, hit))
+            for callee in sorted(self_calls):
+                stack.append((callee, path + [callee], depth + 1))
+        return found
+
+
+def audit_file(path: Path):
+    try:
+        tree = ast.parse(path.read_text(encoding="utf-8"))
+    except (OSError, SyntaxError):
+        return []
+    analyzer = Analyzer(tree)
+    return analyzer.reachable_from(RENDER_ENTRY)
+
+
+def main():
+    ap = argparse.ArgumentParser(description=__doc__,
+                                 formatter_class=argparse.RawDescriptionHelpFormatter)
+    root = Path(__file__).resolve().parent.parent
+    ap.add_argument("--dir", default=str(root / "plugin-repos"))
+    ap.add_argument("--plugin", help="only this plugin directory")
+    ap.add_argument("--all-hits", action="store_true",
+                    help="include low-signal hits (open/read/load on locals)")
+    args = ap.parse_args()
+
+    base = Path(args.dir)
+    if not base.is_dir():
+        sys.exit("not a directory: %s" % base)
+
+    plugins = [base / args.plugin] if args.plugin else sorted(
+        d for d in base.iterdir() if d.is_dir())
+
+    total = 0
+    for plugin in plugins:
+        rows = []
+        for src in sorted(plugin.glob("*.py")):
+            if src.name.startswith("test_"):
+                continue
+            for path, (name, s_base, why, lineno) in audit_file(src):
+                signal = any(h in s_base.lower() for h in HIGH_SIGNAL)
+                if not signal and not args.all_hits:
+                    continue
+                rows.append((src.name, lineno, "->".join(path),
+                             ("%s.%s" % (s_base, name)) if s_base else name, why))
+        if rows:
+            total += len(rows)
+            print("\n%s" % plugin.name)
+            for fname, lineno, chain, call, why in sorted(rows):
+                print("  %s:%-5d %-34s via %s" % (fname, lineno, call + "  (" + why + ")", chain))
+
+    print("\n%d blocking call(s) reachable from display() across %d plugin(s)"
+          % (total, len(plugins)))
+    print("Heuristic: a hit guarded by an interval check may be fine. Look, do "
+          "not assume.")
+
+
+if __name__ == "__main__":
+    main()

src/plugin_system/store_manager.py

@@ -1589,6 +1589,7 @@ class PluginStoreManager:
             with open(manifest_path, 'r') as f:
                 manifest = json.load(f)
             
+            requested_id = plugin_id
             plugin_id = plugin_id or manifest.get('id')
             if not plugin_id:
                 return {
@@ -1664,6 +1665,15 @@ class PluginStoreManager:
             
             branch_info = f" (branch: {branch_used})" if branch_used else ""
             self.logger.info(f"Successfully installed plugin from URL: {plugin_id}{branch_info}")
+            # User deliberately (re)installed this plugin -- clear any persistent
+            # uninstall record, exactly as install_plugin() does. Without this the
+            # id stays in config/uninstalled_plugins.json and
+            # purge_uninstalled_plugins(), which runs at every web-app startup,
+            # deletes the directory again: the plugin works for the rest of the
+            # session and is gone after the next reboot.
+            self.forget_uninstalled_plugin(
+                *(pid for pid in (requested_id, plugin_id, manifest.get('id')) if pid)
+            )
             result = {
                 'success': True,
                 'plugin_id': plugin_id,
@@ -2423,7 +2433,66 @@ class PluginStoreManager:
                     return plugin_path
         except (OSError, ValueError):
             pass
-        
+
+        # Last resort: the directory name may differ from the id being looked
+        # up. install_plugin() deliberately renames a plugin's directory to the
+        # MANIFEST id when it differs from the REGISTRY id (see the rename near
+        # "doesn't match registry ID" above), so `stocks` in the registry lands
+        # in `ledmatrix-stocks/`. Every lookup above is by directory name, so
+        # update_plugin("stocks") found nothing and reported the plugin as not
+        # installed -- silently, and for good: the user sees no error and stays
+        # on a stale version. Four installed plugins hit this in practice
+        # (leaderboard, music, stocks, weather).
+        #
+        # Deliberately last so the two lookups above keep their exact meaning;
+        # this only runs when a direct hit already failed. See
+        # test_discovery_path_contract.py, which pins that ordering.
+        for search_dir in self._candidate_plugin_dirs():
+            match = self._find_by_manifest_id(search_dir, plugin_id)
+            if match is not None:
+                self.logger.debug(
+                    "Resolved plugin '%s' to %s via its manifest id "
+                    "(directory name differs from the id)", plugin_id, match)
+                return match
+
+        return None
+
+    def _candidate_plugin_dirs(self) -> List[Path]:
+        """Directories that may hold installed plugins, configured one first."""
+        dirs = [self.plugins_dir]
+        try:
+            base = self.plugins_dir if self.plugins_dir.is_absolute()                 else self.plugins_dir.resolve()
+            sibling = base.parent / 'plugins'
+            if sibling != self.plugins_dir:
+                dirs.append(sibling)
+        except (OSError, ValueError):
+            pass
+        return [d for d in dirs if d.exists()]
+
+    @staticmethod
+    def _find_by_manifest_id(search_dir: Path, plugin_id: str) -> Optional[Path]:
+        """A subdirectory of `search_dir` whose manifest declares `plugin_id`.
+
+        Skips half-finished installs: store_manager renames a directory aside
+        with '.standalone-backup-' during install and rollback, and treating
+        one as installed would resurrect a ghost plugin.
+        """
+        try:
+            entries = sorted(search_dir.iterdir())
+        except (OSError, ValueError):
+            return None
+        for entry in entries:
+            if not entry.is_dir() or '.standalone-backup-' in entry.name:
+                continue
+            manifest = entry / 'manifest.json'
+            if not manifest.is_file():
+                continue
+            try:
+                with open(manifest, 'r', encoding='utf-8') as handle:
+                    if json.load(handle).get('id') == plugin_id:
+                        return entry
+            except (OSError, ValueError):
+                continue
         return None
     
     _SKIN_ID_PATTERN = re.compile(r'^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$')

test/test_discovery_path_contract.py

@@ -116,6 +116,77 @@ class TestFallbackDivergence:
             plugin_dir / "config_schema.json"
 
 
+class TestRegistryIdVersusDirectoryName:
+    """install_plugin() renames a plugin's directory to the MANIFEST id when it
+    differs from the REGISTRY id, so `stocks` in the registry lands in
+    `ledmatrix-stocks/`. Every path lookup is by directory name, so
+    update_plugin("stocks") used to find nothing and report the plugin as not
+    installed -- silently. Four installed plugins hit this in practice
+    (leaderboard, music, stocks, weather): the user clicks update, nothing
+    happens, no error, and they stay on a stale version indefinitely.
+    """
+
+    def test_resolves_a_plugin_whose_directory_name_differs_from_its_id(self, tmp_path):
+        configured = tmp_path / "plugin-repos"
+        configured.mkdir()
+        plugin_dir = _write_plugin(configured, "stocks", dir_name="ledmatrix-stocks")
+
+        store = PluginStoreManager(plugins_dir=str(configured))
+        assert store._find_plugin_path("stocks") == plugin_dir
+
+    def test_direct_directory_hit_still_wins(self, tmp_path):
+        """The manifest scan is a last resort. A directory named for the id
+        must still be preferred, so the two documented lookups above keep
+        their exact meaning."""
+        configured = tmp_path / "plugin-repos"
+        configured.mkdir()
+        direct = _write_plugin(configured, "demo", dir_name="demo")
+        # A second directory whose manifest claims the same id.
+        _write_plugin(configured, "demo", dir_name="zz-other-demo")
+
+        store = PluginStoreManager(plugins_dir=str(configured))
+        assert store._find_plugin_path("demo") == direct
+
+    def test_unknown_id_is_still_not_found(self, tmp_path):
+        configured = tmp_path / "plugin-repos"
+        configured.mkdir()
+        _write_plugin(configured, "stocks", dir_name="ledmatrix-stocks")
+
+        store = PluginStoreManager(plugins_dir=str(configured))
+        assert store._find_plugin_path("no-such-plugin") is None
+
+    def test_half_finished_installs_are_not_resurrected(self, tmp_path):
+        """A directory renamed aside during install/rollback carries a valid
+        manifest. Matching one would report a ghost plugin as installed."""
+        configured = tmp_path / "plugin-repos"
+        configured.mkdir()
+        _write_plugin(configured, "ghost",
+                      dir_name="ghost.standalone-backup-1234")
+
+        store = PluginStoreManager(plugins_dir=str(configured))
+        assert store._find_plugin_path("ghost") is None
+
+    def test_unreadable_manifest_does_not_break_the_scan(self, tmp_path):
+        configured = tmp_path / "plugin-repos"
+        configured.mkdir()
+        broken = configured / "broken-plugin"
+        broken.mkdir()
+        (broken / "manifest.json").write_text("{ not json")
+        wanted = _write_plugin(configured, "stocks", dir_name="ledmatrix-stocks")
+
+        store = PluginStoreManager(plugins_dir=str(configured))
+        assert store._find_plugin_path("stocks") == wanted
+
+    def test_sibling_plugins_dir_is_searched_too(self, tmp_path):
+        configured = tmp_path / "plugin-repos"
+        configured.mkdir()
+        legacy = _write_plugin(tmp_path / "plugins", "weather",
+                               dir_name="ledmatrix-weather")
+
+        store = PluginStoreManager(plugins_dir=str(configured))
+        assert store._find_plugin_path("weather") == legacy
+
+
 class TestStandaloneBackupContract:
     def test_discovery_skips_backup_dirs(self, tmp_path):
         plugins_dir = tmp_path / "plugins"