Add devices/clone command for fast same-config fleet provisioning#384
Merged
Conversation
Closes esphome/feature-requests#2851. Designed for "I bought 10 of the same bulb" workflows: the clone copies the source YAML's components and wiring intact but takes a fresh ``esphome.name`` (and therefore a fresh mDNS hostname / API endpoint), a fresh ``friendly_name``, and a freshly-generated ``api.encryption.key`` so two siblings forked from the same source don't share encryption material — compromise of one device must not compromise the others. Refactor: the three call sites that rewrite scalars in user YAML (``rewrite_esphome_name``, ``rewrite_friendly_name``, ``rewrite_api_encryption_key``) shared the same shape — walk the file line-by-line, find a key at a given parent path, swap the value preserving indentation / trailing comments. Consolidate that into a single ``rewrite_yaml_scalar(yaml_text, path, transform)`` walker driven by a callback; the three concrete helpers become thin wrappers (the encryption-key one keeps the indirection skip — ``!secret`` / ``${...}`` values stay shared with the source). The walker handles arbitrary mapping paths and correctly skips list-item nesting (``sensor: -name: foo`` does NOT satisfy ``("sensor", "name")``), so a future caller wanting ``logger.logs.api`` or ``substitutions.<key>`` gets the same machinery for free. ``clone_device`` itself stays small: two executor hops (one read phase that bundles the existence checks + source read + metadata read, one write phase that bundles the exclusive YAML write + metadata sidecar). No StorageJSON init — ``load_device_from_storage`` already handles a missing sidecar by reading the YAML, and the first compile writes a real one; the wizard-style stub-init that ``create_device`` does is belt-and-suspenders we don't need to duplicate. User-correctable failures (empty new_name, same-name as source, target collision, missing source) raise typed ``CommandError(INVALID_ARGS, ...)`` so the clone dialog can show specific messages instead of the WS layer's generic "Command failed" fallback. Pre-existing 1757 tests stay green; 13 new tests cover the generic walker, the two new wrappers, and the eight clone-path edge cases.
The clone command's encryption-key rewrite is a no-op when the source doesn't carry an ``api: encryption: key:`` block — pin that with two follow-up tests so a future regression that silently *adds* a fresh encryption block to a plaintext clone fails CI. Both shapes appear in real configs: a source with no ``api:`` block at all (ad-hoc / private-network device, no Home Assistant integration), and a source with ``api: password:`` (plaintext API, deliberately not encryption-upgraded). The user's choice to run plaintext is intentional; forcing encryption onto a clone would silently change the security posture. Closes a coverage gap raised on PR #384.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #384 +/- ##
==========================================
+ Coverage 98.82% 98.84% +0.02%
==========================================
Files 53 53
Lines 6023 6166 +143
==========================================
+ Hits 5952 6095 +143
Misses 71 71
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds backend support for cloning an existing device configuration to rapidly provision multiple devices with the same wiring/components but unique identity settings (hostname, friendly name, API encryption key), plus a refactor of YAML scalar rewriting into a reusable walker with new test coverage.
Changes:
- Introduces a new
devices/cloneWebSocket command that writes a new YAML file derived from an existing one, rewrites identity fields, and triggers a rescan. - Refactors YAML scalar rewriting into
rewrite_yaml_scalar()and adds helpers for friendly name and API encryption key rewrites, plus key generation. - Adds unit tests for the YAML rewrite helper behavior and end-to-end controller tests for clone success and validation errors.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
esphome_device_builder/controllers/devices/controller.py |
Adds the devices/clone command implementation (file creation, rewrites, metadata carry-forward, rescan). |
esphome_device_builder/helpers/yaml.py |
Adds rewrite_yaml_scalar, rewrite_friendly_name, rewrite_api_encryption_key, and generate_api_encryption_key; refactors name rewrite to use the generic walker. |
tests/controllers/devices/test_clone.py |
New test suite covering clone happy path and invalid-argument failures. |
tests/test_yaml_helpers.py |
Adds focused tests for the generic YAML scalar walker and the new rewrite/keygen helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9 tasks
Two real-world bugs the v1 implementation produced silently-wrong clones for. Both surfaced on review of esphome/feature-requests#2851: **1. ``esphome.name`` and the filename can drift apart.** A ``kitchen.yaml`` carrying ``esphome.name: my-kitchen-bulb`` (hand-edited or legacy from a prior rename) used to derive ``old_name`` from the filename and gate the rewrite on a value-match — silently no-oping when the two diverged. ``rewrite_esphome_name`` is now unconditional by default; ``_manual_rename`` opts back into the gated form via the new ``only_if_current=`` keyword. **2. The wizard's ``substitutions`` pattern broke.** ESPHome's standard wizard / ``dashboard_import`` shape pairs ``substitutions: { devicename: foo, friendly_name: Foo Bar }`` with ``esphome: { name: ${devicename}, friendly_name: ${friendly_name} }``. A literal rewrite of the leaf would silently orphan the substitution and break any other consumer of ``${devicename}`` in the same file. Add ``rewrite_name_or_substitution`` which detects pure ``$var`` / ``${var}`` references on the leaf and walks to ``substitutions.<var>`` instead. Mixed values (``${prefix}-suffix``) and substitutions defined in include files / packages fall through to the leaf rewrite. Two new shared primitives in ``helpers/yaml.py``: - ``read_yaml_scalar(yaml, path)`` — non-mutating sibling of ``rewrite_yaml_scalar``, returns the raw leaf value or ``None``. - ``parse_substitution_ref(value)`` — accepts ``$var`` / ``${var}`` optionally quoted; rejects mixed and malformed. 24 new tests cover the substitution-redirect path, the filename-drift case, the unconditional-replace contract, and the new primitives. 1789/1789 backend tests pass.
Three real bugs Copilot flagged in the YAML rewriter and clone
guards. Fix all three; add coverage for each.
**1. ``rewrite_yaml_scalar`` matched off-path nested keys.**
The walker only pushed *on-path* mapping keys onto its ancestor
stack. For path ``("api", "encryption", "key")``, YAML like
``api: { something: { encryption: { key: ... } } }`` would
falsely satisfy the path comparison because ``something`` was
invisible to the ancestor check. Push every mapping key (on-path
or not) so off-path branches show up in the ancestor chain and
the comparison fails correctly.
**2. ``.yaml`` vs ``.yml`` extension slipped past the same-name
guard.** ``configuration="kitchen.yml"`` + ``new_name="kitchen"``
produced ``new_filename="kitchen.yaml"``, which doesn't equal the
configuration string, so the same-name check passed — but both
files would carry ``esphome.name: kitchen`` and collide on mDNS
once the clone was flashed. Compare on stems instead of filenames.
**3. Friendly names with YAML-special characters got silently
truncated / split.** A friendly name like ``Bedroom #2`` written
as a plain scalar would round-trip as just ``Bedroom`` (everything
after `` #`` becomes a YAML comment); ``Lamp: Kitchen`` would
split into a key/value pair. Add ``_safe_yaml_scalar`` which emits
plain scalars when safe and double-quoted otherwise. Route
``rewrite_name_or_substitution`` and the encryption-key rewrite
through it.
DRY pass:
- Extract ``_strip_yaml_quotes`` shared by ``rewrite_esphome_name``
and ``parse_substitution_ref``.
- Drop the unused ``rewrite_friendly_name`` (clone now goes
through ``rewrite_name_or_substitution``).
22 new / updated tests pin the off-path soundness fix, the
``.yml`` collision rejection, the safe-quoting matrix, and the
``_strip_yaml_quotes`` helper. 1811/1811 backend tests pass.
Four lines from PR #384's diff weren't exercised by any test — codecov flagged them at 91-97% patch coverage. Add the four missing-coverage tests: - ``yaml.py:186`` — the inner ``stack.pop()`` inside the list-item branch fires when a second list item at the same indent appears with a deeper key already on the stack (``sensor: -name: x -name: y``). Pinned by walking a path that doesn't match anything, which keeps the walker running past the first list item without an early return. - ``yaml.py:193`` — the ``not m: continue`` branch for non-key non-list lines fires for block-scalar continuations (``comment: |`` content). Pinned by a YAML with a multi-line block scalar between two top-level keys. - ``controller.py:886`` — ``set_device_metadata`` carry-forward for the source's ``board_id``. Pinned with a real ``set_device_metadata`` call (not the stub fixture) and a ``get_device_metadata`` round-trip. - ``controller.py:890-896`` — the ``FileExistsError`` race- handling branch. Pinned by patching ``open`` to raise so the ``"x"`` mode's exclusive-create produces the typed error without needing real concurrency. 1815 tests pass (1811 + 4 new).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this implement/fix?
Adds a
devices/cloneWS command for fast same-config fleet provisioning. Closes esphome/feature-requests#2851.The use case: someone buys 10 of the same device (light bulbs, plugs, etc.), flashes the first one, and wants to spin up nine more configs that mirror the first but each have their own hostname / friendly name / API endpoint. Today they have to copy-paste the YAML manually and fix up identity fields by hand. After this change a single dialog produces a fresh, ready-to-flash YAML.
The clone copies the source's components and wiring intact but takes:
esphome.name(mDNS hostname / API endpoint),esphome.friendly_name(defaults to the slug-derived form ofnew_name; caller can override),api.encryption.keyso two siblings don't share encryption material — compromise of one device must not compromise the others.Indirections (
!secret api_key/${api_key}) are preserved deliberately — the indirection target is shared with the source on disk, so swapping the indirection name to a fresh literal would silently desync the rendered config.Refactor along the way: the three call sites that rewrite scalars in user YAML (
rewrite_esphome_name,rewrite_friendly_name,rewrite_api_encryption_key) shared the same shape — walk the file line-by-line, find a key at a given parent path, swap the value preserving indentation / trailing comments. Consolidated into a singlerewrite_yaml_scalar(yaml_text, path, transform)walker driven by a callback; the three concrete helpers are now thin wrappers. The walker handles arbitrary mapping paths and correctly skips list-item nesting (sensor: -name: foodoes NOT satisfy("sensor", "name")), so a future caller wantinglogger.logs.apiorsubstitutions.<key>gets the same machinery for free.Why line-based string edits instead of a YAML library
This came up on review, worth pinning the trade-off:
pyyaml(already a dep, used for reading YAML viaesphome.yaml_util) loses comments, blank lines, and quote style on round-trip. Useless for in-place edits to user-edited config — asafe_load→ mutate →safe_dumpwould clobber every comment and reflow the file.ruamel.yamldoes round-trip-preserving edits but isn't currently a dependency. Adopting it for the four targeted edits we do (esphome.name,friendly_name,api.encryption.key,substitutions.<var>) would be a much bigger commitment than the helpers it would replace, and even ruamel sometimes normalises quoting / indent / flow style.esphome/__main__.pycommand_rename—re.subagainst the file contents, with the same\$\{?name\}?$substitution-detection shape used here. It refuses "complex YAML" rather than handle every edge case.rewrite_yaml_scalaris a more general / more careful version of that exact pattern — path-aware (so off-path lookalikes don't false-match), list-item-aware (so leaves nested in a list can't satisfy a plain mapping path), and quote-state-aware (sofriendly_name: "Bedroom #2"doesn't mis-split at the#). All formatting / comments / quote style survives untouched, which matters because users hand-edit these configs and review the diffs after a clone.Implementation notes
clone_deviceruns only two executor hops total: one read phase that bundles the existence checks + source read + metadata read, one write phase that bundles the exclusive YAML write + metadata sidecar.load_device_from_storagealready handles a missing sidecar by reading the YAML, and the first compile writes a real one. The wizard-style stub-init thatcreate_devicedoes is belt-and-suspenders we don't need to duplicate for clones.new_name, same-name as source, target collision, missing source, source has no inlineesphome.name) raise typedCommandError(INVALID_ARGS, ...)so the clone dialog can show specific messages instead of the WS layer's generic "Command failed" fallback.Related issue or feature (if applicable):
Types of changes
bugfixnew-featureenhancementbreaking-changerefactordocsmaintenancecidependenciesFrontend coordination
Checklist
ruff,codespell, yaml/json/python checks).tests/where applicable.components.jsonhas not been hand-edited (regenerate viascript/sync_components.pyif a sync is needed).docs/ARCHITECTURE.mdand/ordocs/API.md.