fix: return RESOURCE_NOT_FOUND instead of false success on dashboard deletion (#680)

* fix: return RESOURCE_NOT_FOUND instead of false success on dashboard deletion

ha_config_delete_dashboard and ha_config_delete_dashboard_resource
previously returned success:true when the target didn't exist, misleading
AI agents into believing the operation succeeded.

Changes:
- ha_config_delete_dashboard now resolves both url_path and internal ID,
  returning RESOURCE_NOT_FOUND when neither matches
- ha_config_delete_dashboard_resource now returns RESOURCE_NOT_FOUND
  instead of false success on non-existent resources
- Both tools use structured error helpers and exception_to_structured_error
- Removed idempotentHint from both tools (no longer silently succeed)
- Updated E2E tests to expect RESOURCE_NOT_FOUND on non-existent targets

Closes #671

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: update unit test to expect RESOURCE_NOT_FOUND on missing resource

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: kingpanther13 <kingpanther13@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: kingpanther13

src/ha_mcp/tools/tools_config_dashboards.py

@@ -16,7 +16,7 @@
 from pydantic import Field
 
 from ..config import get_global_settings
-from ..errors import ErrorCode, create_error_response
+from ..errors import ErrorCode, create_error_response, create_resource_not_found_error
 from ..utils.python_sandbox import (
     PythonSandboxError,
     get_security_documentation,
@@ -1144,7 +1144,6 @@ async def ha_config_set_dashboard(
     @mcp.tool(
         annotations={
             "destructiveHint": True,
-            "idempotentHint": True,
             "tags": ["dashboard"],
             "title": "Delete Dashboard",
         }
@@ -1153,7 +1152,9 @@ async def ha_config_set_dashboard(
     async def ha_config_delete_dashboard(
         dashboard_id: Annotated[
             str,
-            Field(description="Dashboard ID to delete (typically same as url_path)"),
+            Field(
+                description="Dashboard ID or URL path to delete (e.g., 'my-dashboard' or 'my_dashboard')"
+            ),
         ],
     ) -> dict[str, Any]:
         """
@@ -1162,14 +1163,49 @@ async def ha_config_delete_dashboard(
         WARNING: This permanently deletes the dashboard and all its configuration.
         Cannot be undone. Does not work on YAML-mode dashboards.
 
+        Accepts either the internal dashboard ID or the URL path.
+        The tool resolves url_path to internal ID automatically.
+
         EXAMPLES:
         - Delete dashboard: ha_config_delete_dashboard("mobile-dashboard")
 
         Note: The default dashboard cannot be deleted via this method.
         """
         try:
+            # Fetch dashboard list to resolve the provided identifier.
+            # HA internal IDs may differ from url_path (e.g. hyphens → underscores),
+            # so we accept either and resolve to the actual registry ID.
+            list_result = await client.send_websocket_message(
+                {"type": "lovelace/dashboards/list"}
+            )
+            if isinstance(list_result, dict) and "result" in list_result:
+                dashboards = list_result["result"]
+            elif isinstance(list_result, list):
+                dashboards = list_result
+            else:
+                dashboards = []
+
+            resolved_id = None
+            for d in dashboards:
+                if d.get("id") == dashboard_id:
+                    resolved_id = d["id"]
+                    break
+                if d.get("url_path") == dashboard_id:
+                    resolved_id = d["id"]
+                    break
+
+            if resolved_id is None:
+                return create_resource_not_found_error(
+                    "Dashboard",
+                    dashboard_id,
+                    details=(
+                        f"No dashboard found with ID or URL path '{dashboard_id}'. "
+                        "Use ha_config_get_dashboard(list_only=True) to see available dashboards."
+                    ),
+                )
+
             response = await client.send_websocket_message(
-                {"type": "lovelace/dashboards/delete", "dashboard_id": dashboard_id}
+                {"type": "lovelace/dashboards/delete", "dashboard_id": resolved_id}
             )
 
             # Check response for error indication
@@ -1182,68 +1218,39 @@ async def ha_config_delete_dashboard(
 
                 logger.error(f"Error deleting dashboard: {error_str}")
 
-                # If the error is "not found" / "doesn't exist", treat as success (idempotent)
-                if (
-                    "unable to find" in error_str.lower()
-                    or "not found" in error_str.lower()
-                ):
-                    return {
-                        "success": True,
-                        "action": "delete",
-                        "dashboard_id": dashboard_id,
-                        "message": "Dashboard already deleted or does not exist",
-                    }
-
-                # For other errors, return failure
-                return {
-                    "success": False,
-                    "action": "delete",
-                    "dashboard_id": dashboard_id,
-                    "error": error_str,
-                    "suggestions": [
+                return create_error_response(
+                    code=ErrorCode.SERVICE_CALL_FAILED,
+                    message=f"Failed to delete dashboard: {error_str}",
+                    context={"action": "delete", "dashboard_id": dashboard_id},
+                    suggestions=[
                         "Verify dashboard exists and is storage-mode",
                         "Check that you have admin permissions",
                         "Use ha_config_get_dashboard(list_only=True) to see available dashboards",
                         "Cannot delete YAML-mode or default dashboard",
                     ],
-                }
+                )
 
             # Delete successful
-            return {
+            result: dict[str, Any] = {
                 "success": True,
                 "action": "delete",
                 "dashboard_id": dashboard_id,
                 "message": "Dashboard deleted successfully",
             }
+            if resolved_id != dashboard_id:
+                result["resolved_id"] = resolved_id
+            return result
         except Exception as e:
-            error_str = str(e)
-            logger.error(f"Error deleting dashboard: {error_str}")
-
-            # If the error is "not found" / "doesn't exist", treat as success (idempotent)
-            if (
-                "unable to find" in error_str.lower()
-                or "not found" in error_str.lower()
-            ):
-                return {
-                    "success": True,
-                    "action": "delete",
-                    "dashboard_id": dashboard_id,
-                    "message": "Dashboard already deleted or does not exist",
-                }
-
-            # For other errors, return failure
-            return {
-                "success": False,
-                "action": "delete",
-                "dashboard_id": dashboard_id,
-                "error": error_str,
-                "suggestions": [
-                    "Verify dashboard exists and is storage-mode",
-                    "Check that you have admin permissions",
-                    "Use ha_config_get_dashboard(list_only=True) to see available dashboards",
-                    "Cannot delete YAML-mode or default dashboard",
+            logger.error(f"Error deleting dashboard: {e}")
+            return exception_to_structured_error(
+                e,
+                context={"action": "delete", "dashboard_id": dashboard_id},
+                raise_error=False,
+                suggestions=[
+                    "Check Home Assistant connection",
+                    "Verify dashboard exists with ha_config_get_dashboard(list_only=True)",
                 ],
-            }
+            )
 
     @mcp.tool(
         annotations={

src/ha_mcp/tools/tools_resources.py

@@ -14,8 +14,8 @@
 
 from pydantic import Field
 
-from ..errors import ErrorCode, create_error_response
-from .helpers import log_tool_usage
+from ..errors import ErrorCode, create_error_response, create_resource_not_found_error
+from .helpers import exception_to_structured_error, log_tool_usage
 
 logger = logging.getLogger(__name__)
 
@@ -498,7 +498,6 @@ class MyCard extends HTMLElement {
     @mcp.tool(
         annotations={
             "destructiveHint": True,
-            "idempotentHint": True,
             "tags": ["dashboard", "resources"],
             "title": "Delete Dashboard Resource",
         }
@@ -516,8 +515,7 @@ async def ha_config_delete_dashboard_resource(
         Delete a dashboard resource.
 
         Removes a resource from Home Assistant. The resource will no longer
-        be loaded on dashboards. This operation is idempotent - deleting
-        a non-existent resource will succeed.
+        be loaded on dashboards.
 
         WARNING: Deleting a resource used by custom cards in your dashboards
         will cause those cards to fail to load.
@@ -544,21 +542,25 @@ async def ha_config_delete_dashboard_resource(
                 else:
                     error_str = str(error_msg)
 
-                # If "not found", treat as success (idempotent)
                 if "not found" in error_str.lower() or "unable to find" in error_str.lower():
-                    return {
-                        "success": True,
-                        "action": "delete",
-                        "resource_id": resource_id,
-                        "message": "Resource already deleted or does not exist",
-                    }
+                    return create_resource_not_found_error(
+                        "Dashboard resource",
+                        resource_id,
+                        details=(
+                            f"Resource '{resource_id}' not found. "
+                            "Use ha_config_list_dashboard_resources() to see available resources."
+                        ),
+                    )
 
-                return {
-                    "success": False,
-                    "action": "delete",
-                    "resource_id": resource_id,
-                    "error": error_str,
-                }
+                return create_error_response(
+                    code=ErrorCode.SERVICE_CALL_FAILED,
+                    message=f"Failed to delete dashboard resource: {error_str}",
+                    context={"action": "delete", "resource_id": resource_id},
+                    suggestions=[
+                        "Verify resource ID using ha_config_list_dashboard_resources()",
+                        "Check that you have admin permissions",
+                    ],
+                )
 
             logger.info(f"Dashboard resource deleted: id={resource_id}")
 
@@ -569,25 +571,13 @@ async def ha_config_delete_dashboard_resource(
                 "message": "Resource deleted successfully",
             }
         except Exception as e:
-            error_str = str(e)
-            logger.error(f"Error deleting dashboard resource: {error_str}")
-
-            # If "not found", treat as success (idempotent)
-            if "not found" in error_str.lower() or "unable to find" in error_str.lower():
-                return {
-                    "success": True,
-                    "action": "delete",
-                    "resource_id": resource_id,
-                    "message": "Resource already deleted or does not exist",
-                }
-
-            return {
-                "success": False,
-                "action": "delete",
-                "resource_id": resource_id,
-                "error": error_str,
-                "suggestions": [
+            logger.error(f"Error deleting dashboard resource: {e}")
+            return exception_to_structured_error(
+                e,
+                context={"action": "delete", "resource_id": resource_id},
+                raise_error=False,
+                suggestions=[
                     "Verify resource ID using ha_config_list_dashboard_resources()",
                     "Check that you have admin permissions",
                 ],
-            }
+            )

tests/src/e2e/workflows/dashboards/test_lifecycle.py

@@ -344,17 +344,16 @@ async def test_get_nonexistent_dashboard(self, mcp_client):
         logger.info("Get nonexistent dashboard test completed successfully")
 
     async def test_delete_nonexistent_dashboard(self, mcp_client):
-        """Test deleting non-existent dashboard."""
+        """Test deleting non-existent dashboard returns RESOURCE_NOT_FOUND."""
         logger.info("Starting delete nonexistent dashboard test")
 
         result = await mcp_client.call_tool(
             "ha_config_delete_dashboard",
             {"dashboard_id": "nonexistent-dashboard-67890"},
         )
         data = parse_mcp_result(result)
-        # Home Assistant handles delete as idempotent - deleting nonexistent item succeeds
-        # This is expected behavior and consistent with other HA operations
-        assert data["success"] is True
+        assert data["success"] is False
+        assert data["error"]["code"] == "RESOURCE_NOT_FOUND"
 
         logger.info("Delete nonexistent dashboard test completed successfully")
 

tests/src/e2e/workflows/dashboards/test_resources.py

@@ -259,16 +259,17 @@ async def test_invalid_resource_type(self, mcp_client):
         logger.info("Invalid resource type test completed successfully")
 
     async def test_delete_nonexistent_resource(self, mcp_client):
-        """Test that deleting nonexistent resource is idempotent (succeeds)."""
+        """Test that deleting nonexistent resource returns RESOURCE_NOT_FOUND."""
         logger.info("Starting delete nonexistent resource test")
         mcp = MCPAssertions(mcp_client)
 
-        # Deleting a resource that doesn't exist should succeed (idempotent)
-        delete_data = await mcp.call_tool_success(
+        # Deleting a resource that doesn't exist should return RESOURCE_NOT_FOUND
+        delete_data = await mcp.call_tool_failure(
             "ha_config_delete_dashboard_resource",
             {"resource_id": "nonexistent-resource-id-12345"},
+            expected_error="not found",
         )
-        assert delete_data["success"] is True
+        assert delete_data["success"] is False
 
         logger.info("Delete nonexistent resource test completed successfully")
 

tests/src/unit/test_tools_resources.py

@@ -411,17 +411,17 @@ async def test_delete_success(self, delete_tool, mock_client):
         assert result["resource_id"] == "abc123"
 
     @pytest.mark.asyncio
-    async def test_delete_idempotent_not_found(self, delete_tool, mock_client):
-        """Test that deleting non-existent resource is idempotent."""
+    async def test_delete_not_found_returns_error(self, delete_tool, mock_client):
+        """Test that deleting non-existent resource returns RESOURCE_NOT_FOUND."""
         mock_client.send_websocket_message.return_value = {
             "success": False,
             "error": {"message": "Resource not found"},
         }
 
         result = await delete_tool(resource_id="nonexistent")
 
-        assert result["success"] is True  # Idempotent
-        assert "already deleted" in result["message"].lower()
+        assert result["success"] is False
+        assert result["error"]["code"] == "RESOURCE_NOT_FOUND"
 
 
 class TestToolRegistration: