[store_yaml] Simplify: packages records remote sources, single tree walker, dict-style test packages

esphome/components/api/api_connection.cpp

@@ -1220,19 +1220,19 @@ void APIConnection::on_get_yaml_request() {
 #ifdef USE_ESP8266
   this->store_yaml_chunk_buf_ = std::make_unique<uint8_t[]>(STORE_YAML_CHUNK_SIZE);
 #endif
-  // All responses — including the single data-less done=true frame for a
-  // missing/empty blob — go through the loop-driven retry below, so a full
-  // TX buffer at request time can't strand the client without a terminal frame.
+  // All responses go through the loop-driven retry below, so a full TX
+  // buffer at request time can't strand the client without a terminal frame.
   this->store_yaml_pos_ = 0;
   this->try_send_store_yaml_();
 }
 
 // Caller guarantees: store_yaml_pos_ != SIZE_MAX (a request is in flight).
 void APIConnection::try_send_store_yaml_() {
+  // Every component's setup() completes before the app loop services API
+  // messages, and codegen always embeds a non-empty blob, so the component
+  // is present and total > 0 whenever a request is serviced.
   auto *comp = store_yaml::global_store_yaml;
-  // comp is only null if the request arrived before the component's setup();
-  // treat that like an empty blob and send just the terminal frame.
-  const size_t total = comp == nullptr ? 0 : comp->get_size();
+  const size_t total = comp->get_size();
 
 #ifdef USE_ESP8266
   const size_t chunk_size = STORE_YAML_CHUNK_SIZE;
@@ -1250,19 +1250,13 @@ void APIConnection::try_send_store_yaml_() {
     const size_t to_send = std::min(remaining, chunk_size);
 
     GetYamlResponse resp;
-    if (to_send != 0) {
 #ifdef USE_ESP8266
-      progmem_memcpy(this->store_yaml_chunk_buf_.get(), comp->get_data() + this->store_yaml_pos_, to_send);
-      resp.set_data(this->store_yaml_chunk_buf_.get(), to_send);
+    progmem_memcpy(this->store_yaml_chunk_buf_.get(), comp->get_data() + this->store_yaml_pos_, to_send);
+    resp.set_data(this->store_yaml_chunk_buf_.get(), to_send);
 #else
-      resp.set_data(comp->get_data() + this->store_yaml_pos_, to_send);
+    resp.set_data(comp->get_data() + this->store_yaml_pos_, to_send);
 #endif
-    } else {
-      // Terminal frame for an empty blob: a valid empty pointer keeps the
-      // forced `data` field's memcpy well-defined.
-      resp.set_data(reinterpret_cast<const uint8_t *>(""), 0);
-    }
-    if (this->store_yaml_pos_ == 0 && total != 0) {
+    if (this->store_yaml_pos_ == 0) {
       resp.total_size = static_cast<uint32_t>(total);
       resp.encoding = StringRef(store_yaml::ENCODING);
     }

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
 from pathlib import Path
 from typing import Any
@@ -33,9 +34,43 @@ from esphome.const import (
     CONF_VARS,
     __version__ as ESPHOME_VERSION,
 )
-from esphome.core import EsphomeError
+from esphome.core import CORE, EsphomeError
 
 DOMAIN = CONF_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."""
+
+    remote_sources: list[RemotePackageSource] = field(default_factory=list)
+
+
+def _get_data() -> PackagesData:
+    if DOMAIN not in CORE.data:
+        CORE.data[DOMAIN] = PackagesData()
+    return CORE.data[DOMAIN]
+
+
+def get_remote_package_sources() -> list[RemotePackageSource]:
+    """Remote sources fetched while processing this config, in fetch order.
+
+    Consumers (e.g. store_yaml) use this to tell which parts of the config
+    came from remote repositories rather than local files.
+    """
+    if (data := CORE.data.get(DOMAIN)) is None:
+        return []
+    return data.remote_sources
+
+
 # Guard against infinite include chains (e.g. A includes B includes A).
 MAX_INCLUDE_DEPTH = 20
 
@@ -189,6 +224,10 @@ def _process_remote_package(config: dict[str, Any]) -> dict[str, Any]:
         username=config.get(CONF_USERNAME),
         password=config.get(CONF_PASSWORD),
     )
+    source = RemotePackageSource(config[CONF_URL], config.get(CONF_REF))
+    remote_sources = _get_data().remote_sources
+    if source not in remote_sources:
+        remote_sources.append(source)
     files: list[dict[str, Any]] = []
 
     # ``repo_root`` is the directory containing ``.git`` and must be passed

esphome/components/store_yaml/__init__.py

@@ -3,12 +3,12 @@ from __future__ import annotations
 from collections.abc import Generator
 from dataclasses import dataclass
 import logging
-import os
 from pathlib import Path
 import struct
 
 from esphome import yaml_util
 import esphome.codegen as cg
+from esphome.components import packages
 from esphome.components.api import CONF_ENCRYPTION
 import esphome.config_validation as cv
 from esphome.const import CONF_API, CONF_ID, CONF_RAW_DATA_ID
@@ -27,9 +27,6 @@ CODEOWNERS = ["@bdraco"]
 DEPENDENCIES = ["api"]
 
 CONF_INCLUDE_SECRETS = "include_secrets"
-# Avoid an `_api:` substring in the key name so the integration-test harness
-# (which naively str-replaces `api:` to inject a port directive) doesn't
-# clobber configs that opt into this escape hatch.
 CONF_ALLOW_UNENCRYPTED = "allow_unencrypted"
 
 store_yaml_ns = cg.esphome_ns.namespace("store_yaml")
@@ -120,14 +117,11 @@ def _gather_files(
     entries: list[tuple[str, Path]] = []
     secret_rels: set[str] = set()
     for path in discovered.files:
-        try:
-            rel_str = path.relative_to(root).as_posix()
-        except ValueError:
-            # Outside the project root (e.g. ../common.yaml or a secrets file in
-            # $HOME). Use a relative path with ".." components instead of just
-            # the basename so the include graph is preserved and files from
-            # different directories with the same basename don't collide.
-            rel_str = os.path.relpath(path, root).replace(os.sep, "/")
+        # Files outside the project root (e.g. ../common.yaml or a secrets file
+        # in $HOME) keep their ".." components so the include graph is preserved
+        # and files from different directories with the same basename don't
+        # collide.
+        rel_str = path.relative_to(root, walk_up=True).as_posix()
 
         if path in discovered.secrets:
             secret_rels.add(rel_str)
@@ -151,26 +145,6 @@ def _read_files_verbatim(entries: list[tuple[str, Path]]) -> list[tuple[str, byt
     return files
 
 
-def _iter_sensitive_values(
-    node: object, path: tuple[str, ...] = ()
-) -> Generator[tuple[tuple[str, ...], str]]:
-    """Yield (config_path, value) for every cv.sensitive value in a config tree."""
-    if isinstance(node, yaml_util.SensitiveStr):
-        yield path, str(node)
-    elif isinstance(node, dict):
-        for key, value in node.items():
-            yield from _iter_sensitive_values(value, (*path, str(key)))
-    elif isinstance(node, (list, tuple)):
-        for item in node:
-            yield from _iter_sensitive_values(item, path)
-
-
-@dataclass
-class _SensitiveValue:
-    secret_name: str
-    config_path: str  # dotted path, for warnings (never log the value itself)
-
-
 def _iter_scalars(
     node: object, path: tuple[str, ...] = ()
 ) -> Generator[tuple[tuple[str, ...], object]]:
@@ -185,12 +159,27 @@ def _iter_scalars(
         yield path, node
 
 
+def _iter_sensitive_values(node: object) -> Generator[tuple[tuple[str, ...], str]]:
+    """Yield (config_path, value) for every cv.sensitive value in a config tree."""
+    for path, value in _iter_scalars(node):
+        if isinstance(value, yaml_util.SensitiveStr):
+            yield path, str(value)
+
+
+@dataclass
+class _SensitiveValue:
+    secret_name: str
+    config_path: str  # dotted path, for warnings (never log the value itself)
+
+
 def _warn_sensitive_collisions(sensitive: dict[str, _SensitiveValue]) -> None:
     """Redaction is value-keyed: any scalar equal to a sensitive value is
     rewritten to its `!secret` reference, including unrelated ones (e.g.
     `platform: esp32` when a password is literally "esp32"). Filling in a
     different value during recovery would then silently rewrite those
     unrelated scalars too — warn so the trap is documented, not silent."""
+    if not sensitive:
+        return
     for path, value in _iter_scalars(CORE.config):
         if isinstance(value, yaml_util.SensitiveStr):
             continue
@@ -256,43 +245,19 @@ def _uncaptured_note(
     return (UNCAPTURED_NOTE_PATH, "".join(parts).encode("utf-8"))
 
 
-def _find_remote_packages(entries: list[tuple[str, Path]]) -> list[str]:
-    """Describe every `packages:` entry that pulls content from a remote source.
+def _remote_package_descriptions() -> list[str]:
+    """Describe every remote source packages were fetched from.
 
-    Remote packages are downloaded during validation, which the fresh parse
-    used for discovery never reaches, so their files cannot be embedded. The
-    entry file still records the source, so the config is re-fetchable; this
-    only makes the gap visible instead of silent.
+    Remote packages are downloaded while the config is processed; the packages
+    component records each source, and this formats that record. Their files
+    cannot be embedded, but the entry file still records the package config, so
+    the config is re-fetchable; this only makes the gap visible instead of
+    silent.
     """
-    remote: list[str] = []
-    for _, path in entries:
-        try:
-            tree = yaml_util.load_yaml(path, clear_secrets=False)
-        except EsphomeError:
-            # Discovery already loaded this file once; a failure here would
-            # have been reported as a load_error and failed the build.
-            continue
-        if not isinstance(tree, dict):
-            continue
-        packages = tree.get("packages")
-        if isinstance(packages, dict):
-            candidates = packages.items()
-        elif isinstance(packages, list):
-            candidates = ((None, item) for item in packages)
-        else:
-            continue
-        for name, value in candidates:
-            desc = None
-            if isinstance(value, dict) and "url" in value:
-                url = value.get("url")
-                ref = value.get("ref")
-                desc = f"{url}@{ref}" if ref else str(url)
-            elif isinstance(value, str) and "//" in value:
-                # Shorthand form, e.g. `github://org/repo/file.yaml@main`
-                desc = value
-            if desc is not None:
-                remote.append(f"{name}: {desc}" if name is not None else desc)
-    return remote
+    return [
+        f"{source.url}@{source.ref}" if source.ref else source.url
+        for source in packages.get_remote_package_sources()
+    ]
 
 
 def _build_secrets_skeleton(keys: set[str]) -> bytes:
@@ -421,26 +386,23 @@ def unpack_envelope(blob: bytes) -> dict[str, bytes]:
         raise EsphomeError("envelope must start with EHY1 magic")
     pos = 4
     files: dict[str, bytes] = {}
-    try:
-        (count,) = struct.unpack_from("<I", blob, pos)
-        pos += 4
-        for _ in range(count):
-            (path_len,) = struct.unpack_from("<H", blob, pos)
-            pos += 2
-            if pos + path_len > len(blob):
-                raise EsphomeError("truncated envelope")
-            path = blob[pos : pos + path_len].decode("utf-8")
-            if path.startswith(("/", "\\")) or (len(path) >= 2 and path[1] == ":"):
-                raise EsphomeError(f"envelope contains non-relative path: {path}")
-            pos += path_len
-            (content_len,) = struct.unpack_from("<I", blob, pos)
-            pos += 4
-            if pos + content_len > len(blob):
-                raise EsphomeError("truncated envelope")
-            files[path] = blob[pos : pos + content_len]
-            pos += content_len
-    except struct.error as err:
-        raise EsphomeError(f"truncated envelope: {err}") from err
+
+    def take(n: int) -> bytes:
+        nonlocal pos
+        if pos + n > len(blob):
+            raise EsphomeError("truncated envelope")
+        chunk = blob[pos : pos + n]
+        pos += n
+        return chunk
+
+    (count,) = struct.unpack("<I", take(4))
+    for _ in range(count):
+        (path_len,) = struct.unpack("<H", take(2))
+        path = take(path_len).decode("utf-8")
+        if path.startswith(("/", "\\")) or (len(path) >= 2 and path[1] == ":"):
+            raise EsphomeError(f"envelope contains non-relative path: {path}")
+        (content_len,) = struct.unpack("<I", take(4))
+        files[path] = take(content_len)
     if pos != len(blob):
         raise EsphomeError("envelope has trailing bytes")
     return files
@@ -460,7 +422,7 @@ async def to_code(config: ConfigType) -> None:
         files = _read_files_verbatim(entries)
     else:
         files = _generate_redacted_files(entries, secret_rels)
-    remote_packages = _find_remote_packages(entries)
+    remote_packages = _remote_package_descriptions()
     if remote_packages:
         _LOGGER.warning(
             "store_yaml: %d package(s) come from remote sources and cannot be "

tests/components/store_yaml/test.bk72xx-ard.yaml

@@ -2,4 +2,5 @@ wifi:
   ssid: MySSID
   password: password1
 
-<<: !include common.yaml
+packages:
+  store_yaml: !include common.yaml

tests/components/store_yaml/test.esp32-idf.yaml

@@ -2,4 +2,5 @@ wifi:
   ssid: MySSID
   password: password1
 
-<<: !include common.yaml
+packages:
+  store_yaml: !include common.yaml

tests/components/store_yaml/test.esp8266-ard.yaml

@@ -2,4 +2,5 @@ wifi:
   ssid: MySSID
   password: password1
 
-<<: !include common.yaml
+packages:
+  store_yaml: !include common.yaml

tests/components/store_yaml/test.host.yaml

@@ -1,3 +1,4 @@
-<<: !include common.yaml
-
 network:
+
+packages:
+  store_yaml: !include common.yaml

tests/components/store_yaml/test.ln882x-ard.yaml

@@ -2,4 +2,5 @@ wifi:
   ssid: MySSID
   password: password1
 
-<<: !include common.yaml
+packages:
+  store_yaml: !include common.yaml

tests/components/store_yaml/test.rp2040-ard.yaml

@@ -2,4 +2,5 @@ wifi:
   ssid: MySSID
   password: password1
 
-<<: !include common.yaml
+packages:
+  store_yaml: !include common.yaml

tests/components/store_yaml/test.rtl87xx-ard.yaml

@@ -2,4 +2,5 @@ wifi:
   ssid: MySSID
   password: password1
 
-<<: !include common.yaml
+packages:
+  store_yaml: !include common.yaml

tests/integration/conftest.py

@@ -10,6 +10,7 @@ import logging
 import os
 from pathlib import Path
 import platform
+import re
 import signal
 import socket
 import subprocess
@@ -178,10 +179,9 @@ async def yaml_config(request: pytest.FixtureRequest, unused_tcp_port: int) -> s
     loop = asyncio.get_running_loop()
     content = await loop.run_in_executor(None, fixture_path.read_text)
 
-    # Replace the port in the config if it contains api section
-    if "api:" in content:
-        # Add port configuration after api:
-        content = content.replace("api:", f"api:\n  port: {unused_tcp_port}")
+    # Replace the port in the config if it contains an api section. Anchored to
+    # the start of a line so keys that merely end in "api:" are left alone.
+    content = re.sub(r"(?m)^api:", f"api:\n  port: {unused_tcp_port}", content)
 
     # Add debug build flags for integration tests to enable assertions
     if "esphome:" in content and "platformio_options:" not in content:

tests/integration/test_store_yaml_recovery.py

@@ -19,12 +19,8 @@ import contextlib
 
 import pytest
 
-try:
-    from compression import zstd  # type: ignore[import-not-found]
-except ImportError:
-    from backports import zstd  # type: ignore[import-not-found, no-redef]
-
-from esphome.components.store_yaml import unpack_envelope
+# The component resolves the stdlib-vs-backport zstd import once; reuse it.
+from esphome.components.store_yaml import unpack_envelope, zstd
 from esphome.yaml_util import find_secret_references
 
 from .types import RunCompiledFunction

tests/unit_tests/components/test_store_yaml.py

@@ -8,16 +8,17 @@ from pathlib import Path
 import pytest
 
 from esphome import yaml_util
+from esphome.components import packages
 from esphome.components.store_yaml import (
     CONF_ALLOW_UNENCRYPTED,
     SECRETS_SKELETON_HEADER,
     UNCAPTURED_NOTE_PATH,
     _final_validate,
-    _find_remote_packages,
     _gather_files,
     _generate_redacted_files,
     _pack_envelope,
     _read_files_verbatim,
+    _remote_package_descriptions,
     _uncaptured_note,
     unpack_envelope,
 )
@@ -290,40 +291,36 @@ def test_uncaptured_note_lists_remote_packages() -> None:
     """Remote packages that can't be captured are recorded with their source
     so the user knows to re-fetch them."""
     rel, content = _uncaptured_note(
-        [], ["base: https://github.com/org/repo@main", "github://org/repo/file.yaml"]
+        [], ["https://github.com/org/repo@main", "https://github.com/org/other"]
     )
     assert rel == UNCAPTURED_NOTE_PATH
     text = content.decode()
-    assert "#   base: https://github.com/org/repo@main" in text
-    assert "#   github://org/repo/file.yaml" in text
+    assert "#   https://github.com/org/repo@main" in text
+    assert "#   https://github.com/org/other" in text
 
 
-def test_find_remote_packages_detects_url_and_shorthand(project: Path) -> None:
-    """`packages:` entries with a url (dict or shorthand string) are reported;
-    local `!include` packages are not."""
-    (project / "entry.yaml").write_text(
-        "packages:\n"
-        "  base:\n"
-        "    url: https://github.com/org/repo\n"
-        "    ref: main\n"
-        "    files: [common.yaml]\n"
-        "  shorthand: github://org/repo/file.yaml@main\n"
-        "  local: !include wifi.yaml\n"
-        "esphome:\n  name: test\n"
+def test_remote_package_descriptions_read_packages_record(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Remote sources recorded by the packages component during config
+    processing are formatted as url@ref (url alone when ref is absent)."""
+    monkeypatch.delitem(CORE.data, packages.DOMAIN, raising=False)
+    data = packages._get_data()
+    data.remote_sources.append(
+        packages.RemotePackageSource("https://github.com/org/repo", "main")
     )
-    discovered = _sources(project, "entry.yaml", "wifi.yaml")
-    entries, _ = _gather_files(discovered)
-    remote = _find_remote_packages(entries)
-    assert remote == [
-        "base: https://github.com/org/repo@main",
-        "shorthand: github://org/repo/file.yaml@main",
+    data.remote_sources.append(
+        packages.RemotePackageSource("https://github.com/org/other", None)
+    )
+    assert _remote_package_descriptions() == [
+        "https://github.com/org/repo@main",
+        "https://github.com/org/other",
     ]
 
 
-def test_find_remote_packages_ignores_local_only(project: Path) -> None:
-    discovered = _sources(project, "entry.yaml", "wifi.yaml")
-    entries, _ = _gather_files(discovered)
-    assert _find_remote_packages(entries) == []
+def test_remote_package_descriptions_empty_without_packages() -> None:
+    CORE.data.pop(packages.DOMAIN, None)
+    assert _remote_package_descriptions() == []
 
 
 def test_redacted_skips_empty_sensitive_values(project: Path) -> None: