rick/LEDMatrix
1
0
Fork 0
mirror of https://github.com/ChuckBuilds/LEDMatrix.git synced 2026-08-23 19:38:13 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(plugins): say when discovery skips a directory (#489)

Browse Source 
* fix(plugins): say when discovery skips a directory

A plugin can be enabled in config, enabled in plugin state, present on disk
with a valid manifest and an importable entry point -- and simply absent from
the running process, with nothing anywhere to say why.

That is not hypothetical. hockey-scoreboard on a live rig is enabled in both
places, imports cleanly when loaded by hand, and is listed in the Vegas plugin
order, but is not among the 22 plugins the process actually holds. Establishing
even that much meant comparing cache-file mtimes to find it had last run three
days earlier. The journal had nothing, because discovery does not report what
it declines to load.

Two paths were silent. A directory with no manifest.json was skipped without
comment, which is defensible until it is the thing you are trying to explain.
Quieter still, a manifest that parsed but carried no "id" was read
successfully and then dropped on the floor -- no warning, no trace, and the
plugin simply does not exist as far as the rest of the system is concerned.

Both now log a warning naming the directory and the reason.

This does not explain the rig above; its manifest has an id. It makes the next
occurrence diagnosable from the journal instead of from file timestamps.

Reverting the change fails both tests. 65 plugin-system tests pass.

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

* fix(plugins): warn once per directory, not once per scan

Self-review catch. Discovery runs on every web UI page load and every config
reconcile, so warning unconditionally about an unloadable directory would put
a line in the journal each time someone opened a page -- the same log-volume
problem this change exists to help diagnose.

The skip is now reported once per directory per process. The diagnostic value
is unchanged: the reason a plugin is missing still appears in the journal,
once, where before it appeared nowhere.

Test added covering five consecutive scans producing one warning.

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

* fix(plugins): one unusable manifest no longer aborts the whole scan

json.load accepts any JSON value, so a manifest.json holding null, [],
"text" or 42 parses without complaint and then raises AttributeError on
manifest.get('id'). Nothing catches that: the outer handler around the
scan takes OSError and PermissionError only.

So a single malformed manifest did not skip that one directory -- it
aborted _scan_directory_for_plugins outright, and every other plugin on
disk, however healthy, silently failed to register. Reproduced with
three directories, the middle one holding `null`:

    SCAN ABORTED -> AttributeError: 'NoneType' object has no attribute 'get'
      the two valid plugins never registered

That is the same failure this PR set out to fix, in its most severe
form: a plugin enabled in config, enabled in plugin state, present on
disk, and absent from the running process with nothing to say why --
except here it takes every other plugin with it.

A manifest that is not a JSON object is now skipped like any other
unusable directory, named once, with what it actually was:

    Skipping bad-null: its manifest.json is NoneType, not a JSON object
    Skipping bad-list: its manifest.json is list, not a JSON object
    scan returned: ['aaa-good', 'zzz-good']

Verified: removing the guard fails 6 of the 10 tests. Covers null, list,
string, int and bool, and asserts the healthy plugins either side of the
bad one still register.

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 (1M context) <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-08-21 16:19:33 -04:00
committed by GitHub
co-authored by Claude Opus 5
parent 863e4a1ecd
commit 1fbe244e49
2 changed files with 182 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/plugin_system/plugin_manager.py
+56 -12
View File
@@ -76,6 +76,9 @@ class PluginManager:
# Lock protecting plugin_manifests and plugin_directories from
# concurrent mutation (background reconciliation) and reads (requests).
self._discovery_lock = threading.RLock()
#: Directories already reported as unloadable, so the warning is
#: emitted once rather than on every discovery scan.
self._skip_reported: set = set()
# Lock protecting plugin_last_update from concurrent mutation/iteration.
# It's written from run_scheduled_updates()/update_all_plugins() (main
@@ -195,18 +198,59 @@ class PluginManager:
continue
manifest_path = item / "manifest.json"
if manifest_path.exists():
try:
with open(manifest_path, 'r', encoding='utf-8') as f:
manifest = json.load(f)
plugin_id = manifest.get('id')
if plugin_id:
plugin_ids.append(plugin_id)
new_manifests[plugin_id] = manifest
new_directories[plugin_id] = item
except (json.JSONDecodeError, PermissionError, OSError) as e:
self.logger.warning("Error reading manifest from %s: %s", manifest_path, e, exc_info=True)
continue
if not manifest_path.exists():
# Once per directory per process. Discovery runs on every
# web UI page load and every config reconcile, so warning
# unconditionally would put a line in the journal each
# time someone opened a page -- the same log-volume
# problem this is meant to help diagnose.
# A directory here that carries no manifest is not a
# plugin. Said once, because the alternative is a plugin
# that is enabled in config, enabled in plugin state,
# present on disk, and simply absent from the running
# process with nothing anywhere to say why. Working that
# out afterwards means reading cache-file mtimes.
if item.name not in self._skip_reported:
self._skip_reported.add(item.name)
self.logger.warning(
"Skipping %s: no manifest.json, so it cannot be "
"loaded as a plugin", item.name)
continue
try:
with open(manifest_path, 'r', encoding='utf-8') as f:
manifest = json.load(f)
except (json.JSONDecodeError, PermissionError, OSError) as e:
self.logger.warning("Error reading manifest from %s: %s", manifest_path, e, exc_info=True)
continue
# json.load accepts any JSON value, so a manifest holding
# null, [] or "text" parses and then raises AttributeError on
# .get(). Nothing here catches that -- the outer handler takes
# OSError/PermissionError only -- so a single malformed
# manifest aborted the whole scan and every other plugin on
# disk, however healthy, silently failed to register.
if not isinstance(manifest, dict):
if item.name not in self._skip_reported:
self._skip_reported.add(item.name)
self.logger.warning(
"Skipping %s: its manifest.json is %s, not a JSON "
"object", item.name, type(manifest).__name__)
continue
plugin_id = manifest.get('id')
if not plugin_id:
# Parsed but unusable. This was the quietest path of all:
# the manifest is read successfully and then dropped.
if item.name not in self._skip_reported:
self._skip_reported.add(item.name)
self.logger.warning(
"Skipping %s: its manifest.json has no \"id\", so "
"there is nothing to register it under", item.name)
continue
plugin_ids.append(plugin_id)
new_manifests[plugin_id] = manifest
new_directories[plugin_id] = item
except (OSError, PermissionError) as e:
self.logger.error("Error scanning directory %s: %s", directory, e, exc_info=True)