Skip to content

Derive the advertised mDNS hostname from dashboard_id, not the OS hostname - #1632

Merged
bdraco merged 4 commits into
mainfrom
stable-build-server-mdns-advert
Jun 20, 2026
Merged

Derive the advertised mDNS hostname from dashboard_id, not the OS hostname#1632
bdraco merged 4 commits into
mainfrom
stable-build-server-mdns-advert

Conversation

@bdraco

@bdraco bdraco commented Jun 20, 2026

Copy link
Copy Markdown
Member

What does this implement/fix?

The dashboard's peer-link mDNS SRV target was composed from socket.gethostname(), read live on every startup. macOS returns different values across reboots (mac one boot, macbook-pro the next), so the advertised hostname flipped even though the machine and its IP were unchanged; a paired peer saw the endpoint move and ran a needless rebind on every restart.

This derives the service-instance name and the SRV target purely from the persisted dashboard_id (esphome-builder-<id>.local), so the advertised identity is stable across reboots and the OS hostname can no longer flip it. The human machine label now travels in a new friendly_name TXT entry, surfaced on RemoteBuildPeer, so peers can still display the friendly name now that the instance name is an opaque identifier.

Existing pairings rebind once to the stable name on upgrade and then stay put; older receivers that don't broadcast friendly_name simply show the non-friendly identifier until they update.

Related issue or feature (if applicable):

  • fixes

Types of changes

  • Bugfix (non-breaking change which fixes an issue) — bugfix

Frontend coordination

Must land lockstep: name becomes an opaque esphome-builder-<id> identifier and the human label moves to the friendly_name TXT field. The frontend ships prebuilt inside our wheel, so the bundle in any release carrying this change must include esphome/device-builder-frontend#925, or the discovered-build-server picker shows the opaque id instead of the machine name.

