Skip to content

Commit 8e96914

Browse files
committed
refactor: address PR #1397 KP13 second-pass review (D4 available_*_ids parity)
Extend the #1297 RESOURCE_NOT_FOUND classifier audit to the three remaining sites flagged in KP13's second-pass review — same audit family as the labels / categories / zones / devices sibling sites already covered in this PR: - ``ha_config_delete_dashboard`` (tools_config_dashboards.py): reuse the dashboard list returned by ``_resolve_dashboard`` (previously discarded via ``_``) and splice first-10 ``url_path`` entries into ``available_dashboard_ids``. - ``_get_automation_config_internal`` (tools_config_automations.py): new central not-found raise — catches ``HomeAssistantAPIError(status_code=404)`` from the REST client and emits ``RESOURCE_NOT_FOUND`` with ``available_automation_ids`` sourced from a best-effort ``config/entity_registry/list`` filtered by ``automation.`` prefix. Other ``HomeAssistantAPIError`` instances propagate unchanged. - ``_fetch_script_config_envelope`` (tools_config_scripts.py): new private helper mirroring the automations shape, used by both ``ha_config_get_script`` and ``_get_script_config_internal`` to centralize the 404→RESOURCE_NOT_FOUND translation. ``available_script_ids`` populated the same way. Both list-fetch helpers (``_list_automation_entity_ids`` / ``_list_script_entity_ids``) are best-effort: a registry-list failure yields an empty list rather than masking the 404 behind a registry error. 5 new regression tests in ``tests/src/unit/test_error_code_consistency_1297.py`` pin the new shape at sibling depth — error code + list-tool suggestion + ``available_*_ids`` payload, plus the empty-list-on-registry-failure fallback for the automations and scripts paths.
1 parent 466ce2c commit 8e96914

4 files changed

Lines changed: 357 additions & 5 deletions

File tree

src/ha_mcp/tools/tools_config_automations.py

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from pydantic import Field
1414

1515
from ..client.rest_client import (
16+
HomeAssistantAPIError,
1617
HomeAssistantAuthError,
1718
HomeAssistantConnectionError,
1819
)
@@ -838,15 +839,68 @@ async def ha_config_set_automation(
838839
suggestions=suggestions,
839840
)
840841

842+
async def _list_automation_entity_ids(self) -> list[str]:
843+
"""Best-effort list of automation entity_ids from the entity registry.
844+
845+
Used to populate ``available_automation_ids`` in RESOURCE_NOT_FOUND
846+
error context. Returns an empty list on any failure — caller treats
847+
absence as "no IDs to report" rather than failing the structured
848+
error raise.
849+
"""
850+
try:
851+
result = await self._client.send_websocket_message(
852+
{"type": "config/entity_registry/list"}
853+
)
854+
except Exception as e:
855+
logger.debug(
856+
"Failed to list automation entity_ids from registry: %s", e
857+
)
858+
return []
859+
entries = result.get("result", []) if isinstance(result, dict) else result
860+
if not isinstance(entries, list):
861+
return []
862+
return [
863+
entry["entity_id"]
864+
for entry in entries
865+
if isinstance(entry, dict)
866+
and isinstance(entry.get("entity_id"), str)
867+
and entry["entity_id"].startswith("automation.")
868+
]
869+
841870
async def _get_automation_config_internal(
842871
self, identifier: str
843872
) -> tuple[dict[str, Any], str]:
844873
"""Fetch and normalize automation config without logging or category injection.
845874
846875
Returns (normalized_config, config_hash) tuple.
847876
Used internally by _fetch_and_verify_hash and ha_config_get_automation.
877+
878+
Raises a structured ``RESOURCE_NOT_FOUND`` ToolError when the REST
879+
client returns 404, populating ``available_automation_ids`` so
880+
agents can recover without a separate search round-trip. Other
881+
``HomeAssistantAPIError`` instances propagate unchanged to caller
882+
exception handlers (``exception_to_structured_error`` route).
848883
"""
849-
config_result = await self._client.get_automation_config(identifier)
884+
try:
885+
config_result = await self._client.get_automation_config(identifier)
886+
except HomeAssistantAPIError as e:
887+
if e.status_code == 404:
888+
available_ids = await self._list_automation_entity_ids()
889+
raise_tool_error(
890+
create_error_response(
891+
ErrorCode.RESOURCE_NOT_FOUND,
892+
f"Automation not found: {identifier}",
893+
context={
894+
"automation_id": identifier,
895+
"available_automation_ids": available_ids[:10],
896+
},
897+
suggestions=[
898+
"Use ha_search_entities(domain_filter='automation') to find existing automations",
899+
"Verify the entity_id or unique_id is correct",
900+
],
901+
)
902+
)
903+
raise
850904
normalized_config = _normalize_config_for_roundtrip(config_result)
851905
config_hash_value = compute_config_hash(normalized_config)
852906
return normalized_config, config_hash_value

