Raise ToolError instead of returning error strings (isError: true)

All tool errors now raise ToolError (from fastmcp.exceptions) instead
of returning JSON/XML error strings. This sets isError: true in the MCP
response, letting agents distinguish errors from data and make better
retry/recovery decisions.

Changes:
- handle_api_error() raises ToolError instead of returning a string
- All validation errors (empty query, invalid max_results, etc.) raise
  ToolError instead of returning JSON
- fetch_artifacts validation raises ToolError instead of returning XML
- 38 tests updated to expect ToolError via pytest.raises()
- E2E tests now verify result.is_error flag alongside error text

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

Author: rodion-m

src/tests/test_artifact_relationships.py

@@ -6,6 +6,7 @@
 from unittest.mock import AsyncMock, MagicMock, patch
 
 from fastmcp import Context
+from fastmcp.exceptions import ToolError
 
 from tools.artifact_relationships import (
     PROFILE_MAP,
@@ -284,22 +285,18 @@ async def test_explicit_profile_maps_correctly(self, mock_get_api_key):
         assert call_args[1]["json"]["profile"] == "InheritanceOnly"
 
     @pytest.mark.asyncio
-    async def test_empty_identifier_returns_error(self):
+    async def test_empty_identifier_raises_tool_error(self):
         ctx = MagicMock(spec=Context)
-        result = await get_artifact_relationships(ctx=ctx, identifier="")
-        data = json.loads(result)
-        assert "error" in data
-        assert "required" in data["error"]
+        with pytest.raises(ToolError, match="required"):
+            await get_artifact_relationships(ctx=ctx, identifier="")
 
     @pytest.mark.asyncio
-    async def test_unsupported_profile_returns_error(self):
+    async def test_unsupported_profile_raises_tool_error(self):
         ctx = MagicMock(spec=Context)
-        result = await get_artifact_relationships(
-            ctx=ctx, identifier="id", profile="invalidProfile"
-        )
-        data = json.loads(result)
-        assert "error" in data
-        assert "Unsupported profile" in data["error"]
+        with pytest.raises(ToolError, match="Unsupported profile"):
+            await get_artifact_relationships(
+                ctx=ctx, identifier="id", profile="invalidProfile"
+            )
 
     @pytest.mark.asyncio
     @patch("tools.artifact_relationships.get_api_key_from_context")
@@ -327,11 +324,8 @@ async def test_api_error_returns_error_json(self, mock_get_api_key):
         mock_context.base_url = "https://app.codealive.ai"
         ctx.request_context.lifespan_context = mock_context
 
-        result = await get_artifact_relationships(ctx=ctx, identifier="id")
-
-        data = json.loads(result)
-        assert "error" in data
-        assert "401" in data["error"]
+        with pytest.raises(ToolError, match="401"):
+            await get_artifact_relationships(ctx=ctx, identifier="id")
 
     @pytest.mark.asyncio
     @patch("tools.artifact_relationships.get_api_key_from_context")

src/tests/test_chat_tool.py

@@ -4,6 +4,7 @@
 from unittest.mock import AsyncMock, MagicMock, patch
 import json
 from fastmcp import Context
+from fastmcp.exceptions import ToolError
 from tools.chat import chat, codebase_consultant
 
 
@@ -162,12 +163,12 @@ async def test_chat_empty_question_validation(mock_get_api_key):
     ctx.request_context.lifespan_context = MagicMock()
 
     # Test with empty question
-    result = await chat(ctx=ctx, question="")
-    assert "Error: No question provided" in result
+    with pytest.raises(ToolError, match="No question provided"):
+        await chat(ctx=ctx, question="")
 
     # Test with whitespace only
-    result = await chat(ctx=ctx, question="   ")
-    assert "Error: No question provided" in result
+    with pytest.raises(ToolError, match="No question provided"):
+        await chat(ctx=ctx, question="   ")
 
 
 
@@ -176,9 +177,9 @@ async def test_chat_empty_question_validation(mock_get_api_key):
 @patch('tools.chat.get_api_key_from_context')
 @patch('tools.chat.handle_api_error')
 async def test_chat_error_handling(mock_handle_error, mock_get_api_key):
-    """Test error handling in chat."""
+    """Test error handling in chat — handle_api_error raises ToolError."""
     mock_get_api_key.return_value = "test_key"
-    mock_handle_error.return_value = "Error: Authentication failed"
+    mock_handle_error.side_effect = ToolError("Error: Authentication failed")
 
     ctx = MagicMock(spec=Context)
     ctx.info = AsyncMock()
@@ -192,11 +193,11 @@ async def test_chat_error_handling(mock_handle_error, mock_get_api_key):
 
     ctx.request_context.lifespan_context = mock_codealive_context
 
-    result = await chat(
-        ctx=ctx,
-        question="Test",
-        data_sources=["repo123"]
-    )
+    with pytest.raises(ToolError, match="Authentication failed"):
+        await chat(
+            ctx=ctx,
+            question="Test",
+            data_sources=["repo123"]
+        )
 
-    assert result == "Error: Authentication failed"
     mock_handle_error.assert_called_once()

src/tests/test_e2e_tools.py

@@ -159,9 +159,8 @@ async def test_backend_500_returns_error(self):
             result = await client.call_tool("get_data_sources", {}, raise_on_error=False)
 
         text = _text(result)
-        data = json.loads(text)
-        assert "error" in data
-        assert "500" in data["error"] or "Server error" in data["error"]
+        assert result.is_error
+        assert "500" in text or "Server error" in text
 
 
 # ---------------------------------------------------------------------------
@@ -218,9 +217,8 @@ async def test_empty_query_returns_error(self):
             )
 
         text = _text(result)
