homeassistant-ai
diff --git a/‎AGENTS.md‎
Lines changed: 67 additions & 26 deletions b/‎AGENTS.md‎
Lines changed: 67 additions & 26 deletions
diff --git a/‎src/ha_mcp/tools/backup.py‎
Lines changed: 70 additions & 65 deletions b/‎src/ha_mcp/tools/backup.py‎
Lines changed: 70 additions & 65 deletions
@@ -431,20 +431,27 @@ cp .env.example .env       # Configure HA connection
 - Personal workflow helper (gitignored, not committed)
 
 ### Testing
-E2E tests are in `tests/src/e2e/` (not `tests/e2e/`).
+E2E tests are in `tests/src/e2e/` (not `tests/e2e/`). Tests use **testcontainers** to spin up
+an isolated Docker HA instance — Docker daemon must be running.
 
 ```bash
-# Run E2E tests (requires Docker daemon)
-uv run pytest tests/src/e2e/ -v --tb=short
+# Run FULL E2E suite (required before claiming all tests pass)
+cd tests && uv run pytest src/e2e/ -v --tb=short
 
-# Run specific test
-uv run pytest tests/src/e2e/workflows/automation/test_lifecycle.py -v
+# Run specific file (partial coverage only — never substitute for full suite)
+cd tests && uv run pytest src/e2e/workflows/automation/test_lifecycle.py -v
 
 # Interactive test environment
 uv run hamcp-test-env                    # Interactive mode
 uv run hamcp-test-env --no-interactive   # For automation
 ```
 
+**CRITICAL RULES:**
+- Always run from the `tests/` directory so pytest picks up the correct `conftest.py`
+- Always run the **full suite** before declaring tests pass
+- `tests/.env.test` contains placeholder values only; testcontainers sets the real URL dynamically
+- Never set `HOMEASSISTANT_URL` manually in your shell before running tests
+
 Test token centralized in `tests/test_constants.py`.
 
 ### Code Quality
@@ -535,45 +542,79 @@ def register_<domain>_tools(mcp, client, **kwargs):
 
 **Always use the dedicated error functions** from `errors.py` and `helpers.py`. Never construct raw error dicts manually — the helpers ensure consistent structure, error codes, and suggestions across all tools.
 
-**Domain-specific errors** (`errors.py`) — use these when the error type is known:
+**All tool-level failures must raise `ToolError`** (sets `isError=true` per MCP spec). Batch item failures within result arrays are the only exception — those return structured dicts without raising.
+
+**Pattern A — Exception blocks** (most common): call `exception_to_structured_error` without `return` — it raises `ToolError` by default:
 ```python
-from ..errors import create_entity_not_found_error, create_validation_error, create_service_error
+from .helpers import exception_to_structured_error, raise_tool_error
+from fastmcp.exceptions import ToolError
 
-# Entity lookup failures (404 / not found)
-return create_entity_not_found_error(entity_id, details=str(e))
+try:
+    # ... tool logic ...
+except ToolError:
+    raise  # must re-raise; prevents ToolError being swallowed by outer except
+except Exception as e:
+    exception_to_structured_error(
+        e,
+        context={"entity_id": entity_id},
+        suggestions=["Verify entity exists", "Check HA connection"],
+    )
+```
 
-# Invalid parameters
-return create_validation_error("Invalid format", parameter="entity_ids", details=str(e))
+The `except ToolError: raise` guard is required whenever `raise_tool_error()` or validation errors are called inside the same `try` block — without it, `except Exception` catches the `ToolError` and re-maps it to `INTERNAL_ERROR`.
 
-# Service call failures
-return create_service_error(domain, service, message=f"Service call failed: {e}", details=str(e))
+**Pattern B — Input validation errors**:
+```python
+from ..errors import ErrorCode, create_error_response, create_validation_error
+
+if not entity_id.startswith("light."):
+    raise_tool_error(create_error_response(
+        ErrorCode.VALIDATION_INVALID_PARAMETER,
+        f"entity_id must start with 'light.', got: {entity_id}",
+        suggestions=["Use ha_search_entities(domain_filter='light') to find valid IDs"],
+        context={"entity_id": entity_id},
+    ))
 ```
 
-Available helpers: `create_entity_not_found_error`, `create_connection_error`, `create_auth_error`, `create_service_error`, `create_validation_error`, `create_config_error`, `create_timeout_error`, `create_resource_not_found_error`, and the generic `create_error_response`.
-
-**Catch-all exception handler** (`helpers.py`) — use in `except Exception` blocks:
+**Pattern C — WebSocket / service call failures**:
 ```python
-from .helpers import exception_to_structured_error
+if not result.get("success"):
+    raise_tool_error(create_error_response(
+        ErrorCode.SERVICE_CALL_FAILED,
+        result.get("error", "Operation failed"),
+        context={"entity_id": entity_id},
+    ))
+```
 
-except Exception as e:
-    return exception_to_structured_error(e, context={"entity_id": entity_id})
+**Pattern D — Batch item failures** (items inside a results list — do NOT raise):
+```python
+results.append(create_error_response(
+    ErrorCode.SERVICE_CALL_FAILED,
+    str(e),
+    context={"entity_id": eid},
+))
 ```
 
-**Pattern for tools**: Use `exception_to_structured_error` as the catch-all — it already classifies 404s, auth errors, timeouts, etc. based on exception type and message. Pass `context={"entity_id": ...}` so it can produce `ENTITY_NOT_FOUND` for 404 errors automatically. No manual 404 string matching needed:
+**Special case** — when the error dict needs post-processing before raising (e.g., timezone metadata injection), use `raise_error=False` then `raise_tool_error()`:
 ```python
-try:
-    result = await client.get_entity_state(entity_id)
-    return await add_timezone_metadata(client, result)
 except Exception as e:
-    error_response = exception_to_structured_error(e, context={"entity_id": entity_id})
-    return await add_timezone_metadata(client, error_response)
+    error_response = exception_to_structured_error(
+        e, context={"entity_id": entity_id}, raise_error=False
+    )
+    error_with_tz = await add_timezone_metadata(client, error_response)
+    raise_tool_error(error_with_tz)
 ```
 
+Available `errors.py` helpers: `create_entity_not_found_error`, `create_connection_error`, `create_auth_error`, `create_service_error`, `create_validation_error`, `create_config_error`, `create_timeout_error`, `create_resource_not_found_error`, and the generic `create_error_response`.
+
+`exception_to_structured_error` already classifies 404s, auth errors, timeouts, etc. based on exception type. Pass `context={"entity_id": ...}` so it produces `ENTITY_NOT_FOUND` for 404 errors automatically — no manual string matching needed.
+
 ### Return Values
 ```python
 {"success": True, "data": result}                    # Success
 {"success": True, "partial": True, "warning": "..."}  # Degraded
-{"success": False, "error": {...}}                    # Failure
+raise ToolError(json.dumps({...}))                   # Tool-level failure (isError=true)
+{"success": False, "error": {...}}                   # Batch item failure only (in results list)
 ```
 
 ### Tool Consolidation
 
@@ -9,11 +9,18 @@
 from datetime import datetime
 from typing import TYPE_CHECKING, Annotated, Any
 
+from fastmcp.exceptions import ToolError
 from pydantic import Field
 
 from ..client.rest_client import HomeAssistantClient
 from ..client.websocket_client import HomeAssistantWebSocketClient
-from .helpers import get_connected_ws_client, log_tool_usage
+from ..errors import ErrorCode, create_error_response
+from .helpers import (
+    exception_to_structured_error,
+    get_connected_ws_client,
+    log_tool_usage,
+    raise_tool_error,
+)
 
 if TYPE_CHECKING:
     from fastmcp import FastMCP
@@ -94,12 +101,18 @@ async def create_backup(
         # Connect to WebSocket
         ws_client, error = await get_connected_ws_client(client.base_url, client.token)
         if error:
-            return error
+            raise_tool_error(error or create_error_response(
+                ErrorCode.CONNECTION_FAILED,
+                "Failed to connect to Home Assistant WebSocket for backup",
+            ))
 
         # Get backup password
         password, error = await _get_backup_password(ws_client)
         if error:
-            return error
+            raise_tool_error(create_error_response(
+                ErrorCode.SERVICE_CALL_FAILED,
+                error.get("error", "Failed to retrieve backup password"),
+            ))
 
         # Generate backup name if not provided
         if not name:
@@ -120,11 +133,10 @@ async def create_backup(
         result = await ws_client.send_command("backup/generate", **backup_params)
 
         if not result.get("success"):
-            return {
-                "success": False,
-                "error": "Backup creation failed",
-                "details": result,
-            }
+            raise_tool_error(create_error_response(
+                ErrorCode.SERVICE_CALL_FAILED,
+                result.get("error", "Backup creation failed"),
+            ))
 
         backup_job_id = result.get("result", {}).get("backup_job_id")
         logger.info(f"Backup job started: {backup_job_id}, waiting for completion...")
@@ -185,30 +197,30 @@ async def create_backup(
 
                 # Check if backup failed
                 elif event_state == "failed":
-                    return {
-                        "success": False,
-                        "error": "Backup creation failed",
-                        "backup_job_id": backup_job_id,
-                        "last_event": last_event,
-                    }
+                    raise_tool_error(create_error_response(
+                        ErrorCode.SERVICE_CALL_FAILED,
+                        "Backup creation failed",
+                        context={"backup_job_id": backup_job_id},
+                    ))
 
         # Timeout waiting for backup
         logger.warning(f"Backup did not complete within {max_wait_seconds} seconds")
-        return {
-            "success": False,
-            "error": f"Backup creation timed out after {max_wait_seconds} seconds",
-            "backup_job_id": backup_job_id,
-            "name": name,
-            "suggestion": "Backup may still be in progress. Check Home Assistant backup status.",
-        }
-
+        raise_tool_error(create_error_response(
+            ErrorCode.TIMEOUT_OPERATION,
+            f"Backup creation timed out after {max_wait_seconds} seconds",
+            context={"backup_job_id": backup_job_id, "name": name},
+            suggestions=["Backup may still be in progress. Check Home Assistant backup status."],
+        ))
+
+    except ToolError:
+        raise
     except Exception as e:
         logger.error(f"Error creating backup: {e}")
-        return {
-            "success": False,
-            "error": f"Failed to create backup: {str(e)}",
-            "suggestion": "Check Home Assistant connection and backup configuration",
-        }
+        exception_to_structured_error(
+            e,
+            context={"tool": "create_backup"},
+            suggestions=["Check Home Assistant connection and backup configuration"],
+        )
     finally:
         # Always disconnect WebSocket
         if ws_client:
@@ -240,35 +252,28 @@ async def restore_backup(
         # Connect to WebSocket
         ws_client, error = await get_connected_ws_client(client.base_url, client.token)
         if error:
-            return error
+            raise_tool_error(error or create_error_response(
+                ErrorCode.CONNECTION_FAILED,
+                "Failed to connect to Home Assistant WebSocket for restore",
+            ))
 
         # Verify backup exists
         backup_info = await ws_client.send_command("backup/info")
         if not backup_info.get("success"):
-            return {
-                "success": False,
-                "error": "Failed to retrieve backup information",
-                "details": backup_info,
-            }
+            raise_tool_error(create_error_response(
+                ErrorCode.SERVICE_CALL_FAILED,
+                backup_info.get("error", "Failed to retrieve backup information"),
+            ))
 
         backups = backup_info.get("result", {}).get("backups", [])
         backup_exists = any(b.get("backup_id") == backup_id for b in backups)
 
         if not backup_exists:
-            available_backups = [
-                {
-                    "backup_id": b.get("backup_id"),
-                    "name": b.get("name"),
-                    "date": b.get("date"),
-                }
-                for b in backups[:5]
-            ]
-            return {
-                "success": False,
-                "error": f"Backup '{backup_id}' not found",
-                "available_backups": available_backups,
-                "suggestion": "Use one of the available backup IDs listed above",
-            }
+            raise_tool_error(create_error_response(
+                ErrorCode.RESOURCE_NOT_FOUND,
+                f"Backup '{backup_id}' not found",
+                suggestions=["Use ha_backup_list() to see available backups"],
+            ))
 
         # Create safety backup BEFORE restoring
         logger.info("Creating safety backup before restore...")
@@ -293,12 +298,11 @@ async def restore_backup(
             )
 
             if not safety_backup.get("success"):
-                return {
-                    "success": False,
-                    "error": "Failed to create safety backup before restore",
-                    "details": safety_backup,
-                    "suggestion": "Cannot proceed with restore without safety backup",
-                }
+                raise_tool_error(create_error_response(
+                    ErrorCode.SERVICE_CALL_FAILED,
+                    safety_backup.get("error", "Failed to create safety backup before restore"),
+                    suggestions=["Cannot proceed with restore without safety backup"],
+                ))
 
             safety_backup_id = safety_backup.get("result", {}).get("backup_job_id")
             logger.info(f"Safety backup created: {safety_backup_id}")
@@ -326,20 +330,21 @@ async def restore_backup(
                 "note": "A safety backup was created before restore. You can restore from it if needed.",
             }
         else:
-            return {
-                "success": False,
-                "error": "Restore operation failed",
-                "details": result,
-                "safety_backup_id": safety_backup_id,
-            }
-
+            raise_tool_error(create_error_response(
+                ErrorCode.SERVICE_CALL_FAILED,
+                result.get("error", "Restore operation failed"),
+                context={"backup_id": backup_id},
+            ))
+
+    except ToolError:
+        raise
     except Exception as e:
         logger.error(f"Error restoring backup: {e}")
-        return {
-            "success": False,
-            "error": f"Failed to restore backup: {str(e)}",
-            "suggestion": "Check Home Assistant connection and backup availability",
-        }
+        exception_to_structured_error(
+            e,
+            context={"tool": "restore_backup", "backup_id": backup_id},
+            suggestions=["Check Home Assistant connection and backup availability"],
+        )
     finally:
         # Always disconnect WebSocket
         if ws_client: