Compare commits

..
Author SHA1 Message Date
ChuckBuildsandClaude Opus 5 f0efdafbdb fix(web): stop dumping the config and request headers to the journal
save_main_config logged its entire POST body and the full request headers at
ERROR on every save. The body is the configuration itself, and the headers
carry the session cookie, so a routine settings change wrote both to the
journal -- at a level that guarantees they survive any sane log filter.

The lines are leftover debug output: they say "DEBUG:" in the message while
calling logging.error, and they went through the root logger rather than the
module logger, bypassing the level configured for this blueprint.

Replaced with a debug-level line recording the shape of the request, which is
the part with diagnostic value. The local `import logging` went with them; it
shadowed a module-level import that was already there.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
2026-08-20 05:55:45 -04:00
3 changed files with 18 additions and 127 deletions
+12 -42
View File
@@ -76,9 +76,6 @@ 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
@@ -198,45 +195,18 @@ class PluginManager:
continue
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.
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
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
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
except (OSError, PermissionError) as e:
self.logger.error("Error scanning directory %s: %s", directory, e, exc_info=True)
@@ -1,81 +0,0 @@
#!/usr/bin/env python3
"""Discovery must say when it skips a directory.
A plugin can be enabled in config, enabled in plugin state, present on disk
with a valid entry point -- and simply absent from the running process, with
nothing in the journal to say why. Working that out afterwards meant comparing
cache-file mtimes to find when it had last run.
Two paths were silent. A directory with no manifest.json was ignored, and --
quieter still -- a manifest that parsed but carried no "id" was read
successfully and then dropped on the floor.
"""
import json
import logging
import sys
from pathlib import Path
from unittest.mock import MagicMock
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
from src.plugin_system.plugin_manager import PluginManager # noqa: E402
def _manager(tmp_path):
pm = PluginManager.__new__(PluginManager)
pm.plugins_dir = tmp_path
pm.logger = logging.getLogger("test.discovery")
pm.plugin_manifests = {}
pm.plugin_directories = {}
pm._discovery_lock = __import__("threading").RLock()
pm._skip_reported = set()
pm.schema_manager = MagicMock()
return pm
def test_a_directory_without_a_manifest_is_reported(tmp_path, caplog):
(tmp_path / "not-a-plugin").mkdir()
pm = _manager(tmp_path)
with caplog.at_level(logging.WARNING, logger="test.discovery"):
pm._scan_directory_for_plugins(tmp_path)
joined = " ".join(r.message for r in caplog.records)
assert "not-a-plugin" in joined and "manifest" in joined, (
f"skip was silent; log said: {joined!r}")
def test_a_manifest_without_an_id_is_reported(tmp_path, caplog):
d = tmp_path / "idless"
d.mkdir()
(d / "manifest.json").write_text(json.dumps({"name": "No Id", "version": "1.0.0"}))
pm = _manager(tmp_path)
with caplog.at_level(logging.WARNING, logger="test.discovery"):
pm._scan_directory_for_plugins(tmp_path)
joined = " ".join(r.message for r in caplog.records)
assert "idless" in joined and "id" in joined, (
f"a parsed-but-unusable manifest vanished silently; log said: {joined!r}")
def test_a_good_plugin_still_registers(tmp_path, caplog):
d = tmp_path / "real-plugin"
d.mkdir()
(d / "manifest.json").write_text(json.dumps(
{"id": "real-plugin", "name": "Real", "version": "1.0.0"}))
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"
+6 -4
View File
@@ -715,10 +715,12 @@ def save_main_config():
if not data:
return jsonify({'status': 'error', 'message': 'No data provided'}), 400
import logging
logging.error(f"DEBUG: save_main_config received data: {data}")
logging.error(f"DEBUG: Content-Type header: {request.content_type}")
logging.error(f"DEBUG: Headers: {dict(request.headers)}")
# What arrives here is the config itself, and the headers carry the
# session cookie -- neither belongs in the journal, least of all at
# ERROR on every save. The shape of the request is the part with
# diagnostic value, so log that, at the level it deserves.
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()