Skip to content

Commit 73e8858

Browse files
Patch76claude
andauthored
fix: join entity registry in ha_config_list_helpers so renamed helpers show current entity_id/name (#1818)
* fix: join entity registry in ha_config_list_helpers so renamed helpers show current entity_id/name ha_config_list_helpers returned the {type}/list storage-collection response verbatim, which carries the immutable id (unique_id) and creation-time name and never consults the entity registry. After a UI rename the id/name go stale, and because ha_config_set_helper resolves a helper by constructing an entity_id from the passed id and reading the registry, feeding a stale id back in fails with ENTITY_NOT_FOUND – the current entity_id is the working key (#1794). Join config/entity_registry/list by unique_id + platform onto each record: add the current entity_id, move the storage name to original_name, and set name to the current display name (registry name, falling back to original_name). The storage id is kept for reference. Types without a matching entity (e.g. tag) and platform mismatches are left untouched; a failed registry read degrades open with a warning rather than dropping the list. The list now runs the {type}/list response through _flatten_helper_list_result first: person/list returns {"storage": [...], "config": [...]} rather than a flat list, which previously made the person count wrong (len of the two keys) and would have left the enrichment iterating dict keys. Other types are unaffected (a flat list flattens to itself). - unit: rename, null-name fallback, no-match, platform-mismatch, registry-failure, malformed-read, person-dict-flatten - e2e: the input_boolean list lifecycle now asserts the current entity_id is surfaced Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: harden list_helpers registry join per review Addresses the review on #1818: - Degrade-open the whole enrichment body, not just the WS call: the join lookups now run under a broad `except Exception` (mirroring `_get_entities_for_config_entry`), so an unexpected registry shape (e.g. a non-hashable id) flags the result instead of failing the list. Drops the unreachable `(HomeAssistantAPIError, ConnectionError, TimeoutError)` tuple — `send_websocket_message` returns `{"success": false}` rather than raising. - Add unit cases for the branch production takes (`{"success": false}`) and the unexpected-shape degrade (non-hashable id). - Add a dedicated e2e that renames the entity_id + name, then asserts the list surfaces the current registry values while storage keeps the creation-time id/original_name, and round-trips set_helper with the surfaced entity_id. - Point the stale-registry warning at ha_search (find by name) instead of ha_get_entity, which needs the entity_id the warning says is missing. - Log the malformed-response branch, and mark entity_id/original_name as conditional in the tool docstring (absent for tag and when the read degrades). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: reconcile registry join with component routing after rebase Rebasing onto master pulled in #1812's component-backed helpers_list path, which sits ahead of the legacy {type}/list body this PR enriches. Two test-side adjustments so the legacy join is exercised deterministically: - test_helper_list_registry_join: the mock client leaves base_url/token unset so get_component_caps returns None via its no-credentials guard and the tool takes the legacy path (rather than the component path short-circuiting the join under test). - test_ha_config_list_helpers_component_routing: the RoutingClient spy now serves config/entity_registry/list (the join's read) with a registry matching the canned rename shape, so the legacy path enriches to the same record the component path emits — keeping the two paths parity-equal — and the registry read is not tallied as a {type}/list fetch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: surface original_name on the component helpers_list path The rebase onto #1812 exposed a shape divergence: on component installs ha_config_list_helpers is served by the component path, whose shaped record carried id/entity_id/current name but not original_name — the field the legacy join adds and the tool docstring now promises. The new #1794 rename e2e caught it on the four E2E Validation lanes (component serves the list there), and the routing suite already asserts the two paths stay parity-equal. _shape_collection_helper_record starts from the storage body (config), whose name is the creation-time name (a rename updates the registry, not the body — #1794), then overrides name with the current display name. Preserve the body name as original_name before that override, mirroring the legacy join's additive shape, so both serving paths carry the same fields. Fixed at the shaper rather than the component because the creation name is already present in the body — no component change or version bump needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 96eb04e commit 73e8858

4 files changed

Lines changed: 576 additions & 8 deletions

File tree

src/ha_mcp/tools/tools_config_helpers.py

Lines changed: 110 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1399,6 +1399,85 @@ async def _find_collision_in_simple_helpers(
13991399
return None
14001400

14011401

1402+
_REGISTRY_JOIN_STALE_WARNING = (
1403+
"Could not read the entity registry, so a renamed helper's 'name' may be the "
1404+
"creation-time name and no current 'entity_id' is shown; use ha_search to find "
1405+
"the helper by name for the authoritative current values."
1406+
)
1407+
1408+
1409+
async def _enrich_helpers_with_current_registry(
1410+
client: Any, helper_type: str, items: list[Any]
1411+
) -> list[str]:
1412+
"""Join the entity registry onto storage-collection helper records.
1413+
1414+
The ``{helper_type}/list`` response carries the immutable storage ``id``
1415+
(the unique_id) and the creation-time ``name``; after a UI rename the
1416+
current ``entity_id`` and display name live only in the entity registry, so
1417+
the raw list goes stale (issue #1794). For each record matched by
1418+
``unique_id`` **and** ``platform == helper_type`` this adds the current
1419+
``entity_id``, moves the storage name to ``original_name``, and sets
1420+
``name`` to the current display name (registry ``name``, falling back to
1421+
``original_name``). ``items`` is mutated in place.
1422+
1423+
Storage types without a matching entity (e.g. ``tag``) and platform
1424+
mismatches are left untouched. Returns a warnings list — non-empty only
1425+
when the registry read failed, so the caller can flag the un-enriched
1426+
result instead of silently returning stale values.
1427+
"""
1428+
if not items:
1429+
# Nothing to enrich — skip the full-registry fetch entirely.
1430+
return []
1431+
# Degrade-open: enrichment is cosmetic, so any failure — the registry read,
1432+
# an unexpected registry shape (e.g. a non-hashable ``id`` breaking the
1433+
# lookup), or a malformed response — flags the result rather than raising
1434+
# out and letting the caller's handler turn a list call into a failure.
1435+
# Mirrors _get_entities_for_config_entry, which reads the same endpoint.
1436+
# send_websocket_message returns {"success": false, ...} instead of raising,
1437+
# so the malformed-response check below is the branch production takes.
1438+
try:
1439+
reg_result = await client.send_websocket_message(
1440+
{"type": "config/entity_registry/list"}
1441+
)
1442+
# A missing / non-list ``result`` (or a non-dict / unsuccessful response)
1443+
# is a malformed read, not an empty registry — flag it rather than
1444+
# silently returning un-enriched records. A present-but-empty ``[]`` is a
1445+
# legitimate no-match and passes through below without a warning.
1446+
if not (
1447+
isinstance(reg_result, dict)
1448+
and reg_result.get("success")
1449+
and isinstance(reg_result.get("result"), list)
1450+
):
1451+
logger.debug(
1452+
"list_helpers registry enrichment: malformed registry response: %r",
1453+
reg_result,
1454+
)
1455+
return [_REGISTRY_JOIN_STALE_WARNING]
1456+
registry = reg_result["result"]
1457+
reg_by_uid = {
1458+
entry["unique_id"]: entry
1459+
for entry in registry
1460+
if isinstance(entry, dict)
1461+
and entry.get("platform") == helper_type
1462+
and entry.get("unique_id")
1463+
}
1464+
for item in items:
1465+
if not isinstance(item, dict):
1466+
continue
1467+
entry = reg_by_uid.get(item.get("id"))
1468+
if entry is None:
1469+
continue
1470+
item["entity_id"] = entry.get("entity_id")
1471+
item["original_name"] = item.get("name")
1472+
item["name"] = (
1473+
entry.get("name") or entry.get("original_name") or item.get("name")
1474+
)
1475+
except Exception as e:
1476+
logger.debug(f"list_helpers registry enrichment failed: {e}")
1477+
return [_REGISTRY_JOIN_STALE_WARNING]
1478+
return []
1479+
1480+
14021481
async def _check_name_collision(
14031482
client: Any,
14041483
helper_type: str,
@@ -3391,6 +3470,14 @@ def _shape_collection_helper_record(rec: dict[str, Any]) -> dict[str, Any]:
33913470
out["entity_id"] = entity_id
33923471
name = rec.get("name")
33933472
if name is not None:
3473+
# The storage body's ``name`` is the creation-time name (a rename
3474+
# updates the registry, not the body — #1794); preserve it as
3475+
# ``original_name`` before the current display name overrides it, so a
3476+
# component-served record carries the same additive shape as the legacy
3477+
# join (both paths promise entity_id/original_name in the docstring).
3478+
original_name = out.get("name")
3479+
if original_name is not None:
3480+
out["original_name"] = original_name
33943481
out["name"] = name
33953482
return out
33963483

@@ -3538,11 +3625,19 @@ async def ha_config_list_helpers(
35383625
List all Home Assistant helpers of a specific type with their configurations.
35393626
35403627
Returns complete configuration for all helpers of the specified type including:
3541-
- ID (storage id), name, icon
3542-
- entity_id and current display name (where available)
3628+
- id (immutable storage key), entity_id (current — address the helper by
3629+
this, where available), name (current display name), original_name
3630+
(creation-time name), icon
35433631
- Type-specific settings (min/max for input_number, options for input_select, etc.)
35443632
- Area and label assignments
35453633
3634+
For a helper renamed in the UI, id/original_name keep the storage values while
3635+
entity_id/name reflect the current entity registry (entity_id is the identifier
3636+
ha_config_set_helper resolves against, so prefer it over id for a renamed helper).
3637+
entity_id/original_name are present only for storage-collection helpers matched in
3638+
the entity registry — types with no backing entity (e.g. tag), and every record when
3639+
the registry read degrades, carry only id/name (a warning flags the degraded case).
3640+
35463641
SUPPORTED HELPER TYPES:
35473642
- input_button: Virtual buttons for triggering automations
35483643
- input_boolean: Toggle switches/checkboxes
@@ -3602,14 +3697,25 @@ async def ha_config_list_helpers(
36023697
{"type": f"{helper_type}/list"}
36033698
)
36043699
if result.get("success"):
3605-
items = result.get("result", [])
3606-
return {
3700+
# Flatten first: person/list returns {"storage": [...],
3701+
# "config": [...]} rather than a flat list, so a raw
3702+
# result["result"] would be a dict here — breaking both the
3703+
# count and the registry enrichment below (which iterates
3704+
# records). _flatten_helper_list_result normalises both shapes.
3705+
items = _flatten_helper_list_result(result)
3706+
warnings = await _enrich_helpers_with_current_registry(
3707+
self._client, helper_type, items
3708+
)
3709+
response: dict[str, Any] = {
36073710
"success": True,
36083711
"helper_type": helper_type,
36093712
"count": len(items),
36103713
"helpers": items,
36113714
"message": f"Found {len(items)} {helper_type} helper(s)",
36123715
}
3716+
if warnings:
3717+
response["warnings"] = warnings
3718+
return response
36133719
raise_tool_error(
36143720
create_error_response(
36153721
ErrorCode.SERVICE_CALL_FAILED,

tests/src/e2e/workflows/config/test_helper_crud.py

Lines changed: 109 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,12 +97,17 @@ async def test_input_boolean_full_lifecycle(self, mcp_client, cleanup_tracker):
9797
)
9898
list_data = assert_mcp_success(list_result, "List after create")
9999

100-
found = False
100+
found_helper = None
101101
for helper in list_data.get("helpers", []):
102102
if helper.get("name") == helper_name:
103-
found = True
103+
found_helper = helper
104104
break
105-
assert found, f"Created helper not found in list: {helper_name}"
105+
assert found_helper, f"Created helper not found in list: {helper_name}"
106+
# #1794: the list joins the entity registry, so each record carries the
107+
# current entity_id (not just the storage id / creation name).
108+
assert found_helper.get("entity_id") == entity_id, (
109+
f"list_helpers should surface the current entity_id, got: {found_helper}"
110+
)
106111
logger.info("Input boolean verified in list")
107112

108113
# UPDATE
@@ -143,6 +148,107 @@ async def test_input_boolean_full_lifecycle(self, mcp_client, cleanup_tracker):
143148
)
144149
logger.info("Input boolean deletion verified")
145150

151+
async def test_renamed_helper_surfaces_current_registry_values(
152+
self, mcp_client, cleanup_tracker
153+
):
154+
"""#1794 regression: after a rename the storage collection keeps the
155+
creation-time id/name, while ha_config_list_helpers must surface the
156+
registry's current entity_id/name — and that surfaced entity_id must be a
157+
working key for ha_config_set_helper.
158+
159+
A dedicated test rather than an extension of the lifecycle case above:
160+
the rename swaps the entity_id, so folding it into that flow would leave
161+
its later update/delete steps pointing at the stale id.
162+
"""
163+
original_name = "E2E Rename Join"
164+
original_entity_id = "input_boolean.e2e_rename_join"
165+
new_entity_id = "input_boolean.e2e_rename_join_renamed"
166+
new_name = "E2E Rename Join Renamed"
167+
168+
# CREATE
169+
create_data = assert_mcp_success(
170+
await mcp_client.call_tool(
171+
"ha_config_set_helper",
172+
{"helper_type": "input_boolean", "name": original_name},
173+
),
174+
"Create input_boolean",
175+
)
176+
assert (
177+
get_entity_id_from_response(create_data, "input_boolean")
178+
== original_entity_id
179+
), f"unexpected creation entity_id: {create_data}"
180+
cleanup_tracker.track("input_boolean", new_entity_id)
181+
assert await wait_for_entity_registration(mcp_client, original_entity_id), (
182+
f"Entity not registered: {original_entity_id}"
183+
)
184+
185+
# RENAME entity_id + display name (diverges the registry from storage)
186+
assert_mcp_success(
187+
await mcp_client.call_tool(
188+
"ha_set_entity",
189+
{
190+
"entity_id": original_entity_id,
191+
"new_entity_id": new_entity_id,
192+
"name": new_name,
193+
},
194+
),
195+
"Rename input_boolean",
196+
)
197+
assert await wait_for_entity_registration(mcp_client, new_entity_id), (
198+
f"Renamed entity not registered: {new_entity_id}"
199+
)
200+
201+
# LIST — the join must surface the renamed entity_id/name; storage keeps
202+
# its creation-time id/original_name (the exact divergence #1794 is about).
203+
list_data = assert_mcp_success(
204+
await mcp_client.call_tool(
205+
"ha_config_list_helpers", {"helper_type": "input_boolean"}
206+
),
207+
"List after rename",
208+
)
209+
record = next(
210+
(
211+
h
212+
for h in list_data.get("helpers", [])
213+
if h.get("original_name") == original_name
214+
),
215+
None,
216+
)
217+
assert record, f"renamed helper not found by original_name: {list_data}"
218+
assert record.get("entity_id") == new_entity_id, (
219+
f"list must surface the renamed entity_id, got: {record}"
220+
)
221+
assert record.get("name") == new_name, (
222+
f"list must surface the current display name, got: {record}"
223+
)
224+
assert record.get("entity_id") != f"input_boolean.{record.get('id')}", (
225+
f"entity_id should diverge from the storage-id slug after rename: {record}"
226+
)
227+
228+
# ROUND-TRIP — the surfaced entity_id is the working key for set_helper.
229+
assert_mcp_success(
230+
await mcp_client.call_tool(
231+
"ha_config_set_helper",
232+
{
233+
"helper_type": "input_boolean",
234+
"helper_id": record["entity_id"],
235+
"icon": "mdi:check",
236+
},
237+
),
238+
"Update via surfaced entity_id",
239+
)
240+
logger.info("Round-tripped set_helper via the surfaced entity_id")
241+
242+
# CLEANUP
243+
await mcp_client.call_tool(
244+
"ha_remove_helpers_integrations",
245+
{
246+
"helper_type": "input_boolean",
247+
"target": new_entity_id,
248+
"confirm": True,
249+
},
250+
)
251+
146252
async def test_input_boolean_with_initial_state(self, mcp_client, cleanup_tracker):
147253
"""Test creating input_boolean with initial state."""
148254
logger.info("Testing input_boolean with initial state")

tests/src/unit/test_ha_config_list_helpers_component_routing.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,21 @@
5555
# exact #1794 shape (legacy list would emit only the storage id + stale name).
5656
_LEGACY_ITEMS = [{"id": "abc123", "name": "Old Name", "icon": "mdi:flash"}]
5757

58+
# Entity-registry entry the legacy path joins onto ``_LEGACY_ITEMS`` (#1794):
59+
# unique_id ``abc123`` on platform ``input_boolean`` currently lives at
60+
# ``input_boolean.foo`` with display name "New Name". Enriching the legacy list
61+
# with this yields the same record the component path emits, so the two paths
62+
# stay parity-equal instead of the legacy body degrading on an unhandled read.
63+
_LEGACY_REGISTRY = [
64+
{
65+
"entity_id": "input_boolean.foo",
66+
"unique_id": "abc123",
67+
"platform": "input_boolean",
68+
"name": "New Name",
69+
"original_name": "Old Name",
70+
}
71+
]
72+
5873
_CAPS_HELPERS = {
5974
"schema_version": 1,
6075
"component_version": "1.1.0",
@@ -106,8 +121,13 @@ def __init__(self) -> None:
106121
self.list_calls = 0
107122

108123
async def send_websocket_message(self, msg: dict[str, Any]) -> dict[str, Any]:
109-
self.list_calls += 1
110124
msg_type = msg.get("type", "")
125+
# The legacy body joins the entity registry (#1794); serve it, but don't
126+
# tally it as a {type}/list fetch — the counter tracks the helper-list
127+
# round-trip the routing assertions care about.
128+
if msg_type == "config/entity_registry/list":
129+
return {"success": True, "result": [dict(e) for e in _LEGACY_REGISTRY]}
130+
self.list_calls += 1
111131
if msg_type == "input_boolean/list":
112132
return {"success": True, "result": [dict(i) for i in _LEGACY_ITEMS]}
113133
if msg_type == "tag/list":

0 commit comments

Comments
 (0)