From f7492e573a0368cd54a8bba3a2ccc36b2a9ab026 Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Fri, 21 Aug 2026 10:30:56 -0400 Subject: [PATCH] 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) Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW --- src/plugin_system/plugin_manager.py | 24 +++++++++++++++------ test/test_plugin_discovery_reports_skips.py | 17 +++++++++++++++ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/plugin_system/plugin_manager.py b/src/plugin_system/plugin_manager.py index 8500465a..e0cfb8f3 100644 --- a/src/plugin_system/plugin_manager.py +++ b/src/plugin_system/plugin_manager.py @@ -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 @@ -196,15 +199,22 @@ class PluginManager: manifest_path = item / "manifest.json" 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. - self.logger.warning( - "Skipping %s: no manifest.json, so it cannot be loaded " - "as a plugin", item.name) + 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: @@ -217,9 +227,11 @@ class PluginManager: if not plugin_id: # Parsed but unusable. This was the quietest path of all: # the manifest is read successfully and then dropped. - self.logger.warning( - "Skipping %s: its manifest.json has no \"id\", so there " - "is nothing to register it under", item.name) + 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) diff --git a/test/test_plugin_discovery_reports_skips.py b/test/test_plugin_discovery_reports_skips.py index 080c780f..4002aea5 100644 --- a/test/test_plugin_discovery_reports_skips.py +++ b/test/test_plugin_discovery_reports_skips.py @@ -28,6 +28,7 @@ def _manager(tmp_path): pm.plugin_manifests = {} pm.plugin_directories = {} pm._discovery_lock = __import__("threading").RLock() + pm._skip_reported = set() pm.schema_manager = MagicMock() return pm @@ -62,3 +63,19 @@ def test_a_good_plugin_still_registers(tmp_path, caplog): pm = _manager(tmp_path) pm._scan_directory_for_plugins(tmp_path) assert "real-plugin" in pm.plugin_manifests, "a valid plugin was not registered" + + +def test_the_warning_does_not_repeat_on_every_scan(tmp_path, caplog): + """Discovery runs on every web UI page load and every config reconcile. + + 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. + """ + (tmp_path / "not-a-plugin").mkdir() + pm = _manager(tmp_path) + with caplog.at_level(logging.WARNING, logger="test.discovery"): + for _ in range(5): + pm._scan_directory_for_plugins(tmp_path) + hits = [r for r in caplog.records if "not-a-plugin" in r.message] + assert len(hits) == 1, f"warned {len(hits)} times across 5 scans"