fix(config): use the shared core-key list in the last three private copies

StartupValidator warned "Plugin 'auto_update' is enabled but not found" on
every display start with auto-update or a dim schedule on; the reserved
plugin-id check missed auto_update, sync, location and the rest; and
ConfigManager's (uncalled) orphan cleanup would have deleted display,
schedule and auto_update. All three now read src/core_config_keys.py, which
also gains CORE_SECRETS_KEYS for the github/youtube secrets sections.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-09-17 08:54:01 -04:00
co-authored by Claude Opus 5
parent 7ae614aa35
commit a0e1ce6c6c
5 changed files with 136 additions and 11 deletions
+7 -5
View File
@@ -29,6 +29,7 @@ import os
import logging
from pathlib import Path
from typing import Dict, Any, Optional, List
from src.core_config_keys import CORE_CONFIG_KEYS, CORE_SECRETS_KEYS
from src.exceptions import ConfigError
from src.logging_config import get_logger
from src.config_manager_atomic import (
@@ -756,12 +757,13 @@ class ConfigManager:
valid_set = set(valid_plugin_ids)
# Find orphaned plugins in main config
main_plugins = set(main_config.keys())
# Find orphaned plugins in main config. Core sections (display,
# schedule, auto_update, ...) are not plugins and never orphans.
main_plugins = set(main_config.keys()) - CORE_CONFIG_KEYS
orphaned_main = main_plugins - valid_set
# Find orphaned plugins in secrets config
secrets_plugins = set(secrets_config.keys())
secrets_plugins = set(secrets_config.keys()) - CORE_CONFIG_KEYS - CORE_SECRETS_KEYS
orphaned_secrets = secrets_plugins - valid_set
all_orphaned = orphaned_main | orphaned_secrets
@@ -815,8 +817,8 @@ class ConfigManager:
if not isinstance(plugin_config, dict):
continue
# Skip non-plugin config sections
if plugin_id in ['display', 'schedule', 'timezone', 'plugin_system']:
# Skip core config sections
if plugin_id in CORE_CONFIG_KEYS:
continue
schema = plugin_schema_manager.load_schema(plugin_id, use_cache=True)
+10
View File
@@ -41,3 +41,13 @@ CORE_CONFIG_KEYS = frozenset({
'vegas_excluded_plugins',
'vegas_scroll_enabled',
})
#: Top-level keys of ``config_secrets.json`` that belong to the core rather than
#: to a plugin: the GitHub token the Plugin Store reads, and the historical
#: ``youtube`` section. Plugin secrets are namespaced by plugin id, so anything
#: deciding whether a secrets section is a plugin's needs this as well as
#: ``CORE_CONFIG_KEYS``.
CORE_SECRETS_KEYS = frozenset({
'github',
'youtube',
})
+7 -4
View File
@@ -13,6 +13,8 @@ from typing import Any, Dict, List, Optional, Tuple
import jsonschema
from jsonschema import Draft7Validator, ValidationError
from src.core_config_keys import CORE_CONFIG_KEYS
def _renders_as_object(prop: Dict[str, Any]) -> bool:
"""``field_type == 'object'`` as ``plugin_config.html`` computes it.
@@ -698,10 +700,11 @@ class SchemaManager:
"""
collisions = []
# Reserved top-level config keys that plugins should not use as IDs
reserved_keys = {
'display', 'schedule', 'timezone', 'plugin_system',
'display_modes', 'system', 'hardware', 'debug',
# Reserved top-level config keys that plugins should not use as IDs:
# every core section (src/core_config_keys.py), plus a few names that
# read as core even though no current section uses them.
reserved_keys = set(CORE_CONFIG_KEYS) | {
'display_modes', 'hardware', 'debug',
'log_level', 'emulator', 'web_interface'
}
+4 -2
View File
@@ -8,6 +8,7 @@ Fails fast with clear error messages to prevent runtime issues.
import os
from typing import Any, List, Optional, Tuple
from pathlib import Path
from src.core_config_keys import CORE_CONFIG_KEYS
from src.exceptions import ConfigError, PluginError, CacheError
from src.logging_config import get_logger
@@ -275,8 +276,9 @@ class StartupValidator:
# Check for enabled plugins that don't exist
for plugin_id, plugin_config in config.items():
# Skip non-plugin config sections
if plugin_id in ['display', 'schedule', 'timezone', 'plugin_system']:
# Skip core sections: auto_update and dim_schedule have an
# 'enabled' key too, and are not plugins that went missing.
if plugin_id in CORE_CONFIG_KEYS:
continue
if not isinstance(plugin_config, dict):
+108
View File
@@ -0,0 +1,108 @@
"""Every "is this top-level key a plugin?" decision uses src/core_config_keys.py.
#589 moved plugin-state reconciliation onto the shared list, but three private
copies stayed behind and did not know about auto_update, dim_schedule, sync,
location, target_fps, ...:
- StartupValidator warned "Plugin 'auto_update' is enabled but not found in
plugins directory" on every display start with auto-update or a dim
schedule switched on;
- SchemaManager.detect_config_key_collisions let a plugin id collide with
those sections silently;
- ConfigManager.cleanup_orphaned_plugin_configs deleted display, schedule,
auto_update, ... as orphans, and validate_all_plugin_configs validated core
sections as plugins.
"""
import json
import re
from pathlib import Path
from unittest.mock import MagicMock
import pytest
from src.config_manager import ConfigManager
from src.core_config_keys import CORE_CONFIG_KEYS, CORE_SECRETS_KEYS
from src.plugin_system.schema_manager import SchemaManager
from src.startup_validator import StartupValidator
REPO = Path(__file__).resolve().parent.parent
class TestStartupValidator:
def test_enabled_core_sections_are_not_missing_plugins(self):
config = {
'display': {'hardware': {}},
'auto_update': {'enabled': True},
'dim_schedule': {'enabled': True},
'schedule': {'enabled': True},
'gone-plugin': {'enabled': True},
}
cm = MagicMock()
cm.get_config.return_value = config
pm = MagicMock()
pm.discover_plugins.return_value = []
validator = StartupValidator(cm, plugin_manager=pm, cache_manager=MagicMock())
validator.warnings = []
validator.errors = []
validator._validate_plugins()
missing = [w for w in validator.warnings if 'not found in plugins directory' in w]
assert missing == ["Plugin 'gone-plugin' is enabled but not found in plugins directory"]
class TestReservedPluginIds:
@pytest.mark.parametrize('key', sorted(CORE_CONFIG_KEYS))
def test_every_core_key_is_reserved(self, key):
collisions = SchemaManager().detect_config_key_collisions([key])
assert [c['type'] for c in collisions] == ['reserved_key_collision']
def test_an_ordinary_plugin_id_is_not(self):
assert SchemaManager().detect_config_key_collisions(['clock-simple']) == []
class TestConfigManagerHelpers:
@pytest.fixture
def manager(self, tmp_path):
config = {key: {'enabled': True} for key in ('display', 'schedule', 'auto_update',
'dim_schedule', 'sync', 'location')}
config.update({'timezone': 'UTC', 'installed': {'enabled': True},
'orphan': {'enabled': True}})
secrets = {'github': {'token': 't'}, 'youtube': {'api_key': 'k'},
'installed': {'api_key': 'a'}, 'orphan': {'api_key': 'o'}}
(tmp_path / 'config.json').write_text(json.dumps(config))
(tmp_path / 'secrets.json').write_text(json.dumps(secrets))
manager = ConfigManager(config_path=str(tmp_path / 'config.json'),
secrets_path=str(tmp_path / 'secrets.json'))
manager.template_path = str(tmp_path / 'no-template.json')
return manager
def test_cleanup_removes_only_real_orphans(self, manager, tmp_path):
removed = manager.cleanup_orphaned_plugin_configs(['installed'])
assert removed == ['orphan']
config = json.loads((tmp_path / 'config.json').read_text())
assert {'display', 'schedule', 'auto_update', 'dim_schedule', 'sync',
'location', 'timezone', 'installed'} == set(config)
secrets = json.loads((tmp_path / 'secrets.json').read_text())
assert set(secrets) == {'github', 'youtube', 'installed'}
def test_validate_all_skips_core_sections(self, manager):
schema_manager = MagicMock()
schema_manager.load_schema.return_value = None
results = manager.validate_all_plugin_configs(schema_manager)
assert set(results) == {'installed', 'orphan'}
def test_core_secrets_keys_are_not_core_config_keys():
assert not (CORE_SECRETS_KEYS & CORE_CONFIG_KEYS)
@pytest.mark.parametrize('relpath', [
'src/startup_validator.py',
'src/config_manager.py',
'src/plugin_system/schema_manager.py',
])
def test_no_private_copy_of_the_list_remains(relpath):
"""The old copies all started with this literal; a new copy likely would too."""
source = (REPO / relpath).read_text(encoding='utf-8')
assert not re.search(r"""\[\s*['"]display['"]\s*,\s*['"]schedule['"]""", source), \
f'{relpath} has a private core-key list again; use src/core_config_keys.py'