fix(bulk): surface resolution warnings at top level, fix E2E MCPAssertions usage

Two CodeRabbit findings on the latest push:

- resolution.summary()'s warnings list was left nested under
  response["resolution"]["warnings"] on both the dry-run and dispatch
  return paths, contradicting AGENTS.md's "warnings is always a top-level
  list[str], never nested" contract -- a consumer reading the top-level
  key never saw the hidden-entity degradation. Extracted
  _attach_resolution_to_response() (also needed to keep _run_bulk_selector
  under the C901 complexity threshold) which pops the nested warnings and
  extends (not overwrites) any warnings bulk_device_control's own response
  already carries.
- The new E2E dispatch test read the excluded entity's post-dispatch state
  via the raw mcp_client + parse_mcp_result outside the MCPAssertions
  context, instead of mcp.call_tool_success() inside it -- inconsistent
  with this PR's own earlier fix (c62d9e4) and the repo's coding
  guideline for success-path tool calls in E2E tests.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Author: swissmo

src/ha_mcp/tools/tools_service.py

@@ -28,6 +28,7 @@
 from .bulk_selector import (
     BulkControlSelector,
     BulkSelectorInfrastructureError,
+    BulkSelectorResolution,
     BulkSelectorValidationError,
     resolve_bulk_selector,
 )
