Skip to content

Commit 7263bda

Browse files
julienldclaude
andauthored
fix: validate label IDs in ha_manage_entity_labels to prevent silent failures (#486)
* fix: validate label IDs in ha_manage_entity_labels to prevent silent failures Add validation to ha_manage_entity_labels that checks if all label IDs exist before attempting assignment. This prevents the silent failure bug where the tool would report success but fail to apply non-existent labels AND hide all existing labels in the UI. Changes: - Add label ID validation before entity operations in ha_manage_entity_labels - Fetch available labels from label registry via WebSocket - Return clear error message listing non-existent label IDs - Provide helpful suggestions to use ha_config_get_label() or ha_config_set_label() - Add comprehensive E2E tests for label validation scenarios Fixes #475 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * refactor: improve robustness of label validation Address Gemini Code Assist review suggestions: - Add type checking for label list items to prevent AttributeError - Improve test assertions to verify exact invalid label lists - Ensure both helper tool suggestions are validated in tests Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 58957e9 commit 7263bda

27 files changed

Lines changed: 152 additions & 34 deletions

src/ha_mcp/tools/tools_labels.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,6 +675,46 @@ async def ha_manage_entity_labels(
675675
# Deduplicate labels
676676
parsed_labels = list(set(parsed_labels))
677677

678+
# Validate that all label IDs exist before attempting assignment
679+
if parsed_labels:
680+
try:
681+
# Fetch all available labels via WebSocket
682+
message: dict[str, Any] = {
683+
"type": "config/label_registry/list",
684+
}
685+
label_list_result = await client.send_websocket_message(message)
686+
687+
if not label_list_result.get("success"):
688+
return {
689+
"success": False,
690+
"error": "Failed to fetch available labels for validation",
691+
"suggestions": ["Check Home Assistant connection"],
692+
}
693+
694+
available_labels = label_list_result.get("result", [])
695+
available_label_ids = {lbl['label_id'] for lbl in available_labels if isinstance(lbl, dict) and 'label_id' in lbl}
696+
697+
# Check for non-existent labels
698+
invalid_labels = [lbl for lbl in parsed_labels if lbl not in available_label_ids]
699+
700+
if invalid_labels:
701+
return {
702+
"success": False,
703+
"error": f"Labels do not exist: {', '.join(invalid_labels)}",
704+
"invalid_labels": invalid_labels,
705+
"suggestions": [
706+
"Use ha_config_get_label() to see all available labels",
707+
"Create missing labels with ha_config_set_label()",
708+
],
709+
}
710+
except Exception as e:
711+
logger.error(f"Error validating label IDs: {e}")
712+
return {
713+
"success": False,
714+
"error": f"Failed to validate label IDs: {str(e)}",
715+
"suggestions": ["Check Home Assistant connection"],
716+
}
717+
678718
# Coerce parallel parameter
679719
parallel_bool = coerce_bool_param(parallel, "parallel", default=True)
680720
if parallel_bool is None:

tests/addon/test_addon_structure.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
import os
44
import stat
55
import yaml
6-
import pytest
76

87
try:
98
import tomllib # Python 3.11+

tests/initial_test_state/custom_components/hacs/base.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@
6161
from .repositories import REPOSITORY_CLASSES
6262
from .repositories.base import HACS_MANIFEST_KEYS_TO_EXPORT, REPOSITORY_KEYS_TO_EXPORT
6363
from .utils.file_system import async_exists
64-
from .utils.json import json_loads
6564
from .utils.logger import LOGGER
6665
from .utils.queue_manager import QueueManager
6766
from .utils.store import async_load_from_store, async_save_to_store

tests/initial_test_state/custom_components/hacs/data_client.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
from __future__ import annotations
44

5-
import asyncio
65
from typing import Any
76

87
from aiohttp import ClientSession, ClientTimeout

tests/initial_test_state/custom_components/hacs/repositories/base.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@
3434
from ..utils.decorator import concurrent
3535
from ..utils.file_system import async_exists, async_remove, async_remove_directory
3636
from ..utils.filters import filter_content_return_one_of_type
37-
from ..utils.github_graphql_query import GET_REPOSITORY_RELEASES
3837
from ..utils.json import json_loads
3938
from ..utils.logger import LOGGER
4039
from ..utils.path import is_safe

tests/initial_test_state/custom_components/hacs/repositories/integration.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ async def async_get_integration_manifest(self, ref: str = None) -> dict[str, Any
180180
else f"{self.content.path.remote}/{RepositoryFile.MAINIFEST_JSON}"
181181
)
182182

183-
if not manifest_path in (x.full_path for x in self.tree):
183+
if manifest_path not in (x.full_path for x in self.tree):
184184
raise HacsException(f"No {RepositoryFile.MAINIFEST_JSON} file found '{manifest_path}'")
185185

186186
response = await self.hacs.async_github_api_method(

tests/src/e2e/conftest.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,6 @@ def _ensure_hacs_frontend(initial_state_path: Path) -> None:
7474
HACS requires the frontend (~51MB) to be present to fully initialize.
7575
This is not committed to git to keep the repo size manageable.
7676
"""
77-
import subprocess
7877
import tarfile
7978
import urllib.request
8079

tests/src/e2e/workflows/automation/test_lifecycle.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
and production environments. Entity references are dynamically discovered.
99
"""
1010

11-
import asyncio
1211
import logging
1312

1413
import pytest

tests/src/e2e/workflows/automation/test_traces.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
Verifies that ha_get_automation_traces returns non-empty traces after automation runs.
66
"""
77

8-
import asyncio
98
import logging
109

1110
import pytest

tests/src/e2e/workflows/blueprints/test_blueprints.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
and production environments. Blueprint availability may vary.
1010
"""
1111

12-
import asyncio
1312
import logging
1413

1514
import pytest
@@ -380,7 +379,7 @@ async def test_blueprint_automation_lifecycle(mcp_client):
380379
error_msg = str(create_result.get("error", {}).get("message", ""))
381380
# If error is about missing blueprint inputs, our validation passed! HA rejected it.
382381
if "Missing input" in error_msg or "input" in error_msg.lower():
383-
logger.info(f"✅ Our validation passed (config reached HA), HA rejected due to missing blueprint inputs as expected")
382+
logger.info("✅ Our validation passed (config reached HA), HA rejected due to missing blueprint inputs as expected")
384383
logger.info("✅ Blueprint automation lifecycle test completed (validation works)")
385384
return
386385
# If error is about missing trigger/action, our fix didn't work

0 commit comments

Comments
 (0)