Checklist

  • The code change is tested and works locally.
  • Pre-commit hooks pass (ruff, codespell, yaml/json/python checks).
  • Tests have been added or updated under tests/ where applicable.
  • components.index.json / definitions/components/*.json have not been hand-edited (regenerate via script/sync_components.py if a sync is needed).
  • Architecture-level changes are reflected in docs/ARCHITECTURE.md and/or docs/API.md.

@github-actions github-actions Bot added the bugfix Bug fix label Jun 20, 2026
@codspeed-hq

codspeed-hq Bot commented Jun 20, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 27 untouched benchmarks


Comparing stable-build-server-mdns-advert (b24c05c) with main (f4255cd)

Open in CodSpeed

@codecov

codecov Bot commented Jun 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.54%. Comparing base (f4255cd) to head (b24c05c).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1632      +/-   ##
==========================================
- Coverage   99.54%   99.54%   -0.01%     
==========================================
  Files         227      227              
  Lines       17955    17953       -2     
==========================================
- Hits        17874    17872       -2     
  Misses         81       81              
Flag Coverage Δ
py3.12 99.52% <100.00%> (-0.01%) ⬇️
py3.14 99.54% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...e_device_builder/controllers/remote_build/_mdns.py 100.00% <100.00%> (ø)
...ontrollers/remote_build/peer_link_client/client.py 98.66% <ø> (ø)
...home_device_builder/helpers/dashboard_advertise.py 100.00% <100.00%> (ø)
...me_device_builder/models/remote_build/offloader.py 95.23% <100.00%> (+0.07%) ⬆️
...ce_builder/models/remote_build/offloader_events.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@esphbot

esphbot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

Previous review — superseded by a newer review below.

@esphbot esphbot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking issues found.

  • TXT property table missing the new friendly_name entry
  • remote_build_host_added event payload doc omits friendly_name

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR stabilizes the dashboard’s peer-link mDNS identity by deriving the advertised service-instance name and SRV target from the persisted dashboard_id (instead of the OS hostname), and adds a separate friendly_name TXT entry so peers can still display a human-readable machine label.

Changes:

  • Derive the advertised mDNS hostname as esphome-builder-<dashboard_id[:8]>.local (stable across reboots/hostname changes).
  • Add friendly_name to mDNS TXT and propagate it through peer discovery (RemoteBuildPeer + TXT parsing).
  • Update tests and architecture docs to cover the new advertise/TXT behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_remote_build_listener.py Updates assertions to include the new friendly_name TXT key.
tests/test_remote_build_controller.py Extends service-info fakes and adds coverage for parsing friendly_name from TXT.
tests/test_dashboard_advertise.py Revises hostname construction tests to validate dashboard_id-derived stability and updated TXT contents.
esphome_device_builder/models/remote_build/offloader.py Adds friendly_name to the discovered peer model surfaced to the frontend.
esphome_device_builder/helpers/dashboard_advertise.py Implements stable hostname derivation and emits friendly_name in TXT.
esphome_device_builder/controllers/remote_build/_mdns.py Parses friendly_name from TXT into RemoteBuildPeer.
docs/ARCHITECTURE.md Documents the stable per-install mDNS identity and the new TXT field.

Comment thread esphome_device_builder/models/remote_build/offloader.py Outdated
…tname

The dashboard's peer-link mDNS SRV target was composed from
socket.gethostname(), read live on every startup. macOS returns
different values across reboots (mac vs macbook-pro), so the SRV target
flipped even though the machine and IP were unchanged; a paired peer saw
the endpoint move and ran a needless rebind on every restart.

Derive the service-instance name and SRV target purely from the
persisted dashboard_id (esphome-builder-<id>.local), so the advertised
identity is stable across reboots. Carry the human machine label in a
new friendly_name TXT entry, surfaced on RemoteBuildPeer, so peers can
still display it now that the instance name is an opaque identifier.
@bdraco
bdraco force-pushed the stable-build-server-mdns-advert branch from 93be4a9 to a2856a9 Compare June 20, 2026 21:01
Add friendly_name to the API.md TXT-property table and the
remote_build_host_added payload doc. Drop the instance-label and
SRV-target fallback branches that build_mdns_hostname's always-non-empty
return made unreachable.
@bdraco

bdraco commented Jun 20, 2026

Copy link
Copy Markdown
Member Author

Addressed in 69a158a:

  • 🟡 1 + 2 (docs/API.md): added friendly_name to the TXT-property table and to the remote_build_host_added payload doc (noting name is now the opaque esphome-builder-<id> identifier and friendly_name is the display label, "" for older receivers).
  • 🟢 1 (dead fallbacks): dropped both unreachable branches now that build_mdns_hostname always returns a non-empty name — self._name = host.split(".", 1)[0] and server derives straight from self._hostname — and removed the inaccurate fallback comment.

The event fires peer.to_dict(), so the TypedDict must carry every
RemoteBuildPeer field; friendly_name was missing.
@bdraco

bdraco commented Jun 20, 2026

Copy link
Copy Markdown
Member Author

@copilot good catch — fixed in c5c11cb. Added friendly_name: str to the RemoteBuildHostAddedData TypedDict so it matches the peer.to_dict() payload the event actually fires; tests/test_event_payload_contracts.py passes.

The advertised instance name and the peer-link SRV hostname are now
opaque per-install identifiers, so log lines that carried them read as
esphome-builder-<id>. Add the friendly_name to the advertise log and the
receiver label to every peer-link client log so operators can still tell
which peer a line refers to.
@bdraco
bdraco marked this pull request as ready for review June 20, 2026 21:18
@esphbot

esphbot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

PR Review — Derive the advertised mDNS hostname from dashboard_id, not the OS hostname

Solid, well-scoped fix; backend correctness and backward-compat are sound. One frontend-coordination gap is worth surfacing before merge.

Strengths:

  • Root cause fixed at the generator: build_mdns_hostname now derives the SRV target purely from the persisted dashboard_id, so the OS hostname can no longer flip the advertised identity across reboots. Verified the OS-hostname read is fully removed.
  • Backward compatibility for existing pairings is genuinely handled, not just asserted: the offloader rebind path (rebind.py:maybe_schedule_rebind_probeprobe_pairing_endpoint) matches on pin_sha256, not hostname, so a receiver whose SRV target moves to the new stable name is re-detected and rebinds once — exactly the described behavior.
  • The new _receiver_label log additions in peer_link_client/client.py are safe — verified the attribute is set in __init__ (client.py:184).
  • Earlier blocking review items (API.md TXT table, remote_build_host_added payload doc, RemoteBuildHostAddedData TypedDict) were addressed in follow-up commits.

Needs attention:

  • name becomes an opaque identifier and the human label moves to friendly_name; the discovered-host picker will show esphome-builder-<id> to users until the frontend follow-up lands. "No frontend change needed" understates this interim regression — recommend coupling the frontend PR.

🟡 Important

1. Discovered-host picker shows opaque IDs until the frontend follow-up lands
esphome_device_builder/models/remote_build/offloader.py:256

This PR changes RemoteBuildPeer.name (and the remote_build_host_added / hosts snapshot wire shape) from the human machine label (MacBook-Pro) to an opaque per-install identifier (esphome-builder-<id>), and moves the display label into the new friendly_name field.

The frontend currently keys and (per the architecture notes) renders the discovered-host row from name. Until the frontend is updated to read friendly_name, the build-server picker will display esphome-builder-jwywnve instead of MacBook-Pro to real users on upgrade — a visible UX regression that ships with this backend change.

The PR description marks "No frontend change needed" and treats the display swap as an optional follow-up. The mechanism is sound (the data is on the wire), but the interim degradation is user-facing, so I'd treat the frontend PR as a coupled deliverable rather than fully optional — at minimum land it close behind this one so users don't see opaque IDs in between.

(Unverified here: I could not inspect esphome/device-builder-frontend to confirm it renders name directly — worth a 1-minute check of the build-server picker component before merge.)

friendly_name: str = ""

Checklist

  • Root cause fixed at generator (mDNS hostname derivation)
  • Backward compatibility / pairing rebind on hostname change
  • New field reaches frontend via existing event/snapshot path
  • Public API / wire-contract docs updated
  • No new AttributeError from log-label additions
  • Frontend display contract for opaque name handled / coordinated — warning #1

Automated review by Kōan (Claude) HEAD=b24c05c 2 min 38s

@esphbot esphbot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking issues found.

  • Discovered-host picker shows opaque IDs until the frontend follow-up lands

@bdraco

bdraco commented Jun 20, 2026

Copy link
Copy Markdown
Member Author

Agreed — corrected the Frontend coordination section to couple esphome/device-builder-frontend#925 and flagged it as lockstep. Since the frontend ships prebuilt in the wheel, the release carrying this change must bundle #925, otherwise the discovered-host picker shows the opaque esphome-builder-<id> until then. #925 is already approved.

@bdraco
bdraco merged commit 9ae683a into main Jun 20, 2026
22 checks passed
@bdraco
bdraco deleted the stable-build-server-mdns-advert branch June 20, 2026 21:32
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 22, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants