Issue #518 part3 (#569)

* fix: Signal tool errors via isError for automation, service, and registry tools (#518)

Part 1 of the isError signaling fix. Adds core infrastructure and applies
to tools that already have comprehensive E2E test coverage.

Core changes:
- Add raise_tool_error() helper to convert error dicts to ToolError exceptions
- Update exception_to_structured_error() to raise by default (raise_error=True)
- Add @overload type signatures for raise_error parameter
- Add safe_call_tool() and tool_error_to_result() test utilities

Tool changes (3 tools with existing E2E tests):
- tools_config_automations: 5 error paths now raise ToolError
- tools_service: 4 error paths now raise ToolError
- tools_registry: 1 error path now raises ToolError

Test updates:
- Update 11 E2E test files to use safe_call_tool for error path testing
- Add unit tests for raise_tool_error and exception_to_structured_error
- Add retry logic for entity rename tests (timing robustness)

Closes part of #518

https://claude.ai/code/session_01MvDDV6qmWosBfGhmTLYzo8

* refactor: use match statement and keep raise_error=False as default

Per code review feedback:
1. Use match statement for HTTP status code dispatch (Python 3.13)
2. Keep raise_error=False as default to avoid regression during PR stack
   - Tools explicitly pass raise_error=True or use raise_error=False + raise_tool_error()
   - Part 5 will flip the default after all tools are migrated
3. Update unit tests to reflect new default behavior

https://claude.ai/code/session_01MvDDV6qmWosBfGhmTLYzo8

* fix: Address code review feedback on PR #551

Fixes for HIGH priority issues:
- H1: Add `except ToolError: raise` before `except Exception` handlers in
  tools_config_automations.py and tools_service.py to prevent double-wrapping
- H2: Change return type annotation to `dict[str, Any]` (NoReturn in union is meaningless)
- H3: Add case 403 to match statement for auth/permission errors

Fixes for MEDIUM priority issues:
- M2: Add `default=str` to json.dumps in raise_tool_error for non-serializable fallback
- M3: Change pytest.skip to pytest.xfail in test_lights.py to not mask regressions

https://claude.ai/code/session_01MvDDV6qmWosBfGhmTLYzo8

* test: Update 8 E2E test files to handle ToolError via safe_call_tool (#518)

Part 2 of the isError signaling fix. Updates all remaining E2E test
files that expect error responses to use safe_call_tool, which handles
both legacy dict returns and new ToolError exceptions.

Updated test files:
- test_config_entry_flow: 2 error-expecting tests
- test_helper_crud: 3 error tests + wait_for_entity_registration helper
- test_entity_management: 2 error-expecting tests
- test_file_operations: 2 helper functions using parse_mcp_result
- test_hacs: 3 error-expecting tests
- test_integration_management: 1 error-expecting test
- test_device_registry: 4 error-expecting tests
- test_voice_assistant: 2 error-expecting tests

Total: ~28 error-expecting assertions across 8 files

Depends on PR 1 which provides safe_call_tool infrastructure.

https://claude.ai/code/session_01MvDDV6qmWosBfGhmTLYzo8

* fix: Signal tool errors via isError for remaining 9 tools (#518)

Migrate remaining tool modules to use ToolError/raise_tool_error for
MCP protocol-level error signaling. Includes updated unit tests for
voice assistant and entity tools.

https://claude.ai/code/session_01MvDDV6qmWosBfGhmTLYzo8

---------

Co-authored-by: Claude <noreply@anthropic.com>

Author: kingpanther13

src/ha_mcp/errors.py

@@ -257,20 +257,22 @@ def create_connection_error(
     message: str,
     details: str | None = None,
     timeout: bool = False,
+    context: dict[str, Any] | None = None,
 ) -> dict[str, Any]:
     """Create a connection error response."""
     code = ErrorCode.CONNECTION_TIMEOUT if timeout else ErrorCode.CONNECTION_FAILED
-    return create_error_response(code, message, details)
+    return create_error_response(code, message, details, context=context)
 
 
 def create_auth_error(
     message: str,
     details: str | None = None,
     expired: bool = False,
+    context: dict[str, Any] | None = None,
 ) -> dict[str, Any]:
     """Create an authentication error response."""
     code = ErrorCode.AUTH_EXPIRED if expired else ErrorCode.AUTH_INVALID_TOKEN
-    return create_error_response(code, message, details)
+    return create_error_response(code, message, details, context=context)
 
 
 def create_entity_not_found_error(
@@ -350,13 +352,19 @@ def create_timeout_error(
     operation: str,
     timeout_seconds: float,
     details: str | None = None,
+    context: dict[str, Any] | None = None,
 ) -> dict[str, Any]:
     """Create a timeout error response."""
+    final_context: dict[str, Any] = {}
+    if context:
+        final_context.update(context)
+    final_context["operation"] = operation
+    final_context["timeout_seconds"] = timeout_seconds
     return create_error_response(
         ErrorCode.TIMEOUT_OPERATION,
         f"Operation '{operation}' timed out after {timeout_seconds}s",
         details=details,
-        context={"operation": operation, "timeout_seconds": timeout_seconds},
+        context=final_context,
     )
 
 

src/ha_mcp/tools/helpers.py

@@ -137,15 +137,15 @@ def exception_to_structured_error(
     # Handle specific exception types
     if isinstance(error, HomeAssistantConnectionError):
         if "timeout" in error_str:
-            error_response = create_connection_error(error_msg, timeout=True)
+            error_response = create_connection_error(error_msg, timeout=True, context=context)
         else:
-            error_response = create_connection_error(error_msg)
+            error_response = create_connection_error(error_msg, context=context)
 
     elif isinstance(error, HomeAssistantAuthError):
         if "expired" in error_str:
-            error_response = create_auth_error(error_msg, expired=True)
+            error_response = create_auth_error(error_msg, expired=True, context=context)
         else:
-            error_response = create_auth_error(error_msg)
+            error_response = create_auth_error(error_msg, context=context)
 
     elif isinstance(error, HomeAssistantAPIError):
         # Check for specific error patterns
@@ -162,7 +162,7 @@ def exception_to_structured_error(
                         context=context,
                     )
             case 401 | 403:
-                error_response = create_auth_error(error_msg)
+                error_response = create_auth_error(error_msg, context=context)
             case 400:
                 error_response = create_validation_error(error_msg, context=context)
             case _:
@@ -176,10 +176,10 @@ def exception_to_structured_error(
     elif isinstance(error, TimeoutError):
         operation = context.get("operation", "request") if context else "request"
         timeout_seconds = context.get("timeout_seconds", 30) if context else 30
-        error_response = create_timeout_error(operation, timeout_seconds, details=error_msg)
+        error_response = create_timeout_error(operation, timeout_seconds, details=error_msg, context=context)
 
     elif isinstance(error, ValueError):
-        error_response = create_validation_error(error_msg)
+        error_response = create_validation_error(error_msg, context=context)
 
     # Check for common error patterns in error message
     elif "not found" in error_str or "404" in error_str:
@@ -194,13 +194,13 @@ def exception_to_structured_error(
             )
 
     elif "timeout" in error_str:
-        error_response = create_timeout_error("operation", 30, details=error_msg)
+        error_response = create_timeout_error("operation", 30, details=error_msg, context=context)
 
     elif "connection" in error_str or "connect" in error_str:
-        error_response = create_connection_error(error_msg)
+        error_response = create_connection_error(error_msg, context=context)
 
     elif "auth" in error_str or "token" in error_str or "401" in error_str:
-        error_response = create_auth_error(error_msg)
+        error_response = create_auth_error(error_msg, context=context)
 
     else:
         # Default to internal error -- use generic message to avoid leaking internals

src/ha_mcp/tools/tools_areas.py

@@ -8,10 +8,11 @@
 import logging
 from typing import Annotated, Any
 
+from fastmcp.exceptions import ToolError
 from pydantic import Field
 
 from ..errors import ErrorCode, create_error_response
-from .helpers import log_tool_usage
+from .helpers import log_tool_usage, raise_tool_error
 from .util_helpers import parse_string_list_param
 
 logger = logging.getLogger(__name__)
@@ -121,10 +122,10 @@ async def ha_config_set_area(
             try:
                 parsed_aliases = parse_string_list_param(aliases, "aliases")
             except ValueError as e:
-                return create_error_response(
+                raise_tool_error(create_error_response(
                     ErrorCode.VALIDATION_INVALID_PARAMETER,
                     f"Invalid aliases parameter: {e}",
-                )
+                ))
 
             # Determine if this is a create or update operation
             if area_id:
@@ -193,6 +194,8 @@ async def ha_config_set_area(
                     error_response["name"] = name
                 return error_response
 
+        except ToolError:
+            raise
         except Exception as e:
             logger.error(f"Error in ha_config_set_area: {e}")
             error_response = {
@@ -352,10 +355,10 @@ async def ha_config_set_floor(
             try:
                 parsed_aliases = parse_string_list_param(aliases, "aliases")
             except ValueError as e:
-                return create_error_response(
+                raise_tool_error(create_error_response(
                     ErrorCode.VALIDATION_INVALID_PARAMETER,
                     f"Invalid aliases parameter: {e}",
-                )
+                ))
 
             # Determine if this is a create or update operation
             if floor_id:
@@ -420,6 +423,8 @@ async def ha_config_set_floor(
                     error_response["name"] = name
                 return error_response
 
+        except ToolError:
+            raise
         except Exception as e:
             logger.error(f"Error in ha_config_set_floor: {e}")
             error_response = {

src/ha_mcp/tools/tools_config_entry_flow.py

@@ -157,7 +157,7 @@ async def ha_create_config_entry_helper(
             context = {"helper_type": helper_type}
             if flow_id:
                 context["flow_id"] = flow_id
-            return exception_to_structured_error(e, context=context)
+            exception_to_structured_error(e, context=context, raise_error=True)
 
     @mcp.tool(
         annotations={
@@ -220,4 +220,4 @@ async def ha_get_helper_schema(
 
         except Exception as e:
             logger.error(f"Error getting helper schema: {e}")
-            return exception_to_structured_error(e, context={"helper_type": helper_type})
+            exception_to_structured_error(e, context={"helper_type": helper_type}, raise_error=True)

src/ha_mcp/tools/tools_config_helpers.py

@@ -10,10 +10,11 @@
 import logging
 from typing import Annotated, Any, Literal
 
+from fastmcp.exceptions import ToolError
 from pydantic import Field
 
 from ..errors import ErrorCode, create_error_response
-from .helpers import log_tool_usage
+from .helpers import log_tool_usage, raise_tool_error
 from .util_helpers import (
     coerce_bool_param,
     parse_string_list_param,
@@ -399,10 +400,10 @@ async def ha_config_set_helper(
                 labels = parse_string_list_param(labels, "labels")
                 options = parse_string_list_param(options, "options")
             except ValueError as e:
-                return create_error_response(
+                raise_tool_error(create_error_response(
                     ErrorCode.VALIDATION_INVALID_PARAMETER,
                     f"Invalid list parameter: {e}",
-                )
+                ))
 
             # Determine if this is a create or update based on helper_id
             action = "update" if helper_id else "create"
@@ -708,6 +709,8 @@ async def ha_config_set_helper(
                 "error": f"Unexpected action: {action}",
             }
 
+        except ToolError:
+            raise
         except Exception as e:
             return {
                 "success": False,

src/ha_mcp/tools/tools_entities.py

@@ -9,10 +9,11 @@
 import logging
 from typing import Annotated, Any, Literal
 
+from fastmcp.exceptions import ToolError
 from pydantic import Field
 
 from ..errors import ErrorCode, create_error_response
-from .helpers import exception_to_structured_error, log_tool_usage
+from .helpers import exception_to_structured_error, log_tool_usage, raise_tool_error
 from .tools_voice_assistant import KNOWN_ASSISTANTS
 from .util_helpers import coerce_bool_param, parse_json_param, parse_string_list_param
 
@@ -445,65 +446,65 @@ async def ha_set_entity(
                 try:
                     parsed_aliases = parse_string_list_param(aliases, "aliases")
                 except ValueError as e:
-                    return create_error_response(
+                    raise_tool_error(create_error_response(
                         ErrorCode.VALIDATION_INVALID_PARAMETER,
                         f"Invalid aliases parameter: {e}",
-                    )
+                    ))
 
             parsed_labels = None
             if labels is not None:
                 try:
                     parsed_labels = parse_string_list_param(labels, "labels")
                 except ValueError as e:
-                    return create_error_response(
+                    raise_tool_error(create_error_response(
                         ErrorCode.VALIDATION_INVALID_PARAMETER,
                         f"Invalid labels parameter: {e}",
-                    )
+                    ))
 
             # Parse and validate expose_to parameter
             parsed_expose_to: dict[str, bool] | None = None
             if expose_to is not None:
                 try:
                     parsed = parse_json_param(expose_to, "expose_to")
                 except ValueError as e:
-                    return create_error_response(
+                    raise_tool_error(create_error_response(
                         ErrorCode.VALIDATION_INVALID_PARAMETER,
                         str(e),
-                    )
+                    ))
 
                 if not isinstance(parsed, dict):
-                    return create_error_response(
+                    raise_tool_error(create_error_response(
                         ErrorCode.VALIDATION_INVALID_PARAMETER,
                         "expose_to must be a dict mapping assistant IDs to booleans, "
                         'e.g. {"conversation": true, "cloud.alexa": false}',
-                    )
+                    ))
                 parsed_expose_to = parsed
 
                 # Validate assistant names
                 invalid_assistants = [
                     a for a in parsed_expose_to if a not in KNOWN_ASSISTANTS
                 ]
                 if invalid_assistants:
-                    return create_error_response(
+                    raise_tool_error(create_error_response(
                         ErrorCode.VALIDATION_INVALID_PARAMETER,
                         f"Invalid assistant(s) in expose_to: {invalid_assistants}. "
                         f"Valid: {KNOWN_ASSISTANTS}",
-                    )
+                    ))
 
                 # Coerce values to bool
                 for asst, val in parsed_expose_to.items():
                     try:
                         coerced = coerce_bool_param(val, f"expose_to[{asst}]")
                     except ValueError as e:
-                        return create_error_response(
+                        raise_tool_error(create_error_response(
                             ErrorCode.VALIDATION_INVALID_PARAMETER,
                             str(e),
-                        )
+                        ))
                     if coerced is None:
-                        return create_error_response(
+                        raise_tool_error(create_error_response(
                             ErrorCode.VALIDATION_INVALID_PARAMETER,
                             f"expose_to[{asst}] must be a boolean value",
-                        )
+                        ))
                     parsed_expose_to[asst] = coerced
 
             # Single entity case - use existing logic
@@ -579,10 +580,12 @@ async def ha_set_entity(
 
             return response
 
+        except ToolError:
+            raise
         except Exception as e:
             logger.error(f"Error updating entity: {e}")
             eid_context = entity_id if isinstance(entity_id, str) else entity_ids
-            return exception_to_structured_error(e, context={"entity_id": eid_context})
+            exception_to_structured_error(e, context={"entity_id": eid_context}, raise_error=True)
 
     @mcp.tool(
         annotations={
@@ -768,6 +771,7 @@ async def _fetch_entity(eid: str) -> dict[str, Any]:
 
         except Exception as e:
             logger.error(f"Error getting entity: {e}")
-            return exception_to_structured_error(
-                e, context={"entity_id": entity_id if isinstance(entity_id, str) else entity_ids}
+            exception_to_structured_error(
+                e, context={"entity_id": entity_id if isinstance(entity_id, str) else entity_ids},
+                raise_error=True,
             )

src/ha_mcp/tools/tools_filesystem.py

@@ -19,7 +19,7 @@
 
 from pydantic import Field
 
-from .helpers import exception_to_structured_error, log_tool_usage
+from .helpers import exception_to_structured_error, log_tool_usage, raise_tool_error
 from .util_helpers import add_timezone_metadata, coerce_bool_param, coerce_int_param
 
 logger = logging.getLogger(__name__)
@@ -192,8 +192,10 @@ async def ha_list_files(
             error_response = exception_to_structured_error(
                 e,
                 context={"tool": "ha_list_files", "path": path, "pattern": pattern},
+                raise_error=False,
             )
-            return await add_timezone_metadata(client, error_response)
+            error_with_tz = await add_timezone_metadata(client, error_response)
+            raise_tool_error(error_with_tz)
 
     @mcp.tool(
         annotations={
@@ -316,8 +318,10 @@ async def ha_read_file(
             error_response = exception_to_structured_error(
                 e,
                 context={"tool": "ha_read_file", "path": path},
+                raise_error=False,
             )
-            return await add_timezone_metadata(client, error_response)
+            error_with_tz = await add_timezone_metadata(client, error_response)
+            raise_tool_error(error_with_tz)
 
     @mcp.tool(
         annotations={
@@ -458,8 +462,10 @@ async def ha_write_file(
             error_response = exception_to_structured_error(
                 e,
                 context={"tool": "ha_write_file", "path": path},
+                raise_error=False,
             )
-            return await add_timezone_metadata(client, error_response)
+            error_with_tz = await add_timezone_metadata(client, error_response)
+            raise_tool_error(error_with_tz)
 
     @mcp.tool(
         annotations={
@@ -585,5 +591,7 @@ async def ha_delete_file(
             error_response = exception_to_structured_error(
                 e,
                 context={"tool": "ha_delete_file", "path": path},
+                raise_error=False,
             )
-            return await add_timezone_metadata(client, error_response)
+            error_with_tz = await add_timezone_metadata(client, error_response)
+            raise_tool_error(error_with_tz)