[esphome] Warn when a YAML merge (<<:) drops a key (#17246)

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

esphome/config.py

@@ -20,6 +20,7 @@ from esphome.const import (
     CONF_ESPHOME,
     CONF_EXTERNAL_COMPONENTS,
     CONF_ID,
+    CONF_MERGE_WARNINGS,
     CONF_MIN_VERSION,
     CONF_PACKAGES,
     CONF_PLATFORM,
@@ -1184,6 +1185,24 @@ def validate_config(
         )
         return result
 
+    # Warn about any keys silently dropped by `<<` merge includes (shallow,
+    # first-wins). The esphome: section is now known, so we can honor its
+    # `merge_warnings:` opt-out. Always drain the queue to keep it from leaking
+    # into a later run.
+    if (dropped := yaml_util.take_dropped_merge_keys()) and (
+        not isinstance(esphome_conf := config[CONF_ESPHOME], dict)
+        or esphome_conf.get(CONF_MERGE_WARNINGS, True)
+    ):
+        for key, location in dict.fromkeys(dropped):
+            _LOGGER.warning(
+                "Key '%s' (%s) was dropped while processing a '<<' merge because it "
+                "is already defined. Merge keys don't combine sections - the first "
+                "definition wins. Use 'packages:' to merge sections, or set "
+                "'esphome: { merge_warnings: false }' to silence this.",
+                key,
+                location,
+            )
+
     # Snapshot the user's config before any schema validation defaults are
     # applied. preload_core_config and later validation steps rewrite entries
     # in-place with defaulted values; deep-copying here preserves the

esphome/const.py

@@ -613,6 +613,7 @@ CONF_MEASUREMENT_SEQUENCE_NUMBER = "measurement_sequence_number"
 CONF_MEDIA_PLAYER = "media_player"
 CONF_MEDIUM = "medium"
 CONF_MEMORY_BLOCKS = "memory_blocks"
+CONF_MERGE_WARNINGS = "merge_warnings"
 CONF_MESSAGE = "message"
 CONF_METHANE = "methane"
 CONF_METHOD = "method"

esphome/core/config.py

@@ -26,6 +26,7 @@ from esphome.const import (
     CONF_INCLUDES,
     CONF_INCLUDES_C,
     CONF_LIBRARIES,
+    CONF_MERGE_WARNINGS,
     CONF_MIN_VERSION,
     CONF_NAME,
     CONF_NAME_ADD_MAC_SUFFIX,
@@ -316,6 +317,7 @@ CONFIG_SCHEMA = cv.All(
             cv.Optional(CONF_INCLUDES_C, default=[]): cv.ensure_list(valid_include),
             cv.Optional(CONF_LIBRARIES, default=[]): cv.ensure_list(cv.string_strict),
             cv.Optional(CONF_NAME_ADD_MAC_SUFFIX, default=False): cv.boolean,
+            cv.Optional(CONF_MERGE_WARNINGS, default=True): cv.boolean,
             cv.Optional(CONF_DEBUG_SCHEDULER, default=False): cv.boolean,
             cv.Optional(CONF_PROJECT): cv.Schema(
                 {

esphome/yaml_util.py

@@ -51,6 +51,29 @@ _load_listeners: list[Callable[[Path], None]] = []
 
 DocumentPath = list[str | int]
 
+# Key under CORE.data used to accumulate keys that a `<<` merge silently
+# dropped. The warning is emitted later (see esphome.config.validate_config),
+# because the esphome: option that suppresses it isn't known while parsing.
+_MERGE_WARNINGS_KEY = "yaml_dropped_merge_keys"
+
+
+def _record_dropped_merge_key(parent_file: Path, key: Any) -> None:
+    """Record a mapping key that a ``<<`` merge silently dropped.
+
+    Merge keys follow the YAML spec's shallow, first-wins semantics: a key that
+    already exists in the mapping (or came from an earlier merge) is discarded
+    rather than deep-merged the way ``packages:`` would combine it. We collect
+    these so a single warning can be emitted once the config is loaded.
+    """
+    esp_range = getattr(key, "esp_range", None)
+    location = str(esp_range.start_mark) if esp_range is not None else str(parent_file)
+    CORE.data.setdefault(_MERGE_WARNINGS_KEY, []).append((str(key), location))
+
+
+def take_dropped_merge_keys() -> list[tuple[str, str]]:
+    """Return and clear the keys dropped during ``<<`` merges so far."""
+    return CORE.data.pop(_MERGE_WARNINGS_KEY, [])
+
 
 class SensitiveStr(str):
     """Marker subclass for validated strings that should be masked in
@@ -551,6 +574,10 @@ class ESPHomeLoaderMixin:
                     #  is expected to contain mapping nodes and each of these nodes is merged in
                     #  turn according to its order in the sequence. Keys in mapping nodes earlier
                     #  in the sequence override keys specified in later mapping nodes."
+                    #
+                    # This is a silent shallow drop (unlike `packages:`, which deep-merges).
+                    # Record it so a warning can be emitted after the config loads.
+                    _record_dropped_merge_key(self.name, key)
                     continue
                 pairs.append((key, value))
                 # Add key node to seen keys, for sequence merge values.

script/ci-custom.py

@@ -555,7 +555,7 @@ def lint_constants_usage():
 # Maximum allowed CONF_ constants in esphome/const.py.
 # This file is frozen — new constants go in esphome/components/const/__init__.py.
 # Decrease this number when constants are moved out of const.py.
-CONST_PY_MAX_CONF = 1013
+CONST_PY_MAX_CONF = 1014
 
 
 @lint_content_check(include=["esphome/const.py"])

tests/unit_tests/test_config_normalization.py

@@ -1,6 +1,7 @@
 """Unit tests for esphome.config module."""
 
 from collections.abc import Generator
+import logging
 from pathlib import Path
 from unittest.mock import MagicMock, Mock, patch
 
@@ -113,3 +114,57 @@ def test_ota_with_platform_list_and_captive_portal(fixtures_dir: Path) -> None:
     platforms = {p.get("platform") for p in result["ota"]}
     assert "esphome" in platforms, f"Expected esphome platform in {platforms}"
     assert "web_server" in platforms, f"Expected web_server platform in {platforms}"
+
+
+def _write_merge_conflict_config(tmp_path: Path, *, suppress: bool) -> Path:
+    """Create a config where two `<<` includes both define `logger:`.
+
+    The second `logger:` is dropped by the shallow merge. Returns the main file.
+    """
+    (tmp_path / "a.yaml").write_text("logger:\n  level: DEBUG\n")
+    (tmp_path / "b.yaml").write_text("logger:\n  level: INFO\n")
+    esphome_section = "esphome:\n  name: test\n"
+    if suppress:
+        esphome_section += "  merge_warnings: false\n"
+    main = tmp_path / "main.yaml"
+    main.write_text(f"{esphome_section}<<: !include a.yaml\n<<: !include b.yaml\n")
+    return main
+
+
+def test_validate_config_warns_on_dropped_merge_key(
+    tmp_path: Path, caplog: pytest.LogCaptureFixture
+) -> None:
+    """By default, a `<<` merge that drops a key logs a warning."""
+    main = _write_merge_conflict_config(tmp_path, suppress=False)
+    CORE.config_path = main
+    raw_config = yaml_util.load_yaml(main)
+
+    with caplog.at_level(logging.WARNING, logger="esphome.config"):
+        config.validate_config(raw_config, {})
+
+    assert any(
+        "was dropped while processing a '<<' merge" in record.message
+        and "logger" in record.message
+        for record in caplog.records
+    )
+    # The queue is drained so the warning cannot leak into a later run.
+    assert yaml_util.take_dropped_merge_keys() == []
+
+
+def test_validate_config_suppresses_merge_warning(
+    tmp_path: Path, caplog: pytest.LogCaptureFixture
+) -> None:
+    """`esphome: merge_warnings: false` hides the warning but still drains the queue."""
+    main = _write_merge_conflict_config(tmp_path, suppress=True)
+    CORE.config_path = main
+    raw_config = yaml_util.load_yaml(main)
+
+    with caplog.at_level(logging.WARNING, logger="esphome.config"):
+        config.validate_config(raw_config, {})
+
+    assert not any(
+        "was dropped while processing a '<<' merge" in record.message
+        for record in caplog.records
+    )
+    # The queue is drained even when the warning is suppressed.
+    assert yaml_util.take_dropped_merge_keys() == []

tests/unit_tests/test_yaml_util.py

@@ -1395,3 +1395,48 @@ def test_dump__redaction_flag_does_not_leak_between_calls() -> None:
     assert "\\033[8m" in redacted
     assert "\\033[8m" not in raw
     assert "\\033[8m" in redacted_again
+
+
+@pytest.fixture(autouse=True)
+def clear_dropped_merge_keys() -> None:
+    """Reset the dropped-merge-key queue between tests."""
+    core.CORE.data.pop(yaml_util._MERGE_WARNINGS_KEY, None)
+    yield
+    core.CORE.data.pop(yaml_util._MERGE_WARNINGS_KEY, None)
+
+
+def test_merge_include_records_dropped_keys(tmp_path: Path) -> None:
+    """A `<<` merge that overlaps an existing key records it (shallow first-wins)."""
+    (tmp_path / "a.yaml").write_text("api:\n  reboot_timeout: 5min\n")
+    (tmp_path / "b.yaml").write_text("api:\n  password: secret\n")
+    test_yaml = tmp_path / "test.yaml"
+    test_yaml.write_text("<<: !include a.yaml\n<<: !include b.yaml\n")
+
+    with patch.object(core.CORE, "config_path", test_yaml):
+        result = yaml_util.load_yaml(test_yaml)
+
+    # First definition wins; the second `api` block is dropped entirely.
+    assert result["api"] == {"reboot_timeout": "5min"}
+
+    dropped = yaml_util.take_dropped_merge_keys()
+    assert len(dropped) == 1
+    key, location = dropped[0]
+    assert key == "api"
+    assert "b.yaml" in location
+    # Queue is drained after being taken.
+    assert yaml_util.take_dropped_merge_keys() == []
+
+
+def test_merge_include_no_overlap_records_nothing(tmp_path: Path) -> None:
+    """A `<<` merge with distinct top-level keys drops nothing."""
+    (tmp_path / "a.yaml").write_text("api:\n  reboot_timeout: 5min\n")
+    (tmp_path / "b.yaml").write_text("logger:\n  level: DEBUG\n")
+    test_yaml = tmp_path / "test.yaml"
+    test_yaml.write_text("<<: !include a.yaml\n<<: !include b.yaml\n")
+
+    with patch.object(core.CORE, "config_path", test_yaml):
+        result = yaml_util.load_yaml(test_yaml)
+
+    assert result["api"] == {"reboot_timeout": "5min"}
+    assert result["logger"] == {"level": "DEBUG"}
+    assert yaml_util.take_dropped_merge_keys() == []