src/ha_mcp/tools/tools_config_dashboards.py

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1530,8 +1530,13 @@ async def ha_config_delete_dashboard(
15301530
],
15311531
context={"action": "delete"},
15321532
)
1533-
resolved, _ = await _resolve_dashboard(client, url_path)
1533+
resolved, dashboards = await _resolve_dashboard(client, url_path)
15341534
if resolved is None:
1535+
available_ids = [
1536+
d.get("url_path")
1537+
for d in (dashboards or [])[:10]
1538+
if d.get("url_path")
1539+
]
15351540
raise_tool_error(
15361541
create_error_response(
15371542
ErrorCode.RESOURCE_NOT_FOUND,
@@ -1541,7 +1546,11 @@ async def ha_config_delete_dashboard(
15411546
"Use ha_config_get_dashboard(list_only=True) to see available dashboards",
15421547
"YAML-mode and default dashboards are not deletable via this tool",
15431548
],
1544-
context={"action": "delete", "url_path": url_path},
1549+
context={
1550+
"action": "delete",
1551+
"url_path": url_path,
1552+
"available_dashboard_ids": available_ids,
1553+
},
15451554
)
15461555
)
15471556
resolved_id = resolved["id"]

src/ha_mcp/tools/tools_config_scripts.py

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from pydantic import Field
1414

1515
from ..client.rest_client import (
16+
HomeAssistantAPIError,
1617
HomeAssistantAuthError,
1718
HomeAssistantConnectionError,
1819
)
@@ -122,7 +123,7 @@ async def ha_config_get_script(
122123
"Use ha_search_entities(domain_filter='script') to list scripts",
123124
],
124125
)
125-
config_result = await self._client.get_script_config(script_id)
126+
config_result = await self._fetch_script_config_envelope(script_id)
126127
# Extract actual script config body and compute hash before category injection
127128
actual_config = config_result.get("config", config_result)
128129
config_hash_value = compute_config_hash(actual_config)
@@ -170,6 +171,65 @@ async def ha_config_get_script(
170171
],
171172
)
172173

174+
async def _list_script_entity_ids(self) -> list[str]:
175+
"""Best-effort list of script entity_ids from the entity registry.
176+
177+
Used to populate ``available_script_ids`` in RESOURCE_NOT_FOUND
178+
error context. Returns an empty list on any failure — caller
179+
treats absence as "no IDs to report" rather than failing the
180+
structured error raise.
181+
"""
182+
try:
183+
result = await self._client.send_websocket_message(
184+
{"type": "config/entity_registry/list"}
185+
)
186+
except Exception as e:
187+
logger.debug("Failed to list script entity_ids from registry: %s", e)
188+
return []
189+
entries = result.get("result", []) if isinstance(result, dict) else result
190+
if not isinstance(entries, list):
191+
return []
192+
return [
193+
entry["entity_id"]
194+
for entry in entries
195+
if isinstance(entry, dict)
196+
and isinstance(entry.get("entity_id"), str)
197+
and entry["entity_id"].startswith("script.")
198+
]
199+
200+
async def _fetch_script_config_envelope(self, script_id: str) -> dict[str, Any]:
201+
"""Fetch the raw REST envelope, mapping 404 to RESOURCE_NOT_FOUND.
202+
203+
Returns the dict envelope from ``rest_client.get_script_config``
204+
(``success``/``script_id``/``config`` keys). Raises a structured
205+
``RESOURCE_NOT_FOUND`` ToolError when the REST client returns 404,
206+
populating ``available_script_ids`` so agents can recover without
207+
a separate search round-trip. Other ``HomeAssistantAPIError``
208+
instances propagate unchanged to caller exception handlers.
209+
"""
210+
try:
211+
return cast(
212+
dict[str, Any], await self._client.get_script_config(script_id)
213+
)
214+
except HomeAssistantAPIError as e:
215+
if e.status_code == 404:
216+
available_ids = await self._list_script_entity_ids()
217+
raise_tool_error(
218+
create_error_response(
219+
ErrorCode.RESOURCE_NOT_FOUND,
220+
f"Script not found: {script_id}",
221+
context={
222+
"script_id": script_id,
223+
"available_script_ids": available_ids[:10],
224+
},
225+
suggestions=[
226+
"Use ha_search_entities(domain_filter='script') to find existing scripts",
227+
"Verify the script identifier is correct",
228+
],
229+
)
230+
)
231+
raise
232+
173233
async def _get_script_config_internal(
174234
self, script_id: str
175235
) -> tuple[dict[str, Any], str]:
@@ -178,8 +238,11 @@ async def _get_script_config_internal(
178238
Returns (actual_config, config_hash) tuple where actual_config is
179239
the inner script body (not the REST wrapper).
180240
Used internally by _fetch_and_verify_hash and ha_config_get_script.
241+
242+
404 responses from the REST client are mapped to a structured
243+
``RESOURCE_NOT_FOUND`` ToolError via ``_fetch_script_config_envelope``.
181244
"""
182-
config_result = await self._client.get_script_config(script_id)
245+
config_result = await self._fetch_script_config_envelope(script_id)
183246
actual_config = config_result.get("config", config_result)
184247
config_hash_value = compute_config_hash(actual_config)
185248
return actual_config, config_hash_value

0 commit comments

Comments
 (0)