Skip to content

Commit ec86ead

Browse files
Merge branch 'master' into feat/execute-code-tool
2 parents a3870c0 + 596a673 commit ec86ead

3 files changed

Lines changed: 156 additions & 0 deletions

File tree

site/src/data/tools.json

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1116,6 +1116,10 @@
11161116
"properties": {
11171117
"object_id": {
11181118
"type": "Annotated[str, Field(description=\"Group identifier without 'group.' prefix (e.g., 'living_room_lights')\")]"
1119+
},
1120+
"wait": {
1121+
"type": "Annotated[bool | str, Field(description='Wait for group to be fully removed before returning. Default: True.', default=True)]",
1122+
"default": true
11191123
}
11201124
},
11211125
"required": [
@@ -1163,6 +1167,10 @@
11631167
"remove_entities": {
11641168
"type": "Annotated[list[str] | None, Field(description='Remove these entities from an existing group (mutually exclusive with entities)', default=None)]",
11651169
"default": null
1170+
},
1171+
"wait": {
1172+
"type": "Annotated[bool | str, Field(description='Wait for group to be queryable before returning. Default: True. Set to False for bulk operations.', default=True)]",
1173+
"default": true
11661174
}
11671175
},
11681176
"required": [

src/ha_mcp/tools/tools_groups.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@
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 (
17+
coerce_bool_param,
18+
wait_for_entity_registered,
19+
wait_for_entity_removed,
20+
)
1621

1722
logger = logging.getLogger(__name__)
1823

@@ -133,6 +138,13 @@ async def ha_config_set_group(
133138
default=None,
134139
),
135140
] = None,
141+
wait: Annotated[
142+
bool | str,
143+
Field(
144+
description="Wait for group to be queryable before returning. Default: True. Set to False for bulk operations.",
145+
default=True,
146+
),
147+
] = True,
136148
) -> dict[str, Any]:
137149
"""
138150
Create or update a Home Assistant entity group.
@@ -225,12 +237,24 @@ async def ha_config_set_group(
225237
# Determine if this was a create or update based on fields provided
226238
is_create = entities is not None and name is None and add_entities is None and remove_entities is None
227239

240+
# Verify entity is queryable after creation/update
241+
wait_bool = coerce_bool_param(wait, "wait", default=True)
242+
result: dict[str, Any] = {}
243+
if wait_bool:
244+
try:
245+
registered = await wait_for_entity_registered(client, entity_id)
246+
if not registered:
247+
result["warning"] = f"Group created but {entity_id} not yet queryable. It may take a moment to become available."
248+
except Exception as e:
249+
result["warning"] = f"Group created but verification failed: {e}"
250+
228251
return {
229252
"success": True,
230253
"entity_id": entity_id,
231254
"object_id": object_id,
232255
"updated_fields": updated_fields,
233256
"message": f"Successfully {'created' if is_create else 'updated'} group: {entity_id}",
257+
**result,
234258
}
235259

236260
except ToolError:
@@ -253,6 +277,13 @@ async def ha_config_remove_group(
253277
description="Group identifier without 'group.' prefix (e.g., 'living_room_lights')"
254278
),
255279
],
280+
wait: Annotated[
281+
bool | str,
282+
Field(
283+
description="Wait for group to be fully removed before returning. Default: True.",
284+
default=True,
285+
),
286+
] = True,
256287
) -> dict[str, Any]:
257288
"""
258289
Remove a Home Assistant entity group.
@@ -285,11 +316,23 @@ async def ha_config_remove_group(
285316

286317
entity_id = f"group.{object_id}"
287318

319+
# Verify entity is removed
320+
wait_bool = coerce_bool_param(wait, "wait", default=True)
321+
result: dict[str, Any] = {}
322+
if wait_bool:
323+
try:
324+
removed = await wait_for_entity_removed(client, entity_id)
325+
if not removed:
326+
result["warning"] = f"Deletion confirmed by API but {entity_id} may still appear briefly."
327+
except Exception as e:
328+
result["warning"] = f"Deletion confirmed but removal verification failed: {e}"
329+
288330
return {
289331
"success": True,
290332
"entity_id": entity_id,
291333
"object_id": object_id,
292334
"message": f"Successfully removed group: {entity_id}",
335+
**result,
293336
}
294337

295338
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)