Skip to content

Commit c35ca30

Browse files
committed
Merge branch 'pr-853' into addon-repo
2 parents 98e9cb9 + 89e68ea commit c35ca30

2 files changed

Lines changed: 126 additions & 0 deletions

File tree

src/ha_mcp/tools/tools_groups.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
from ..errors import ErrorCode, create_error_response
1515
from .helpers import exception_to_structured_error, log_tool_usage, raise_tool_error
16+
from .util_helpers import wait_for_entity_registered, wait_for_entity_removed
1617

1718
logger = logging.getLogger(__name__)
1819

@@ -225,12 +226,22 @@ async def ha_config_set_group(
225226
# Determine if this was a create or update based on fields provided
226227
is_create = entities is not None and name is None and add_entities is None and remove_entities is None
227228

229+
# Verify entity is queryable after creation/update
230+
result: dict[str, Any] = {}
231+
try:
232+
registered = await wait_for_entity_registered(client, entity_id)
233+
if not registered:
234+
result["warning"] = f"Group created but {entity_id} not yet queryable. It may take a moment to become available."
235+
except Exception as e:
236+
result["warning"] = f"Group created but verification failed: {e}"
237+
228238
return {
229239
"success": True,
230240
"entity_id": entity_id,
231241
"object_id": object_id,
232242
"updated_fields": updated_fields,
233243
"message": f"Successfully {'created' if is_create else 'updated'} group: {entity_id}",
244+
**result,
234245
}
235246

236247
except ToolError:
@@ -285,11 +296,21 @@ async def ha_config_remove_group(
285296

286297
entity_id = f"group.{object_id}"
287298

299+
# Verify entity is removed
300+
result: dict[str, Any] = {}
301+
try:
302+
removed = await wait_for_entity_removed(client, entity_id)
303+
if not removed:
304+
result["warning"] = f"Deletion confirmed by API but {entity_id} may still appear briefly."
305+
except Exception as e:
306+
result["warning"] = f"Deletion confirmed but removal verification failed: {e}"
307+
288308
return {
289309
"success": True,
290310
"entity_id": entity_id,
291311
"object_id": object_id,
292312
"message": f"Successfully removed group: {entity_id}",
313+
**result,
293314
}
294315

295316
except ToolError:
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
"""
2+
Regression tests for group config tool post-operation verification.
3+
4+
Verifies that group tools confirm entity state after service calls,
5+
preventing the false-success anti-pattern where the tool returns
6+
success before the entity is actually queryable.
7+
"""
8+
9+
import logging
10+
11+
import pytest
12+
13+
from ...utilities.assertions import assert_mcp_success, safe_call_tool
14+
15+
logger = logging.getLogger(__name__)
16+
17+
18+
@pytest.mark.group
19+
class TestGroupVerification:
20+
"""Verify that group operations confirm entity state."""
21+
22+
async def test_created_group_is_immediately_queryable(
23+
self, mcp_client, cleanup_tracker
24+
):
25+
"""After ha_config_set_group succeeds, the entity must be queryable.
26+
27+
Regression test: before verification was added, the tool returned
28+
success with a predicted entity_id that might not exist yet.
29+
"""
30+
object_id = "test_e2e_verify_create"
31+
32+
result = await mcp_client.call_tool(
33+
"ha_config_set_group",
34+
{
35+
"object_id": object_id,
36+
"name": "Verification Test Group",
37+
"entities": ["light.bed_light"],
38+
},
39+
)
40+
41+
data = assert_mcp_success(result, "Create group")
42+
entity_id = data.get("entity_id")
43+
assert entity_id == f"group.{object_id}"
44+
cleanup_tracker.track("group", object_id)
45+
46+
# The entity must be queryable immediately after the tool returns
47+
state_result = await mcp_client.call_tool(
48+
"ha_get_state", {"entity_id": entity_id}
49+
)
50+
state_data = assert_mcp_success(state_result, "Get group state after create")
51+
# Response may nest entity data under "data" key
52+
inner = state_data.get("data", state_data)
53+
assert inner.get("entity_id") == entity_id, (
54+
f"Created group not queryable immediately after tool returned success: {state_data}"
55+
)
56+
logger.info(f"Group {entity_id} confirmed queryable after create")
57+
58+
# Cleanup
59+
await mcp_client.call_tool(
60+
"ha_config_remove_group", {"object_id": object_id}
61+
)
62+
63+
async def test_removed_group_is_immediately_gone(
64+
self, mcp_client, cleanup_tracker
65+
):
66+
"""After ha_config_remove_group succeeds, the entity must be gone.
67+
68+
Regression test: before verification was added, the tool returned
69+
success but the entity could still be queryable briefly.
70+
"""
71+
object_id = "test_e2e_verify_remove"
72+
73+
# Create a group first
74+
result = await mcp_client.call_tool(
75+
"ha_config_set_group",
76+
{
77+
"object_id": object_id,
78+
"name": "Removal Verification Group",
79+
"entities": ["light.bed_light"],
80+
},
81+
)
82+
assert_mcp_success(result, "Create group for removal test")
83+
cleanup_tracker.track("group", object_id)
84+
85+
# Remove it
86+
remove_result = await mcp_client.call_tool(
87+
"ha_config_remove_group", {"object_id": object_id}
88+
)
89+
assert_mcp_success(remove_result, "Remove group")
90+
91+
# The entity must NOT be queryable after the tool returns
92+
entity_id = f"group.{object_id}"
93+
state_data = await safe_call_tool(
94+
mcp_client, "ha_get_state", {"entity_id": entity_id}
95+
)
96+
# Should either fail or return not_found
97+
is_gone = (
98+
not state_data.get("success")
99+
or state_data.get("state") == "unavailable"
100+
or "not found" in str(state_data).lower()
101+
)
102+
assert is_gone, (
103+
f"Removed group still queryable after tool returned success: {state_data}"
104+
)
105+
logger.info(f"Group {entity_id} confirmed gone after remove")

0 commit comments

Comments
 (0)