[bundle] Force-resolve nested IncludeFile during file discovery (#15762)

esphome/bundle.py

@@ -151,8 +151,8 @@ class ConfigBundleCreator:
 
     def __init__(self, config: dict[str, Any]) -> None:
         self._config = config
-        self._config_dir = CORE.config_dir
-        self._config_path = CORE.config_path
+        self._config_dir = Path(CORE.config_dir).resolve()
+        self._config_path = Path(CORE.config_path).resolve()
         self._files: list[BundleFile] = []
         self._seen_paths: set[Path] = set()
         self._secrets_paths: set[Path] = set()
@@ -258,21 +258,36 @@ class ConfigBundleCreator:
     def _discover_yaml_includes(self) -> None:
         """Discover YAML files loaded during config parsing.
 
-        We track files by wrapping _load_yaml_internal. The config has already
-        been loaded at this point (bundle is a POST_CONFIG_ACTION), so we
-        re-load just to discover the file list.
+        Deliberately uses a fresh re-parse and force-loads every deferred
+        ``IncludeFile`` to include *all* potentially-reachable includes,
+        even branches not selected by the local substitutions. Bundles are
+        meant to be compiled on another system where command-line
+        substitution overrides may choose a different branch — e.g.
+        ``!include network/${eth_model}/config.yaml`` must ship every
+        candidate so the remote build can pick any one.
+
+        Entries with unresolved substitution variables in the filename
+        path are skipped with a warning (they cannot be resolved without
+        the substitution pass).
 
         Secrets files are tracked separately so we can filter them to
         only include the keys this config actually references.
         """
+        # Must be a fresh parse: IncludeFile.load() caches its result in
+        # _content, and we discover files by listening for loader calls. On
+        # an already-parsed tree the cache is populated, .load() returns
+        # without calling the loader, the listener never fires, and the
+        # referenced files would be silently dropped from the bundle.
         with yaml_util.track_yaml_loads() as loaded_files:
             try:
-                yaml_util.load_yaml(self._config_path)
+                data = yaml_util.load_yaml(self._config_path)
             except EsphomeError:
                 _LOGGER.debug(
                     "Bundle: re-loading YAML for include discovery failed, "
                     "proceeding with partial file list"
                 )
+            else:
+                _force_load_include_files(data)
 
         for fpath in loaded_files:
             if fpath == self._config_path.resolve():
@@ -608,6 +623,57 @@ def _add_bytes_to_tar(tar: tarfile.TarFile, name: str, data: bytes) -> None:
     tar.addfile(info, io.BytesIO(data))
 
 
+def _force_load_include_files(obj: Any, _seen: set[int] | None = None) -> None:
+    """Recursively resolve any ``IncludeFile`` instances in a YAML tree.
+
+    Nested ``!include`` returns a deferred ``IncludeFile`` that is only
+    resolved during the substitution pass. During bundle discovery we need
+    the referenced files to actually load so the ``track_yaml_loads``
+    listener fires for them.
+
+    ``IncludeFile`` instances with unresolved substitution variables in the
+    filename cannot be loaded — we skip and warn about those.
+    """
+    if _seen is None:
+        _seen = set()
+
+    if isinstance(obj, yaml_util.IncludeFile):
+        if id(obj) in _seen:
+            return
+        _seen.add(id(obj))
+        if obj.has_unresolved_expressions():
+            _LOGGER.warning(
+                "Bundle: cannot resolve !include %s (referenced from %s) "
+                "with substitutions in path",
+                obj.file,
+                obj.parent_file,
+            )
+            return
+        try:
+            loaded = obj.load()
+        except EsphomeError as err:
+            _LOGGER.warning(
+                "Bundle: failed to load !include %s (referenced from %s): %s",
+                obj.file,
+                obj.parent_file,
+                err,
+            )
+            return
+        _force_load_include_files(loaded, _seen)
+    elif isinstance(obj, dict):
+        if id(obj) in _seen:
+            return
+        _seen.add(id(obj))
+        for value in obj.values():
+            _force_load_include_files(value, _seen)
+    elif isinstance(obj, (list, tuple)):
+        if id(obj) in _seen:
+            return
+        _seen.add(id(obj))
+        for item in obj:
+            _force_load_include_files(item, _seen)
+
+
 def _resolve_include_path(include_path: Any) -> Path | None:
     """Resolve an include path to absolute, skipping system includes."""
     if isinstance(include_path, str) and include_path.startswith("<"):

tests/unit_tests/fixtures/bundle/bundle_test.yaml

@@ -11,9 +11,9 @@ esp32:
 logger:
   <<: !include common/base.yaml
 
-wifi:
-  ssid: !secret wifi_ssid
-  password: !secret wifi_password
+# Plain nested !include — deferred as an IncludeFile until the substitution
+# pass. The bundle must force-resolve it to pick up common/wifi.yaml.
+wifi: !include common/wifi.yaml
 
 api:
 

tests/unit_tests/fixtures/bundle/common/wifi.yaml

@@ -0,0 +1,2 @@
+ssid: !secret wifi_ssid
+password: !secret wifi_password

tests/unit_tests/test_bundle.py

@@ -5,8 +5,10 @@ from __future__ import annotations
 import io
 import json
 from pathlib import Path
+import shutil
 import tarfile
 from typing import Any
+from unittest.mock import patch
 
 import pytest
 
@@ -20,6 +22,7 @@ from esphome.bundle import (
     _add_bytes_to_tar,
     _default_target_dir,
     _find_used_secret_keys,
+    _force_load_include_files,
     extract_bundle,
     is_bundle_path,
     prepare_bundle_for_compile,
@@ -485,7 +488,7 @@ def test_read_bundle_manifest_minimal(tmp_path: Path) -> None:
 
     result = read_bundle_manifest(bundle_path)
     assert result.esphome_version == "unknown"
-    assert result.files == []
+    assert not result.files
     assert result.has_secrets is False
 
 
@@ -862,6 +865,117 @@ def test_discover_files_skips_missing_directory(tmp_path: Path) -> None:
     assert len(files) == 1
 
 
+def test_discover_files_nested_include(tmp_path: Path) -> None:
+    """Nested !include files (e.g. wifi: !include wifi.yaml) are bundled."""
+    config_dir = _setup_config_dir(tmp_path)
+    (config_dir / "test.yaml").write_text(
+        "esphome:\n  name: test\nwifi: !include wifi.yaml\n"
+    )
+    (config_dir / "wifi.yaml").write_text('ssid: "a"\npassword: "b"\n')
+
+    creator = ConfigBundleCreator({})
+    files = creator.discover_files()
+
+    paths = [f.path for f in files]
+    assert "test.yaml" in paths
+    assert "wifi.yaml" in paths
+
+
+def test_discover_files_deeply_nested_include(tmp_path: Path) -> None:
+    """Chains of !include (a includes b includes c) are fully resolved."""
+    config_dir = _setup_config_dir(tmp_path)
+    (config_dir / "test.yaml").write_text(
+        "esphome:\n  name: test\nwifi: !include level1.yaml\n"
+    )
+    (config_dir / "level1.yaml").write_text("nested: !include level2.yaml\n")
+    (config_dir / "level2.yaml").write_text('value: "leaf"\n')
+
+    creator = ConfigBundleCreator({})
+    files = creator.discover_files()
+
+    paths = [f.path for f in files]
+    assert "level1.yaml" in paths
+    assert "level2.yaml" in paths
+
+
+def test_discover_files_nested_include_unresolved_substitution(
+    tmp_path: Path,
+) -> None:
+    """!include with substitution vars in path cannot be resolved; skipped gracefully."""
+    config_dir = _setup_config_dir(tmp_path)
+    (config_dir / "test.yaml").write_text(
+        "esphome:\n  name: test\nwifi: !include ${platform}.yaml\n"
+    )
+
+    creator = ConfigBundleCreator({})
+    # Should not raise
+    files = creator.discover_files()
+
+    paths = [f.path for f in files]
+    assert "test.yaml" in paths
+
+
+def test_discover_files_nested_include_load_failure(
+    tmp_path: Path, caplog: pytest.LogCaptureFixture
+) -> None:
+    """A nested !include pointing at a missing file is logged and skipped."""
+    config_dir = _setup_config_dir(tmp_path)
+    (config_dir / "test.yaml").write_text(
+        "esphome:\n  name: test\nwifi: !include missing.yaml\n"
+    )
+
+    creator = ConfigBundleCreator({})
+    files = creator.discover_files()
+
+    paths = [f.path for f in files]
+    assert "test.yaml" in paths
+    assert any(
+        "failed to load !include" in r.message and "missing.yaml" in r.message
+        for r in caplog.records
+    )
+
+
+def test_force_load_skips_duplicate_include_file() -> None:
+    """The same IncludeFile referenced twice is only loaded once."""
+
+    class _StubInclude:
+        """Mimics yaml_util.IncludeFile minimally for _force_load testing."""
+
+        def __init__(self) -> None:
+            self.file = Path("dup.yaml")
+            self.parent_file = Path("root.yaml")
+            self.load_calls = 0
+
+        def has_unresolved_expressions(self) -> bool:
+            return False
+
+        def load(self) -> dict[str, Any]:
+            self.load_calls += 1
+            return {}
+
+    stub = _StubInclude()
+    # Same instance appears twice — second visit must hit the _seen guard.
+    tree = {"a": stub, "b": [stub]}
+
+    with patch("esphome.bundle.yaml_util.IncludeFile", _StubInclude):
+        _force_load_include_files(tree)
+
+    assert stub.load_calls == 1
+
+
+def test_force_load_handles_cyclic_containers() -> None:
+    """Cyclic dict/list references don't cause infinite recursion."""
+    cyclic_dict: dict[str, Any] = {}
+    cyclic_dict["self"] = cyclic_dict
+
+    cyclic_list: list[Any] = []
+    cyclic_list.append(cyclic_list)
+
+    # Should return without recursing forever
+    _force_load_include_files(cyclic_dict)
+    _force_load_include_files(cyclic_list)
+
+
 def test_discover_files_yaml_reload_failure(
     tmp_path: Path, monkeypatch: pytest.MonkeyPatch
 ) -> None:
@@ -1008,6 +1122,40 @@ def test_discover_files_walk_tuple_values(tmp_path: Path) -> None:
     assert "a.pem" in paths
 
 
+# ---------------------------------------------------------------------------
+# ConfigBundleCreator - fixture-based end-to-end
+# ---------------------------------------------------------------------------
+
+
+def test_discover_files_fixture_config(fixture_path: Path, tmp_path: Path) -> None:
+    """Use the real ``fixtures/bundle/`` tree as an end-to-end reproducer.
+
+    The fixture config uses ``wifi: !include common/wifi.yaml`` — a plain
+    nested !include that is returned as a deferred ``IncludeFile`` and only
+    resolved during the substitution pass. Before this fix, bundle discovery
+    never ran substitutions, so ``common/wifi.yaml`` was silently missing
+    from the bundle.
+    """
+    # Copy the fixture tree into a tmp dir so the test doesn't rely on the
+    # source repo being writable and so we can set CORE.config_path freely.
+    src = fixture_path / "bundle"
+    dst = tmp_path / "bundle"
+    shutil.copytree(src, dst)
+
+    CORE.config_path = dst / "bundle_test.yaml"
+
+    creator = ConfigBundleCreator({})
+    files = creator.discover_files()
+    paths = {f.path for f in files}
+
+    # Root and top-level !secret-referenced files
+    assert "bundle_test.yaml" in paths
+    assert "secrets.yaml" in paths
+    # The nested !include — this is what regressed when IncludeFile became
+    # deferred (PR #12213).
+    assert "common/wifi.yaml" in paths
+
+
 # ---------------------------------------------------------------------------
 # ConfigBundleCreator - create_bundle
 # ---------------------------------------------------------------------------