diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 76492edd..0000c47f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -79,4 +79,5 @@ jobs: test/test_sports_scroll.py \ test/test_version_consistency.py \ test/test_plugin_compatibility_gate.py \ - test/test_install_preserves_existing.py + test/test_install_preserves_existing.py \ + test/test_core_owned_config_keys.py diff --git a/src/plugin_system/plugin_manager.py b/src/plugin_system/plugin_manager.py index 4a571658..50d6419e 100644 --- a/src/plugin_system/plugin_manager.py +++ b/src/plugin_system/plugin_manager.py @@ -395,6 +395,37 @@ class PluginManager: self.state_manager.set_state(plugin_id, PluginState.ERROR, error=e) return False + #: Config keys the **core** reads out of a plugin's own config block. The + #: plugin never declares them, so a schema with + #: ``"additionalProperties": false`` — 37 of the 42 published ones — reports + #: them as violations and the plugin gets flagged degraded in the web UI for + #: using a documented core feature. + #: + #: Listed explicitly rather than matched on a ``vegas_`` prefix, because + #: ``vegas_mode`` is the opposite case: plugins *do* declare that one, and a + #: prefix rule would silently stop validating it. + #: + #: Read by: ``vegas_mode/plugin_adapter.py`` (``vegas_width_pct``, + #: ``vegas_overflow``) and ``base_plugin.py`` (``vegas_max_width_screens``). + CORE_OWNED_CONFIG_KEYS = frozenset({ + 'vegas_width_pct', + 'vegas_overflow', + 'vegas_max_width_screens', + }) + + def _strip_core_owned_keys(self, config: Dict[str, Any]) -> Dict[str, Any]: + """A shallow copy of ``config`` without the core's own tuning keys. + + Only the top level is touched, and only when such a key is present, so + the common case allocates nothing extra. + """ + if not isinstance(config, dict): + return config + if not self.CORE_OWNED_CONFIG_KEYS.intersection(config): + return config + return {k: v for k, v in config.items() + if k not in self.CORE_OWNED_CONFIG_KEYS} + def _validate_config_schema_soft(self, plugin_id: str, config: Dict[str, Any]) -> None: """Validate a plugin's config against its JSON schema — warn/degrade only. @@ -419,7 +450,7 @@ class PluginManager: try: is_valid, errors = self.schema_manager.validate_config_against_schema( - config, schema, plugin_id + self._strip_core_owned_keys(config), schema, plugin_id ) except Exception as e: # pragma: no cover - defensive # Validation machinery itself failed — do not penalise the plugin. diff --git a/test/test_core_owned_config_keys.py b/test/test_core_owned_config_keys.py new file mode 100644 index 00000000..a134fdc8 --- /dev/null +++ b/test/test_core_owned_config_keys.py @@ -0,0 +1,98 @@ +"""The core's own tuning keys must not make a plugin look broken. + +`vegas_width_pct`, `vegas_overflow` and `vegas_max_width_screens` are read by +the *core* out of each plugin's config block — `vegas_mode/plugin_adapter.py` +and `base_plugin.py`. No plugin declares them, and 37 of the 42 published +config schemas set `"additionalProperties": false`, so schema validation +reported them as violations. + +That is not just log noise: `_validate_config_schema_soft` sets `degraded` in +the health tracker, which the web UI surfaces. Measured on a real device, **9 +of 27 installed plugins** were flagged degraded purely for using a documented +core feature — including `baseball-scoreboard` and `f1-scoreboard`. + +The fix strips those keys before validating. It deliberately does *not* match +on a `vegas_` prefix: `vegas_mode` is plugin-owned and declared in schemas, and +a prefix rule would silently stop validating it. +""" + +from unittest.mock import MagicMock + +import pytest + +from src.plugin_system.plugin_manager import PluginManager + + +STRICT_SCHEMA = { + "type": "object", + "additionalProperties": False, + "properties": { + "enabled": {"type": "boolean"}, + "vegas_mode": {"type": "string"}, # plugin-owned, must stay validated + }, +} + + +@pytest.fixture +def manager(): + mgr = PluginManager.__new__(PluginManager) # skip the heavy constructor + mgr.logger = MagicMock() + mgr.schema_manager = MagicMock() + mgr._set_degraded_safe = MagicMock() + return mgr + + +class TestStripCoreOwnedKeys: + def test_removes_every_core_owned_key(self, manager): + cfg = {"enabled": True, "vegas_width_pct": 50, + "vegas_overflow": "wrap", "vegas_max_width_screens": 2} + assert manager._strip_core_owned_keys(cfg) == {"enabled": True} + + def test_leaves_plugin_owned_vegas_mode_alone(self, manager): + """A prefix rule would have eaten this one.""" + cfg = {"enabled": True, "vegas_mode": "scroll"} + assert manager._strip_core_owned_keys(cfg) == cfg + + def test_returns_the_same_object_when_nothing_to_strip(self, manager): + cfg = {"enabled": True} + assert manager._strip_core_owned_keys(cfg) is cfg + + def test_does_not_mutate_the_caller_config(self, manager): + cfg = {"enabled": True, "vegas_width_pct": 50} + manager._strip_core_owned_keys(cfg) + assert "vegas_width_pct" in cfg, "the live plugin config was mutated" + + def test_tolerates_a_non_dict(self, manager): + assert manager._strip_core_owned_keys(None) is None + + +class TestSoftValidation: + def _validate_with(self, manager, config, valid=True, errors=()): + manager.schema_manager.load_schema.return_value = STRICT_SCHEMA + manager.schema_manager.validate_config_against_schema.return_value = ( + valid, list(errors)) + manager._validate_config_schema_soft("baseball-scoreboard", config) + return manager.schema_manager.validate_config_against_schema.call_args + + def test_core_keys_never_reach_the_validator(self, manager): + """The regression: these keys reaching a strict schema is what flagged + 9 of 27 plugins degraded.""" + args = self._validate_with( + manager, {"enabled": True, "vegas_width_pct": 50}) + validated = args[0][0] + assert "vegas_width_pct" not in validated + assert validated == {"enabled": True} + + def test_plugin_owned_keys_still_reach_the_validator(self, manager): + args = self._validate_with( + manager, {"enabled": True, "vegas_mode": "scroll"}) + assert args[0][0]["vegas_mode"] == "scroll" + + def test_a_genuine_violation_is_still_reported(self, manager): + """Stripping core keys must not turn the check into a no-op.""" + self._validate_with( + manager, {"enabled": True, "typo_key": 1}, + valid=False, errors=["Field root: 'typo_key' was unexpected"]) + manager._set_degraded_safe.assert_called() + reason = manager._set_degraded_safe.call_args[0][1] + assert reason and "typo_key" in reason