@@ -211,6 +212,26 @@ def _selector_only_parameter_offender(
     return None
 
 
+def _attach_resolution_to_response(
+    response: dict[str, Any], resolution: BulkSelectorResolution
+) -> None:
+    """Attach ``resolution.summary()`` to a response, surfacing its warnings
+    at the top level.
+
+    Per AGENTS.md "Return Values", ``warnings`` is always a top-level
+    ``list[str]``, never nested inside another field.
+    ``resolution.summary()`` nests its own warnings (e.g. "N entities were
+    hidden") under ``resolution`` for internal cohesion, so this pops them
+    back out. ``response`` may already carry dispatch-time warnings (e.g.
+    from ``bulk_device_control``) -- those are extended, not overwritten.
+    """
+    summary = resolution.summary()
+    resolution_warnings = summary.pop("warnings", [])
+    response["resolution"] = summary
+    if resolution_warnings:
+        response.setdefault("warnings", []).extend(resolution_warnings)
+
+
 class _AmbiguousDispatch:
     """Sentinel type for a post-send-ambiguous component write (see below)."""
 
@@ -1767,12 +1788,13 @@ async def _run_bulk_selector(
             raise  # unreachable: exception_to_structured_error always raises
 
         if dry_run:
-            return {
+            response: dict[str, Any] = {
                 "success": True,
                 "dry_run": True,
                 "dispatched": False,
-                "resolution": resolution.summary(),
             }
+            _attach_resolution_to_response(response, resolution)
+            return response
         try:
             result = await self._device_tools.bulk_device_control(
                 operations=resolution.operations,
@@ -1799,7 +1821,7 @@ async def _run_bulk_selector(
             )
             raise  # unreachable: exception_to_structured_error always raises
         response = cast(dict[str, Any], result)
-        response["resolution"] = resolution.summary()
+        _attach_resolution_to_response(response, resolution)
         return response
 
     @tool(

tests/src/e2e/workflows/core/test_bulk.py

@@ -179,19 +179,19 @@ async def test_selector_dispatch_turns_off_included_and_spares_excluded(
                     },
                 )
 
+                assert await wait_for_entity_state(mcp_client, included_id, "off"), (
+                    f"Included helper {included_id} was not turned off by the dispatch"
+                )
+                excluded_data = await mcp.call_tool_success(
+                    "ha_get_state", {"entity_id": excluded_id}
+                )
+                assert excluded_data.get("data", {}).get("state") == "on", (
+                    f"Excluded helper's real state must never move: got {excluded_data}"
+                )
+
             assert data.get("dry_run") is None
             assert data["resolution"]["resolved_entity_ids"] == [included_id]
             assert data["resolution"]["excluded_entity_ids"] == [excluded_id]
-            assert await wait_for_entity_state(mcp_client, included_id, "off"), (
-                f"Included helper {included_id} was not turned off by the dispatch"
-            )
-            excluded_state = await mcp_client.call_tool(
-                "ha_get_state", {"entity_id": excluded_id}
-            )
-            excluded_data = parse_mcp_result(excluded_state)
-            assert excluded_data.get("data", {}).get("state") == "on", (
-                f"Excluded helper's real state must never move: got {excluded_data}"
-            )
         finally:
             for entity_id in entity_ids:
                 await safe_call_tool(

tests/src/unit/test_ha_bulk_control_selector.py

@@ -56,6 +56,83 @@ async def test_selector_dry_run_never_dispatches(
     device_tools.bulk_device_control.assert_not_awaited()
 
 
+@pytest.mark.asyncio
+async def test_dry_run_surfaces_resolution_warnings_at_top_level(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Per AGENTS.md, `warnings` must be top-level, never nested under
+    `resolution` -- a consumer that only reads the top-level key must still
+    see a hidden-entity degradation."""
+    resolution_with_warning = BulkSelectorResolution(
+        resolved_entity_ids=("light.sofa",),
+        excluded_entity_ids=(),
+        selected_area_ids=("salon",),
+        expanded_group_ids=(),
+        hidden_entity_count=1,
+        warnings=("1 matching entity was hidden by the entity visibility filter.",),
+        _operation_common={"action": "off", "validate_first": True},
+    )
+    monkeypatch.setattr(
+        "ha_mcp.tools.tools_service.resolve_bulk_selector",
+        AsyncMock(return_value=resolution_with_warning),
+    )
+    tools = ServiceTools(MagicMock(), MagicMock())
+
+    result = await tools.ha_bulk_control(
+        selector={"domain": "light", "area_ids": ["salon"]},
+        action="off",
+        dry_run=True,
+    )
+
+    assert result["warnings"] == [
+        "1 matching entity was hidden by the entity visibility filter."
+    ]
+    assert "warnings" not in result["resolution"]
+
+
+@pytest.mark.asyncio
+async def test_dispatch_merges_resolution_warnings_with_dispatch_warnings(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Dispatch-time warnings from bulk_device_control (per-operation
+    degradations) must be extended with, not overwritten by, the
+    resolution's own warnings -- both belong in the same top-level list."""
+    resolution_with_warning = BulkSelectorResolution(
+        resolved_entity_ids=("light.sofa",),
+        excluded_entity_ids=(),
+        selected_area_ids=("salon",),
+        expanded_group_ids=(),
+        hidden_entity_count=1,
+        warnings=("1 matching entity was hidden by the entity visibility filter.",),
+        _operation_common={"action": "off", "validate_first": True},
+    )
+    monkeypatch.setattr(
+        "ha_mcp.tools.tools_service.resolve_bulk_selector",
+        AsyncMock(return_value=resolution_with_warning),
+    )
+    device_tools = MagicMock()
+    device_tools.bulk_device_control = AsyncMock(
+        return_value={
+            "success": True,
+            "successful": 1,
+            "failed": 0,
+            "warnings": ["light.sofa took longer than expected to confirm"],
+        }
+    )
+    tools = ServiceTools(MagicMock(), device_tools)
+
+    result = await tools.ha_bulk_control(
+        selector={"domain": "light", "area_ids": ["salon"]},
+        action="off",
+    )
+
+    assert result["warnings"] == [
+        "light.sofa took longer than expected to confirm",
+        "1 matching entity was hidden by the entity visibility filter.",
+    ]
+    assert "warnings" not in result["resolution"]
+
+
 @pytest.mark.asyncio
 async def test_selector_dispatches_only_frozen_leaf_operations(
     monkeypatch: pytest.MonkeyPatch,