-        data = json.loads(text)
-        assert "error" in data
-        assert "empty" in data["error"].lower()
+        assert result.is_error
+        assert "empty" in text.lower() or "Query cannot be empty" in text
 
     @pytest.mark.asyncio
     async def test_no_results_returns_empty_json(self):
@@ -265,9 +263,8 @@ async def test_404_returns_not_found_error(self):
             )
 
         text = _text(result)
-        data = json.loads(text)
-        assert "error" in data
-        assert "404" in data["error"] or "not found" in data["error"].lower()
+        assert result.is_error
+        assert "404" in text or "not found" in text.lower()
 
 
 # ---------------------------------------------------------------------------
@@ -354,9 +351,8 @@ async def test_max_results_boundary_0_rejected(self):
                 {"query": "test", "data_sources": ["repo"], "max_results": 0},
                 raise_on_error=False,
             )
-        data = json.loads(_text(result))
-        assert "error" in data
-        assert "max_results" in data["error"]
+        assert result.is_error
+        assert "max_results" in _text(result)
 
     @pytest.mark.asyncio
     async def test_max_results_boundary_501_rejected(self):
@@ -367,9 +363,8 @@ async def test_max_results_boundary_501_rejected(self):
                 {"query": "test", "data_sources": ["repo"], "max_results": 501},
                 raise_on_error=False,
             )
-        data = json.loads(_text(result))
-        assert "error" in data
-        assert "max_results" in data["error"]
+        assert result.is_error
+        assert "max_results" in _text(result)
 
     @pytest.mark.asyncio
     async def test_max_results_boundary_500_accepted(self):
@@ -496,9 +491,9 @@ async def test_404_includes_recovery_hint(self):
                 {"query": "test", "data_sources": ["bad-repo"]},
                 raise_on_error=False,
             )
-        data = json.loads(_text(result))
-        assert "error" in data
-        assert "get_data_sources" in data["error"]
+        text = _text(result)
+        assert result.is_error
+        assert "get_data_sources" in text
 
 
 # ---------------------------------------------------------------------------
@@ -597,8 +592,8 @@ async def test_max_results_boundary_0_rejected(self):
                 {"query": "test", "data_sources": ["repo"], "max_results": 0},
                 raise_on_error=False,
             )
-        data = json.loads(_text(result))
-        assert "error" in data
+        assert result.is_error
+        assert "max_results" in _text(result)
 
     @pytest.mark.asyncio
     async def test_max_results_boundary_501_rejected(self):
@@ -609,8 +604,8 @@ async def test_max_results_boundary_501_rejected(self):
                 {"query": "test", "data_sources": ["repo"], "max_results": 501},
                 raise_on_error=False,
             )
-        data = json.loads(_text(result))
-        assert "error" in data
+        assert result.is_error
+        assert "max_results" in _text(result)
 
     @pytest.mark.asyncio
     async def test_extensions_forwarded(self):
@@ -672,9 +667,8 @@ async def test_empty_query_returns_error(self):
                 {"query": "", "data_sources": ["repo"]},
                 raise_on_error=False,
             )
-        data = json.loads(_text(result))
-        assert "error" in data
-        assert "empty" in data["error"].lower()
+        assert result.is_error
+        assert "empty" in _text(result).lower() or "Query cannot be empty" in _text(result)
 
     @pytest.mark.asyncio
     async def test_data_sources_as_string_normalized(self):
@@ -700,9 +694,8 @@ async def test_404_includes_recovery_hint(self):
                 {"query": "test", "data_sources": ["bad-repo"]},
                 raise_on_error=False,
             )
-        data = json.loads(_text(result))
-        assert "error" in data
-        assert "get_data_sources" in data["error"]
+        assert result.is_error
+        assert "get_data_sources" in _text(result)
 
 
 # ---------------------------------------------------------------------------
@@ -1098,9 +1091,8 @@ async def test_empty_identifier_returns_error(self):
                 raise_on_error=False,
             )
 
-        data = json.loads(_text(result))
-        assert "error" in data
-        assert "required" in data["error"].lower()
+        assert result.is_error
+        assert "required" in _text(result).lower()
 
     @pytest.mark.asyncio
     async def test_max_count_per_type_0_rejected(self):
@@ -1111,9 +1103,8 @@ async def test_max_count_per_type_0_rejected(self):
                 {"identifier": "org/repo::x", "max_count_per_type": 0},
                 raise_on_error=False,
             )
-        data = json.loads(_text(result))
-        assert "error" in data
-        assert "max_count_per_type" in data["error"]
+        assert result.is_error
+        assert "max_count_per_type" in _text(result)
 
     @pytest.mark.asyncio
     async def test_max_count_per_type_1001_rejected(self):
@@ -1124,9 +1115,8 @@ async def test_max_count_per_type_1001_rejected(self):
                 {"identifier": "org/repo::x", "max_count_per_type": 1001},
                 raise_on_error=False,
             )
-        data = json.loads(_text(result))
-        assert "error" in data
-        assert "max_count_per_type" in data["error"]
+        assert result.is_error
+        assert "max_count_per_type" in _text(result)
 
     @pytest.mark.asyncio
     async def test_max_count_per_type_negative_rejected(self):
@@ -1137,9 +1127,8 @@ async def test_max_count_per_type_negative_rejected(self):
                 {"identifier": "org/repo::x", "max_count_per_type": -1},
                 raise_on_error=False,
             )
-        data = json.loads(_text(result))
-        assert "error" in data
-        assert "max_count_per_type" in data["error"]
+        assert result.is_error
+        assert "max_count_per_type" in _text(result)
 
     @pytest.mark.asyncio
     async def test_max_count_per_type_forwarded(self):