Skip to content

Commit c853bce

Browse files
committed
fix: auto-backup silently skipped capture on several write paths
The pre-write auto-backup (#1288) silently skipped a snapshot whenever the id handed to a domain's fetch handler could not be resolved to the existing entity: the fetch returns None and maybe_snapshot skips with no error. Each affected path is fixed so the realistic caller id form resolves. - automation: automation_backup_target stripped the "automation." prefix, yielding a bare slug that _resolve_automation_id mis-treats as a numeric unique_id -> GET 404. The recommended python_transform edit path hit this on every UI-created automation. Pass the identifier unchanged so the resolver maps it to the real unique_id (also fixes a latent wrong-target restore the strip would have caused). - todo_item: _fetch_todo_item matched only on uid, but set/remove pass the item summary (the documented form). Match uid OR summary. - dashboard: _fetch_dashboard did not lazy-resolve HA's internal (underscored) dashboard id, so that form 404'd. Pre-resolve to the canonical url_path like the set/delete tools already do. - helper: _fetch_helper missed storage helpers after an entity_id rename (object_id != collection id). Fall back to the registry unique_id. Coverage gaps closed (the feature promises a snapshot before destructive writes, so the absence was a bug): - ha_remove_entity had no @with_auto_backup -> snapshot entity state before the registry delete. - ha_set_device / ha_remove_device had no coverage -> add a device DomainHandler (capture + restore of name_by_user/area_id/disabled_by/ labels) and decorate both tools. Regression tests added for every fetch path (todo summary-match, helper rename fallback, device fetch/restore, dashboard internal-id resolve, automation target prefix preservation, device handler registration).
1 parent 65dd338 commit c853bce

7 files changed

Lines changed: 766 additions & 67 deletions

File tree

src/ha_mcp/backup_manager.py

Lines changed: 108 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -735,19 +735,46 @@ async def _fetch_dashboard(client: Any, entity_id: str) -> Any:
735735
"""
736736
from fastmcp.exceptions import ToolError
737737

738-
from .tools.tools_config_dashboards import _get_dashboard_config_internal
738+
from .tools.tools_config_dashboards import (
739+
_get_dashboard_config_internal,
740+
_resolve_dashboard,
741+
)
742+
743+
# The set/delete tools accept BOTH the canonical hyphenated url_path
744+
# AND HA's internal (underscored) dashboard id, eagerly resolving the
745+
# latter before writing. ``_get_dashboard_config_internal`` does NOT
746+
# lazy-resolve, so an internal-id identifier 404s with "Unknown config
747+
# specified" and the pre-write snapshot is silently skipped. Pre-resolve
748+
# to the canonical url_path so capture works for whichever form the
749+
# caller passed (matching the form the write tool ultimately targets).
750+
fetch_path = entity_id
751+
try:
752+
match, _ = await _resolve_dashboard(client, entity_id)
753+
if match and match.get("url_path"):
754+
fetch_path = match["url_path"]
755+
except (HomeAssistantError, ToolError) as err:
756+
# Resolver failure (transport/shape) — fall through with the
757+
# original identifier; the canonical form is often already correct.
758+
logger.debug(
759+
"Auto-backup: dashboard resolve failed for %r: %s — using as-is",
760+
entity_id,
761+
err,
762+
)
739763

740764
try:
741-
config, _config_hash = await _get_dashboard_config_internal(client, entity_id)
765+
config, _config_hash = await _get_dashboard_config_internal(client, fetch_path)
742766
except ToolError as err:
743-
# ToolError carries the structured failure payload; treat
744-
# missing-dashboard responses as "entity doesn't exist yet".
767+
# ToolError carries the structured failure payload; treat a
768+
# missing/unknown dashboard as "nothing to back up" (also covers a
769+
# brand-new dashboard on the create path). "Unknown config
770+
# specified" is HA's message for an unresolved url_path.
745771
msg = str(err).lower()
746-
if "not_found" in msg or "config_not_found" in msg:
772+
if "not_found" in msg or "config_not_found" in msg or "unknown config" in msg:
747773
return None
748774
raise
749775
except HomeAssistantError as err:
750-
if "not_found" in str(err).lower() or "config_not_found" in str(err).lower():
776+
msg = str(err).lower()
777+
if "not_found" in msg or "config_not_found" in msg or "unknown config" in msg:
751778
return None
752779
raise
753780
return config
@@ -1009,8 +1036,11 @@ async def _restore_area_or_floor(client: Any, entity_id: str, config: Any) -> An
10091036

10101037

10111038
async def _fetch_todo_item(client: Any, entity_id: str) -> Any:
1012-
cal, _, uid = entity_id.partition("::")
1013-
if not cal or not uid:
1039+
# The second segment is whatever the tool's ``item`` param carried.
1040+
# ha_set_todo_item / ha_remove_todo_item accept EITHER the item uid OR
1041+
# its exact summary/name, so this can be either form.
1042+
cal, _, item_ref = entity_id.partition("::")
1043+
if not cal or not item_ref:
10141044
return None
10151045
payload = {
10161046
"type": "execute_script",
@@ -1037,7 +1067,11 @@ async def _fetch_todo_item(client: Any, entity_id: str) -> Any:
10371067
return None
10381068
items = result.get("response", {}).get("items", {}).get(cal, {}).get("items", [])
10391069
for item in items:
1040-
if item.get("uid") == uid:
1070+
# Match either form. Matching only on uid silently skipped the
1071+
# snapshot whenever the caller passed the human-readable summary
1072+
# (the documented/common case, e.g. ha_remove_todo_item(list, "Buy
1073+
# milk")) — uid != summary, so the loop found nothing -> None.
1074+
if item.get("uid") == item_ref or item.get("summary") == item_ref:
10411075
return {"todo_entity_id": cal, **item}
10421076
return None
10431077

@@ -1071,6 +1105,42 @@ async def _restore_entity_state(client: Any, entity_id: str, config: Any) -> Any
10711105
return await _rest_post(client, f"states/{entity_id}", payload)
10721106

10731107

1108+
# Devices — config/device_registry/{list,update}. ``ha_set_device`` mutates
1109+
# the user-editable registry fields (name_by_user / area_id / disabled_by /
1110+
# labels); restore re-applies exactly those. A device deleted by
1111+
# ``ha_remove_device`` cannot be recreated through the registry, so for that
1112+
# path the snapshot is an informational pre-delete record and restore is
1113+
# best-effort.
1114+
1115+
1116+
async def _fetch_device(client: Any, entity_id: str) -> Any:
1117+
items = await _ws_send(client, {"type": "config/device_registry/list"})
1118+
if not isinstance(items, list):
1119+
return None
1120+
for item in items:
1121+
if item.get("id") == entity_id:
1122+
return item
1123+
return None
1124+
1125+
1126+
async def _restore_device(client: Any, entity_id: str, config: Any) -> Any:
1127+
# Re-apply the captured registry state. Uses the same field NAMES as
1128+
# ``_update_device_internal`` but, unlike that partial-update path, always
1129+
# sends all four — restore reverts the device to the snapshot, so a
1130+
# captured ``None`` area/name is intentionally re-applied (cleared).
1131+
return await _ws_send(
1132+
client,
1133+
{
1134+
"type": "config/device_registry/update",
1135+
"device_id": entity_id,
1136+
"name_by_user": config.get("name_by_user"),
1137+
"area_id": config.get("area_id"),
1138+
"disabled_by": config.get("disabled_by"),
1139+
"labels": config.get("labels", []),
1140+
},
1141+
)
1142+
1143+
10741144
# Integration enable/disable — restore re-applies the disabled flag.
10751145

10761146

@@ -1140,6 +1210,34 @@ async def _fetch_helper(client: Any, entity_id: str, helper_type: str) -> Any:
11401210
for item in items:
11411211
if item.get("id") == object_id or item.get("id") == entity_id:
11421212
return item
1213+
# Fallback for renamed helpers: after an entity_id rename the object_id
1214+
# no longer equals the storage collection id (which stays the original
1215+
# create-time id == the registry unique_id), so the direct match above
1216+
# misses and the snapshot was silently skipped. Resolve the unique_id
1217+
# via the entity registry and match on that — the same key the helper
1218+
# update tool itself resolves to.
1219+
eid = entity_id if "." in entity_id else f"{helper_type}.{entity_id}"
1220+
try:
1221+
entry = await _ws_send(
1222+
client, {"type": "config/entity_registry/get", "entity_id": eid}
1223+
)
1224+
except HomeAssistantError as err:
1225+
# Only a genuine "entity not found" means there's nothing to back up;
1226+
# transport/auth/5xx errors must propagate so maybe_snapshot logs a
1227+
# WARNING rather than silently skipping. Same POLICY as _fetch_automation,
1228+
# but matched on the message substring because config/entity_registry/get
1229+
# failures arrive as a WS command error with no status_code to switch on.
1230+
# Best-effort: if HA's not-found wording ever changes, a real miss
1231+
# degrades to a WARNING + skip (never a swallowed fatal error).
1232+
msg = str(err).lower()
1233+
if "not_found" in msg or "not found" in msg:
1234+
return None
1235+
raise
1236+
unique_id = entry.get("unique_id") if isinstance(entry, dict) else None
1237+
if unique_id:
1238+
for item in items:
1239+
if str(item.get("id")) == str(unique_id):
1240+
return item
11431241
return None
11441242

11451243

@@ -1210,6 +1308,7 @@ def register_default_handlers(mgr: BackupManager, _client: Any) -> None:
12101308
)
12111309
mgr.register(DomainHandler("todo_item", _fetch_todo_item, _restore_todo_item))
12121310
mgr.register(DomainHandler("entity", _fetch_entity_state, _restore_entity_state))
1311+
mgr.register(DomainHandler("device", _fetch_device, _restore_device))
12131312
mgr.register(DomainHandler("integration", _fetch_integration, _restore_integration))
12141313
for helper_type in _KNOWN_HELPER_TYPES:
12151314
mgr.register(_make_helper_handler(helper_type))

src/ha_mcp/tools/auto_backup.py

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -75,17 +75,20 @@ def automation_backup_target(kw: dict[str, Any]) -> str:
7575
config_id = config.get("id")
7676
if config_id:
7777
return str(config_id)
78-
identifier = _resolve_str(kw.get("identifier"))
79-
# Strip the leading ``automation.`` prefix when the caller passed an
80-
# entity_id form (typical for ``python_transform`` calls that don't
81-
# carry a config body). Without this, snapshot files duplicate the
82-
# domain segment as ``automation.automation.<slug>.<ts>.yaml`` — the
83-
# body's entity_id keeps the prefix, only the filename / list key
84-
# tighten up. HA's automation upsert accepts either form for the
85-
# ``identifier`` param, so restore is unaffected.
86-
if identifier.startswith("automation."):
87-
identifier = identifier[len("automation.") :]
88-
return identifier
78+
# Return the identifier UNCHANGED — do NOT strip the ``automation.``
79+
# prefix. Capture and restore resolve the target through
80+
# ``client.get_automation_config`` -> ``_resolve_automation_id``, which
81+
# converts an entity_id ("automation.<slug>") to the real numeric
82+
# ``unique_id`` via a state lookup ONLY when the prefix is present;
83+
# otherwise it assumes the string already IS a unique_id. Stripping the
84+
# prefix produced a bare object_id slug that the resolver mis-treats as
85+
# a unique_id -> GET /config/automation/config/<slug> 404s -> the
86+
# pre-write snapshot is silently skipped (and, had it resolved, restore
87+
# would POST to the wrong key and create a stray automation). The
88+
# doubled domain segment in the snapshot filename
89+
# ("automation.automation.<slug>.<ts>.yaml") is purely cosmetic and is
90+
# exactly what the remove path (id_param="identifier") already produces.
91+
return _resolve_str(kw.get("identifier"))
8992

9093

9194
def with_auto_backup(

src/ha_mcp/tools/tools_entities.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1316,6 +1316,7 @@ async def _fetch_entity(eid: str) -> dict[str, Any]:
13161316
"title": "Remove Entity",
13171317
},
13181318
)
1319+
@with_auto_backup(domain="entity", id_param="entity_id", client=client)
13191320
@log_tool_usage
13201321
async def ha_remove_entity(
13211322
entity_id: Annotated[

src/ha_mcp/tools/tools_registry.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
from ..client.rest_client import HomeAssistantAPIError, HomeAssistantConnectionError
1616
from ..errors import ErrorCode, create_error_response
17+
from .auto_backup import with_auto_backup
1718
from .helpers import (
1819
exception_to_structured_error,
1920
log_tool_usage,
@@ -599,6 +600,7 @@ def get_device_info(device: dict[str, Any]) -> dict[str, Any]:
599600
tags={"Device Registry"},
600601
annotations={"destructiveHint": True, "title": "Set Device"},
601602
)
603+
@with_auto_backup(domain="device", id_param="device_id", client=client)
602604
@log_tool_usage
603605
async def ha_set_device(
604606
device_id: Annotated[
@@ -700,6 +702,7 @@ async def ha_set_device(
700702
"title": "Remove Device",
701703
},
702704
)
705+
@with_auto_backup(domain="device", id_param="device_id", client=client)
703706
@log_tool_usage
704707
async def ha_remove_device(
705708
device_id: Annotated[

0 commit comments

Comments
 (0)