[core] Report incomplete YAML discovery and record remote package sources (#20312)

esphome/bundle.py

@@ -418,6 +418,18 @@ class ConfigBundleCreator:
         must ship every candidate so the remote build can pick any one.
         """
         discovered = yaml_util.discover_user_yaml_files(self._config_path)
+        if discovered.load_errors:
+            _LOGGER.warning(
+                "Bundle may be incomplete; could not load all configuration files: %s",
+                "; ".join(discovered.load_errors),
+            )
+        if discovered.unresolved:
+            _LOGGER.warning(
+                "Bundle may be incomplete; %d !include path(s) use "
+                "substitutions and cannot be captured: %s",
+                len(discovered.unresolved),
+                ", ".join(discovered.unresolved),
+            )
         self._secrets_paths.update(discovered.secrets)
         # A !secret inside a file this re-parse does not reach (for example
         # a git-fetched package the builder re-fetches) still resolves

esphome/components/packages/__init__.py

@@ -1,5 +1,6 @@
 from collections import UserDict
 from collections.abc import Callable
+from dataclasses import dataclass, field
 from functools import reduce
 import logging
 from pathlib import Path
@@ -35,11 +36,45 @@ from esphome.const import (
     CONF_VARS,
     __version__ as ESPHOME_VERSION,
 )
-from esphome.core import EsphomeError
+from esphome.core import CORE, EsphomeError
 
 _LOGGER = logging.getLogger(__name__)
 
 DOMAIN = "packages"
+
+
+@dataclass(frozen=True)
+class RemotePackageSource:
+    """A remote source a package was fetched from while processing the config."""
+
+    url: str
+    ref: str | None
+
+
+@dataclass
+class PackagesData:
+    """Per-run package state, keyed under DOMAIN in CORE.data."""
+
+    # Insertion-ordered and de-duplicated
+    remote_sources: dict[RemotePackageSource, None] = field(default_factory=dict)
+
+
+def _get_data() -> PackagesData:
+    if DOMAIN not in CORE.data:
+        CORE.data[DOMAIN] = PackagesData()
+    return CORE.data[DOMAIN]
+
+
+def get_remote_package_sources() -> tuple[RemotePackageSource, ...]:
+    """Remote sources fetched for this config, in fetch order.
+
+    store_yaml uses them to tell remote parts of the config from local files.
+    """
+    if (data := CORE.data.get(DOMAIN)) is None:
+        return ()
+    return tuple(data.remote_sources)
+
+
 # Guard against infinite include chains (e.g. A includes B includes A).
 MAX_INCLUDE_DEPTH = 20
 
@@ -193,6 +228,9 @@ def _process_remote_package(config: dict[str, Any]) -> dict[str, Any]:
         username=config.get(CONF_USERNAME),
         password=config.get(CONF_PASSWORD),
     )
+    _get_data().remote_sources[
+        RemotePackageSource(config[CONF_URL], config.get(CONF_REF))
+    ] = None
     files: list[dict[str, Any]] = []
 
     # ``repo_root`` is the directory containing ``.git`` and must be passed

esphome/yaml_util.py

@@ -12,7 +12,7 @@ import math
 import os
 from pathlib import Path
 import re
-from typing import Any
+from typing import Any, NamedTuple
 import uuid
 
 import yaml
@@ -410,9 +410,11 @@ def _load_include_candidates(
     include: IncludeFile,
     *,
     warn_on_unresolved: bool,
+    warn_on_load_error: bool,
     seen: set[int],
     expanded_paths: set[Path],
     keepalive: list[Any],
+    result: ForceLoadResult,
 ) -> None:
     """Load every filesystem candidate for an unresolved ``IncludeFile``."""
     from voluptuous import Invalid
@@ -425,6 +427,7 @@ def _load_include_candidates(
             include.file,
             include.parent_file,
         )
+        result.unresolved.append(str(include.file))
         return
     _LOGGER.debug(
         "Expanding !include %s (referenced from %s) to %d candidate file(s)",
@@ -439,27 +442,27 @@ def _load_include_candidates(
         try:
             loaded = include.with_file(candidate.as_posix()).load()
         except (EsphomeError, Invalid) as err:
-            # Unlike an unresolved pattern (expected during the discovery
-            # re-parse), a matched on-disk candidate that fails to load is a
-            # genuine user error; warn in every mode. The file itself is
-            # still tracked (the load listener fires before parsing), only
-            # its nested includes go undiscovered.
-            _LOGGER.warning(
+            # A real user error. The file stays tracked (the listener fires
+            # before parsing); only its nested includes are missed.
+            (_LOGGER.warning if warn_on_load_error else _LOGGER.debug)(
                 "Failed to load candidate %s for !include %s: %s",
                 candidate,
                 include.file,
                 err,
             )
+            result.errors.append(f"{candidate}: {err}")
             continue
         # The throwaway IncludeFile is this tree's only owner; keep the tree
         # alive so ids recorded in ``seen`` stay unique for the traversal.
         keepalive.append(loaded)
-        force_load_include_files(
+        _force_load_include_files(
             loaded,
             warn_on_unresolved=warn_on_unresolved,
+            warn_on_load_error=warn_on_load_error,
             _seen=seen,
             _expanded_paths=expanded_paths,
             _keepalive=keepalive,
+            _result=result,
         )
 
 
@@ -473,14 +476,19 @@ def find_secret_references(text: str) -> set[str]:
     return {match.group(1) for match in _SECRET_REFERENCE_RE.finditer(text)}
 
 
+class ForceLoadResult(NamedTuple):
+    """Templated includes that matched no file, and includes that failed to load.
+
+    Either being non-empty means the walk was incomplete.
+    """
+
+    unresolved: list[str]
+    errors: list[str]
+
+
 def force_load_include_files(
-    obj: Any,
-    *,
-    warn_on_unresolved: bool = True,
-    _seen: set[int] | None = None,
-    _expanded_paths: set[Path] | None = None,
-    _keepalive: list[Any] | None = None,
-) -> None:
+    obj: Any, *, warn_on_unresolved: bool = True, warn_on_load_error: bool = True
+) -> ForceLoadResult:
     """Recursively resolve any deferred ``IncludeFile`` instances in a YAML tree.
 
     Nested ``!include`` returns a deferred ``IncludeFile`` that is only resolved
@@ -495,8 +503,30 @@ def force_load_include_files(
     expression could select. By default a warning is logged when no candidate
     exists; pass ``warn_on_unresolved=False`` (used by discovery paths that
     run on a fresh re-parse where substitutions haven't been applied yet) to
-    demote it to a debug log.
+    demote it to a debug log. Callers that report ``errors`` themselves pass
+    ``warn_on_load_error=False`` so load failures are not logged twice.
     """
+    result = ForceLoadResult([], [])
+    _force_load_include_files(
+        obj,
+        warn_on_unresolved=warn_on_unresolved,
+        warn_on_load_error=warn_on_load_error,
+        _result=result,
+    )
+    return result
+
+
+def _force_load_include_files(
+    obj: Any,
+    *,
+    warn_on_unresolved: bool = True,
+    warn_on_load_error: bool = True,
+    _seen: set[int] | None = None,
+    _expanded_paths: set[Path] | None = None,
+    _keepalive: list[Any] | None = None,
+    _result: ForceLoadResult,
+) -> None:
+    """Walk for :func:`force_load_include_files`, adding to ``result``."""
     from voluptuous import Invalid
 
     if _seen is None:
@@ -519,51 +549,60 @@ def force_load_include_files(
             _load_include_candidates(
                 obj,
                 warn_on_unresolved=warn_on_unresolved,
+                warn_on_load_error=warn_on_load_error,
                 seen=_seen,
                 expanded_paths=_expanded_paths,
                 keepalive=_keepalive,
+                result=_result,
             )
             return
         try:
             loaded = obj.load()
         except (EsphomeError, Invalid) as err:
-            _LOGGER.warning(
+            (_LOGGER.warning if warn_on_load_error else _LOGGER.debug)(
                 "Failed to load !include %s (referenced from %s): %s",
                 obj.file,
                 obj.parent_file,
                 err,
             )
+            _result.errors.append(f"{obj.file}: {err}")
             return
-        force_load_include_files(
+        _force_load_include_files(
             loaded,
             warn_on_unresolved=warn_on_unresolved,
+            warn_on_load_error=warn_on_load_error,
             _seen=_seen,
             _expanded_paths=_expanded_paths,
             _keepalive=_keepalive,
+            _result=_result,
         )
     elif isinstance(obj, dict):
         if id(obj) in _seen:
             return
         _seen.add(id(obj))
         for value in obj.values():
-            force_load_include_files(
+            _force_load_include_files(
                 value,
                 warn_on_unresolved=warn_on_unresolved,
+                warn_on_load_error=warn_on_load_error,
                 _seen=_seen,
                 _expanded_paths=_expanded_paths,
                 _keepalive=_keepalive,
+                _result=_result,
             )
     elif isinstance(obj, (list, tuple)):
         if id(obj) in _seen:
             return
         _seen.add(id(obj))
         for item in obj:
-            force_load_include_files(
+            _force_load_include_files(
                 item,
                 warn_on_unresolved=warn_on_unresolved,
+                warn_on_load_error=warn_on_load_error,
                 _seen=_seen,
                 _expanded_paths=_expanded_paths,
                 _keepalive=_keepalive,
+                _result=_result,
             )
 
 
@@ -575,11 +614,14 @@ class DiscoveredYamlFiles:
     were re-parsing the user's config; ``secrets`` is the subset whose
     *un-resolved* filename matched :data:`esphome.const.SECRETS_FILES` (so
     a ``secrets.yaml`` symlinked to a differently-named target is still
-    flagged as secrets).
+    flagged as secrets). ``files`` is incomplete when ``unresolved`` (templated
+    include paths) or ``load_errors`` is non-empty.
     """
 
     files: list[Path] = field(default_factory=list)
     secrets: set[Path] = field(default_factory=set)
+    unresolved: list[str] = field(default_factory=list)
+    load_errors: list[str] = field(default_factory=list)
 
 
 def discover_user_yaml_files(config_path: Path) -> DiscoveredYamlFiles:
@@ -609,9 +651,13 @@ def discover_user_yaml_files(config_path: Path) -> DiscoveredYamlFiles:
         try:
             try:
                 data = load_yaml(config_path)
-            except EsphomeError:
-                return DiscoveredYamlFiles(list(loaded), secrets)
-            force_load_include_files(data, warn_on_unresolved=False)
+            except EsphomeError as err:
+                return DiscoveredYamlFiles(
+                    list(loaded), secrets, load_errors=[f"{config_path}: {err}"]
+                )
+            unresolved, load_errors = force_load_include_files(
+                data, warn_on_unresolved=False, warn_on_load_error=False
+            )
         finally:
             _load_listeners.remove(_capture_secret)
 
@@ -622,7 +668,7 @@ def discover_user_yaml_files(config_path: Path) -> DiscoveredYamlFiles:
         if path not in seen:
             seen.add(path)
             unique.append(path)
-    return DiscoveredYamlFiles(unique, secrets)
+    return DiscoveredYamlFiles(unique, secrets, unresolved, load_errors)
 
 
 def _add_data_ref(fn):

tests/component_tests/packages/test_packages.py

@@ -10,9 +10,11 @@ import pytest
 from esphome import bundle
 from esphome.components.packages import (
     CONFIG_SCHEMA,
+    RemotePackageSource,
     _substitute_package_definition,
     _walk_packages,
     do_packages_pass,
+    get_remote_package_sources,
     merge_packages,
     resolve_packages,
 )
@@ -614,6 +616,49 @@ def test_package_remove_by_missing_id() -> None:
     assert actual == expected
 
 
+def test_get_remote_package_sources_empty_without_remote_packages() -> None:
+    """A config without remote packages records no sources."""
+    assert get_remote_package_sources() == ()
+
+
+@patch("esphome.yaml_util.load_yaml")
+@patch("pathlib.Path.is_file")
+@patch("esphome.git.clone_or_update")
+def test_get_remote_package_sources_records_each_fetch_once(
+    mock_clone_or_update, mock_is_file, mock_load_yaml
+) -> None:
+    """Every fetched remote source is recorded exactly once."""
+    mock_clone_or_update.return_value = (Path("/tmp/noexists"), MagicMock())
+    mock_is_file.return_value = True
+    mock_load_yaml.side_effect = lambda *args, **kwargs: OrderedDict()
+
+    config = {
+        CONF_PACKAGES: {
+            "first": {
+                CONF_URL: "https://github.com/esphome/repo-a",
+                CONF_REF: "main",
+                CONF_FILES: [TEST_YAML_FILENAME],
+            },
+            "again": {
+                CONF_URL: "https://github.com/esphome/repo-a",
+                CONF_REF: "main",
+                CONF_FILES: ["other.yaml"],
+            },
+            "second": {
+                CONF_URL: "https://github.com/esphome/repo-b",
+                CONF_FILES: [TEST_YAML_FILENAME],
+            },
+        }
+    }
+    packages_pass(config)
+
+    # packages are processed last to first, so repo-b is fetched before repo-a
+    assert get_remote_package_sources() == (
+        RemotePackageSource("https://github.com/esphome/repo-b", None),
+        RemotePackageSource("https://github.com/esphome/repo-a", "main"),
+    )
+
+
 @patch("esphome.yaml_util.load_yaml")
 @patch("pathlib.Path.is_file")
 @patch("esphome.git.clone_or_update")

tests/unit_tests/test_bundle.py

@@ -1363,7 +1363,8 @@ def test_discover_files_candidate_outside_config_dir_skipped(
 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."""
+    """A nested !include pointing at a missing file is reported in the bundle
+    summary, not a second time by yaml_util, and skipped."""
     config_dir = _setup_config_dir(tmp_path)
     (config_dir / "test.yaml").write_text(
         "esphome:\n  name: test\nwifi: !include missing.yaml\n"
@@ -1374,10 +1375,12 @@ def test_discover_files_nested_include_load_failure(
 
     paths = [f.path for f in files]
     assert "test.yaml" in paths
+    warnings = [r.message for r in caplog.records if r.levelname == "WARNING"]
     assert any(
-        "failed to load !include" in r.message.lower() and "missing.yaml" in r.message
-        for r in caplog.records
+        m.startswith("Bundle may be incomplete") and "missing.yaml" in m
+        for m in warnings
     )
+    assert not any("failed to load !include" in m.lower() for m in warnings)
 
 
 def test_force_load_skips_duplicate_include_file() -> None:

tests/unit_tests/test_yaml_util.py

@@ -1280,22 +1280,52 @@ def test_force_load_include_files_unresolved_log_level(
     assert matching == [expect_level]
 
 
+def test_force_load_include_files_returns_unresolved_paths(
+    patch_include_file: None,
+) -> None:
+    """Includes with substitution-templated paths are reported back to the
+    caller; resolvable ones are not."""
+    templated = _StubInclude("${var}.yaml", unresolved=True)
+    plain = _StubInclude("ok.yaml")
+    result = force_load_include_files({"a": templated, "b": plain})
+    assert result.unresolved == [str(templated.file)]
+    assert result.errors == []
+    assert plain.load_calls == 1
+
+
 def test_force_load_include_files_warns_on_load_failure(
     patch_include_file: None,
     caplog: pytest.LogCaptureFixture,
 ) -> None:
-    """An `EsphomeError` raised by `load()` is caught and logged, not propagated."""
+    """An `EsphomeError` raised by `load()` is caught, logged, and reported to
+    the caller — not propagated."""
     stub = _StubInclude("missing.yaml", raise_on_load=EsphomeError("boom"))
     with caplog.at_level("WARNING", logger="esphome.yaml_util"):
-        force_load_include_files({"k": stub})
+        result = force_load_include_files({"k": stub})
     assert any(
         "Failed to load !include" in r.message and "missing.yaml" in r.message
         for r in caplog.records
     )
+    assert result.errors == [f"{stub.file}: boom"]
+    assert result.unresolved == []
+
+
+def test_force_load_include_files_load_failure_quiet_when_caller_reports(
+    patch_include_file: None,
+    caplog: pytest.LogCaptureFixture,
+) -> None:
+    """With warn_on_load_error=False the failure is still returned but only
+    logged at debug, so a caller that reports it does not duplicate it."""
+    stub = _StubInclude("missing.yaml", raise_on_load=EsphomeError("boom"))
+    with caplog.at_level("DEBUG", logger="esphome.yaml_util"):
+        result = force_load_include_files({"k": stub}, warn_on_load_error=False)
+    assert result.errors == [f"{stub.file}: boom"]
+    failures = [r for r in caplog.records if "Failed to load !include" in r.message]
+    assert [r.levelname for r in failures] == ["DEBUG"]
 
 
 def test_discovered_yaml_files_holds_files_and_secrets() -> None:
-    """`DiscoveredYamlFiles` is a small data carrier; both fields are mandatory."""
+    """`DiscoveredYamlFiles` is a small data carrier."""
     files = [Path("/tmp/a.yaml")]
     secrets = {Path("/tmp/a.yaml")}
     discovered = DiscoveredYamlFiles(files, secrets)
@@ -1358,11 +1388,24 @@ def test_discover_user_yaml_files_flags_secrets_symlink(tmp_path: Path) -> None:
     assert target.resolve() in discovered.secrets
 
 
-def test_discover_user_yaml_files_swallows_parse_errors(tmp_path: Path) -> None:
-    """A YAML parse failure returns whatever was tracked so far without raising."""
+def test_discover_user_yaml_files_reports_parse_errors(tmp_path: Path) -> None:
+    """A YAML parse failure is surfaced in `.load_errors` (not raised), so
+    consumers can tell the file set is incomplete."""
     entry = _write(tmp_path, "entry.yaml", "esphome: [unterminated\n")
     discovered = discover_user_yaml_files(entry)
     assert isinstance(discovered, DiscoveredYamlFiles)
+    assert len(discovered.load_errors) == 1
+    assert "entry.yaml" in discovered.load_errors[0]
+
+
+def test_discover_user_yaml_files_reports_unresolved_includes(
+    tmp_path: Path,
+) -> None:
+    """A substitution-templated `!include` path is surfaced in `.unresolved`."""
+    entry = _write_entry_including(tmp_path, "${board}.yaml")
+    discovered = discover_user_yaml_files(entry)
+    assert len(discovered.unresolved) == 1
+    assert "${board}.yaml" in discovered.unresolved[0]
 
 
 def test_discover_user_yaml_files_deduplicates(tmp_path: Path) -> None:
@@ -1589,9 +1632,9 @@ def test_discover_user_yaml_files_many_candidates_keep_nested_includes(
 def test_discover_user_yaml_files_bad_candidate_still_tracked(
     tmp_path: Path, caplog: pytest.LogCaptureFixture
 ) -> None:
-    """A matched candidate that fails to parse warns even during discovery,
-    stays tracked (the load listener fires before parsing), and doesn't block
-    other candidates."""
+    """A matched candidate that fails to parse is reported in ``load_errors``
+    (logged only at debug, the caller reports it), stays tracked (the load
+    listener fires before parsing), and doesn't block other candidates."""
     _write(tmp_path, "keys/good.yaml", "api:\n")
     _write(tmp_path, "keys/bad.yaml", "esphome: [unterminated\n")
     with caplog.at_level("DEBUG", logger="esphome.yaml_util"):
@@ -1604,7 +1647,8 @@ def test_discover_user_yaml_files_bad_candidate_still_tracked(
     matching = [
         r.levelname for r in caplog.records if "Failed to load candidate" in r.message
     ]
-    assert matching == ["WARNING"]
+    assert matching == ["DEBUG"]
+    assert any("bad.yaml" in e for e in discovered.load_errors)
 
 
 def test_discover_user_yaml_files_tolerates_templated_top_level_include(