Skip to content

Commit 17b49c0

Browse files
swissmoclaude
andcommitted
fix(bulk): make selector mode actually reachable after a group-safety rejection
Live-tested (2026-08-23): the group-safety rejection correctly blocked the dangerous batch, but the calling model then failed 6+ times trying to recover via selector mode and gave up entirely -- a net worse outcome than before, even though the dangerous action was correctly prevented. Root cause was two separate, pre-existing gaps the model hit hard once the operations-mode escape hatch was no longer available: 1. It put exclude_entity_ids as a top-level argument instead of nested inside selector, and the rejection message only said "use selector mode with exclude_entity_ids" with no worked example to correct that. 2. It passed area display names (from ha_search's friendly area_names, and a floor's display name) as area_ids, which must be exact HA registry IDs -- the "unknown area_ids" error never said so, or how to find the real ones. Both error messages now show a concrete, correctly-shaped example and point to ha_list_floors_areas (the existing tool for exact area_id/floor_id lookup) instead of just naming the problem. Message-only changes, no behavior change to the group-safety check itself -- it continues to block the exact batch shape confirmed live to cascade into an excluded entity. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 9d66aa3 commit 17b49c0

4 files changed

Lines changed: 60 additions & 4 deletions

File tree

src/ha_mcp/tools/bulk_selector.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -445,12 +445,23 @@ def _select_area_ids(
445445
unknown_areas = sorted(set(area_ids) - set(areas))
446446
unknown_floors = sorted(set(floor_ids) - floors)
447447
if unknown_areas or unknown_floors:
448+
# A caller that only has display names (e.g. from ha_search's
449+
# friendly area_names, or a floor's display name) will otherwise
450+
# retry with the same wrong value indefinitely -- area_ids/floor_ids
451+
# are exact HA registry IDs (usually a lowercase, underscored slug
452+
# like "living_room"), not the names shown in the UI, and nothing
453+
# else in this error says where to find the real ones.
448454
details = []
449455
if unknown_areas:
450456
details.append(f"unknown area_ids: {', '.join(unknown_areas)}")
451457
if unknown_floors:
452458
details.append(f"unknown floor_ids: {', '.join(unknown_floors)}")
453-
raise BulkSelectorValidationError("; ".join(details))
459+
raise BulkSelectorValidationError(
460+
"; ".join(details) + ". area_ids/floor_ids must be exact Home "
461+
"Assistant registry IDs, not display names shown in the UI or "
462+
"returned as ha_search's 'area_names' -- call ha_list_floors_areas "
463+
"to look up the exact area_id/floor_id for each area or floor."
464+
)
454465
selected = set(area_ids)
455466
selected.update(
456467
area_id for area_id, row in areas.items() if row.get("floor_id") in floor_ids

src/ha_mcp/tools/tools_service.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -281,8 +281,12 @@ async def _reject_operations_group_member_conflicts(
281281
"it from the group's own action. Target ONLY the group, or "
282282
"ONLY the specific member(s) you want affected, never both in "
283283
"the same call. To act on most of a group while excluding "
284-
"specific members, use ha_bulk_control's selector mode with "
285-
"exclude_entity_ids instead.",
284+
"specific members, use selector mode instead: exclude_entity_ids "
285+
"goes INSIDE selector, not as a top-level argument, e.g. "
286+
'{"selector": {"domain": "light", "area_ids": ["<area_id>"], '
287+
'"exclude_entity_ids": ["<entity_to_skip>"]}, "action": "off"}. '
288+
"area_ids/floor_ids must be exact registry IDs (call "
289+
"ha_list_floors_areas to look them up), not display names.",
286290
parameter="operations",
287291
)
288292
)

tests/src/unit/test_bulk_selector.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,3 +1046,28 @@ async def test_invalid_structural_ids_fail_closed(
10461046
timeout_seconds=None,
10471047
validate_first=True,
10481048
)
1049+
1050+
1051+
@pytest.mark.asyncio
1052+
async def test_unknown_area_id_message_points_to_the_lookup_tool() -> None:
1053+
"""A caller with only a display name (e.g. ha_search's friendly
1054+
``area_names``, or a floor's display name) retries with the same wrong
1055+
value forever unless told area_ids/floor_ids are a different, exact
1056+
registry ID and where to find it -- confirmed live: a caller that
1057+
passed display names ("Cave", "Couloir Sous-Sol") as ``area_ids`` hit
1058+
this exact error four times in a row with no path to recovery.
1059+
"""
1060+
client = SelectorClient(states=[_state("light.one")], entities=[])
1061+
1062+
with pytest.raises(
1063+
BulkSelectorValidationError,
1064+
match=r"exact Home Assistant registry IDs.*ha_list_floors_areas",
1065+
):
1066+
await resolve_bulk_selector(
1067+
client,
1068+
{"domain": "light", "area_ids": ["Couloir Sous-Sol"]},
1069+
action="off",
1070+
parameters=None,
1071+
timeout_seconds=None,
1072+
validate_first=True,
1073+
)

tests/src/unit/test_ha_bulk_control_selector.py

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

33
from __future__ import annotations
44

5+
import json
56
from unittest.mock import AsyncMock, MagicMock
67

78
import pytest
@@ -259,6 +260,13 @@ async def test_operations_mode_rejects_group_and_member_in_same_batch() -> None:
259260
Operations mode has no ``exclude_entity_ids`` to express that intent,
260261
so the omission alone never protected it; the only safe response is to
261262
reject the whole batch rather than silently dispatch it.
263+
264+
Also confirmed live: the rejection message alone was not enough. The
265+
calling model retried four times with a broken selector call --
266+
``exclude_entity_ids`` at the top level (it belongs inside ``selector``)
267+
and area *display names* where exact ``area_id`` registry values were
268+
required -- and never recovered. The message must show a concrete,
269+
correctly-shaped example, not just name selector mode.
262270
"""
263271
device_tools = MagicMock()
264272
device_tools.bulk_device_control = AsyncMock(return_value={"success": True})
@@ -293,9 +301,17 @@ async def test_operations_mode_rejects_group_and_member_in_same_batch() -> None:
293301
# caller's (unenforceable, in this mode) attempt to exclude it.
294302
]
295303

296-
with pytest.raises(ToolError, match="group/aggregate entity"):
304+
with pytest.raises(ToolError) as exc_info:
297305
await tools.ha_bulk_control(operations, False)
298306

307+
message = json.loads(str(exc_info.value))["error"]["message"]
308+
assert "group/aggregate entity" in message
309+
# The worked example must show exclude_entity_ids nested INSIDE
310+
# selector -- not as a sibling argument, which is the exact mistake
311+
# observed live.
312+
assert '"selector": {"domain": "light"' in message
313+
assert '"exclude_entity_ids"' in message
314+
assert "ha_list_floors_areas" in message
299315
device_tools.bulk_device_control.assert_not_awaited()
300316

301317

0 commit comments

Comments
 (0)