From 68c66a0ff30cf3bae7816d852ee36433b66f3dc2 Mon Sep 17 00:00:00 2001 From: Samson Brock Date: Sun, 6 Sep 2026 02:00:02 -0500 Subject: [PATCH] Fix options flow changes silently discarded for pre-refactor UI entries (#1504) * Fix options flow changes being silently discarded for UI-configured entries validate() merges config_entry.options then config_entry.data, on the assumption that data only ever holds YAML-imported settings (which should win) or, for UI-created entries, just the entry name (harmless to apply last). That assumption doesn't hold for entries created before data/options were split: their data still carries the full settings snapshot from initial setup. Applying it after options means any change made through the options flow for a key that already exists in data (e.g. adding a light) is silently ignored, even though the options flow reports success and the entry reloads without error. Reproduced on a real entry: added a light via the options flow, entry reloaded cleanly, but the light was never picked up by the switch's service-call interceptor ("No switch found for entity_id=...") because data still held the old light list and clobbered the updated options. Fix: only let data win over options for genuinely YAML-imported entries (config_entry.source == SOURCE_IMPORT), matching the existing use of that check elsewhere in this file. For UI-configured entries, apply options last so changes made through the options flow actually take effect. * Add focused tests for the data/options merge order in validate() Covers both source-specific contracts the merge logic relies on, per review feedback on this PR: - SOURCE_USER: options must win over data (this PR's actual fix - proven to fail against the pre-fix code, verified locally by reverting switch.py and re-running). - SOURCE_IMPORT: data must keep winning over options (the existing, intentional YAML-precedence behavior - unchanged by this PR, verified to already pass against the pre-fix code too). Verified against a real Home Assistant instance's test harness (pytest-homeassistant-custom-component + the actual installed homeassistant package), not just reasoned about statically. --------- Co-authored-by: Bas Nijholt --- custom_components/adaptive_lighting/switch.py | 16 ++++++- tests/test_switch.py | 43 ++++++++++++++++++- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/custom_components/adaptive_lighting/switch.py b/custom_components/adaptive_lighting/switch.py index 3b042702..5db91823 100644 --- a/custom_components/adaptive_lighting/switch.py +++ b/custom_components/adaptive_lighting/switch.py @@ -565,8 +565,20 @@ def validate( if config_entry is not None: assert service_data is None assert defaults is None - data.update(config_entry.options) # come from options flow - data.update(config_entry.data) # all yaml settings come from data + if config_entry.source == SOURCE_IMPORT: + # YAML-configured entries: `data` is the authoritative YAML config + # and must win over any stray `options` from a prior UI setup. + data.update(config_entry.options) + data.update(config_entry.data) + else: + # UI-configured entries: settings are meant to live in `options` + # (see OptionsFlowHandler in config_flow.py). `data` here is + # either just the entry name, or - for entries created before + # data/options were split - a stale snapshot from initial setup. + # Applying it last would silently discard newer changes made + # through the options flow, so `options` must win instead. + data.update(config_entry.data) + data.update(config_entry.options) else: assert service_data is not None changed_settings = { diff --git a/tests/test_switch.py b/tests/test_switch.py index eabe226a..ce9d8ec8 100644 --- a/tests/test_switch.py +++ b/tests/test_switch.py @@ -75,6 +75,7 @@ from homeassistant.components.adaptive_lighting.switch import ( is_our_context, is_our_context_id, short_hash, + validate, ) from homeassistant.components.light import ( ATTR_BRIGHTNESS, @@ -98,7 +99,7 @@ except ImportError: from homeassistant.components.template.light import LightTemplate from homeassistant.components.template import light as template_light -from homeassistant.config_entries import ConfigEntryState +from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_USER, ConfigEntryState from homeassistant.const import ( ATTR_AREA_ID, ATTR_ENTITY_ID, @@ -3235,3 +3236,43 @@ async def test_detect_non_ha_changes_with_separate_turn_on_commands(hass): assert ( light.brightness == manual_brightness ), f"AL overrode manual brightness {manual_brightness} with {al_brightness}" + + +def test_validate_ui_options_win_over_stale_data(): + """A UI-configured entry's `options` (from the options flow) must win. + + `data` for a `SOURCE_USER` entry either only holds the entry name, or - + for entries created before `data`/`options` were split - a stale + snapshot from initial setup. Either way, a later change made through + the options flow (stored in `options`) must not be silently discarded + by that stale/legacy `data`. + """ + entry = MockConfigEntry( + domain=DOMAIN, + source=SOURCE_USER, + data={CONF_NAME: DEFAULT_NAME, CONF_LIGHTS: ["light.a"]}, + options={CONF_LIGHTS: ["light.a", "light.b"]}, + ) + + result = validate(entry) + + assert result[CONF_LIGHTS] == ["light.a", "light.b"] + + +def test_validate_yaml_data_wins_over_stray_options(): + """A YAML-imported entry's `data` must keep winning over `options`. + + YAML configuration is the source of truth for a `SOURCE_IMPORT` entry, + so any leftover `options` (e.g. from a UI setup that predates the YAML + import) must not override it. + """ + entry = MockConfigEntry( + domain=DOMAIN, + source=SOURCE_IMPORT, + data={CONF_NAME: DEFAULT_NAME, CONF_LIGHTS: ["light.a"]}, + options={CONF_LIGHTS: ["light.b"]}, + ) + + result = validate(entry) + + assert result[CONF_LIGHTS] == ["light.a"]