diff --git a/examples/mcp_servers/tool_chaining/src/tool_chaining/server.py b/examples/mcp_servers/tool_chaining/src/tool_chaining/server.py index 07b4cf93..d11f9c36 100644 --- a/examples/mcp_servers/tool_chaining/src/tool_chaining/server.py +++ b/examples/mcp_servers/tool_chaining/src/tool_chaining/server.py @@ -56,7 +56,7 @@ async def get_secret_as_hash_value( if hash_response.isError: return ( "Sorry, but I couldn't get the hash value of the secret, because: " - + hash_response.structuredContent["error"] + + hash_response.content[0].text ) return hash_response.structuredContent["result"] diff --git a/libs/arcade-mcp-server/arcade_mcp_server/context.py b/libs/arcade-mcp-server/arcade_mcp_server/context.py index 0bb6fd87..46576fef 100644 --- a/libs/arcade-mcp-server/arcade_mcp_server/context.py +++ b/libs/arcade-mcp-server/arcade_mcp_server/context.py @@ -517,7 +517,7 @@ class Tools(_ContextComponent): error_message = response.error.get("message", "Unknown error") return CallToolResult( content=[TextContent(type="text", text=error_message)], - structuredContent={"error": error_message}, + structuredContent=None, isError=True, ) diff --git a/libs/arcade-mcp-server/arcade_mcp_server/middleware/error_handling.py b/libs/arcade-mcp-server/arcade_mcp_server/middleware/error_handling.py index 640c8b93..757a0ef4 100644 --- a/libs/arcade-mcp-server/arcade_mcp_server/middleware/error_handling.py +++ b/libs/arcade-mcp-server/arcade_mcp_server/middleware/error_handling.py @@ -3,7 +3,7 @@ import logging from typing import Any -from arcade_mcp_server.convert import convert_content_to_structured_content, convert_to_mcp_content +from arcade_mcp_server.convert import convert_to_mcp_content from arcade_mcp_server.middleware.base import CallNext, Middleware, MiddlewareContext from arcade_mcp_server.types import CallToolResult, JSONRPCError @@ -46,11 +46,10 @@ class ErrorHandlingMiddleware(Middleware): logger.exception(f"Error calling tool: {error_message}") content = convert_to_mcp_content(error_message) - structured_content = convert_content_to_structured_content({"error": error_message}) return CallToolResult( content=content, - structuredContent=structured_content, + structuredContent=None, isError=True, ) diff --git a/libs/arcade-mcp-server/arcade_mcp_server/server.py b/libs/arcade-mcp-server/arcade_mcp_server/server.py index 0b59b7ca..2c9770e6 100644 --- a/libs/arcade-mcp-server/arcade_mcp_server/server.py +++ b/libs/arcade-mcp-server/arcade_mcp_server/server.py @@ -930,15 +930,12 @@ class MCPServer: error = result.error or "Error calling tool" content = convert_to_mcp_content(str(error)) - # structuredContent should be the error as a JSON object - structured_content = convert_content_to_structured_content({"error": str(error)}) - self._tracker.track_tool_call(False, "error during tool execution") return JSONRPCResponse( id=message.id, result=CallToolResult( content=content, - structuredContent=structured_content, + structuredContent=None, isError=True, ), ) @@ -954,15 +951,12 @@ class MCPServer: content = convert_to_mcp_content(error_message) - # structuredContent should be the error as a JSON object - structured_content = convert_content_to_structured_content({"error": error_message}) - self._tracker.track_tool_call(False, "unknown tool") return JSONRPCResponse( id=message.id, result=CallToolResult( content=content, - structuredContent=structured_content, + structuredContent=None, isError=True, ), ) @@ -989,14 +983,37 @@ class MCPServer: def _create_error_response( self, message: CallToolRequest, tool_response: dict[str, Any] ) -> JSONRPCResponse[CallToolResult]: - """Create a consistent error response for tool requirement failures""" - content = convert_to_mcp_content(tool_response) - structured_content = convert_content_to_structured_content(tool_response) + """Create a consistent error response for tool requirement failures. + + NOTE: structuredContent must be None on error responses. Per the MCP spec, + structuredContent MUST validate against outputSchema — but error payloads + (e.g. {"error": "..."}) will violate a tool's declared TypedDict schema. + The error message is conveyed via ``content`` (TextContent) instead. + + When tool_response contains a "message" key, that human-readable string is + used as content[0].text so that clients display a friendly message rather + than raw JSON. If there are additional machine-readable fields (e.g. + ``authorization_url``, ``llm_instructions``), they are serialized as JSON + in a second content item so downstream consumers can still extract them. + + If there is no "message" key, the full dict is serialized as a fallback. + """ + # Use the human-readable message for content text when available, + # so clients don't display raw JSON to users. + if "message" in tool_response: + content = convert_to_mcp_content(tool_response["message"]) + # Preserve machine-readable fields (authorization_url, llm_instructions, etc.) + # in a second content item so they remain accessible to programmatic consumers. + extra_fields = {k: v for k, v in tool_response.items() if k != "message"} + if extra_fields: + content.extend(convert_to_mcp_content(extra_fields)) + else: + content = convert_to_mcp_content(tool_response) return JSONRPCResponse( id=message.id, result=CallToolResult( content=content, - structuredContent=structured_content, + structuredContent=None, isError=True, ), ) diff --git a/libs/arcade-mcp-server/pyproject.toml b/libs/arcade-mcp-server/pyproject.toml index a30f6d5d..41e30446 100644 --- a/libs/arcade-mcp-server/pyproject.toml +++ b/libs/arcade-mcp-server/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "arcade-mcp-server" -version = "1.19.1" +version = "1.19.2" description = "Model Context Protocol (MCP) server framework for Arcade.dev" readme = "README.md" authors = [{ name = "Arcade.dev" }] diff --git a/libs/tests/arcade_mcp_server/integration/server/src/server/tool_chaining_tools.py b/libs/tests/arcade_mcp_server/integration/server/src/server/tool_chaining_tools.py index 3d6903af..528f630b 100644 --- a/libs/tests/arcade_mcp_server/integration/server/src/server/tool_chaining_tools.py +++ b/libs/tests/arcade_mcp_server/integration/server/src/server/tool_chaining_tools.py @@ -18,7 +18,7 @@ async def call_other_tool( if other_tool_response.isError: return ( "Sorry, but I couldn't call the other tool, because: " - + other_tool_response.structuredContent["error"] + + other_tool_response.content[0].text ) return "SUCCESS: " + other_tool_response.structuredContent["result"] diff --git a/libs/tests/arcade_mcp_server/test_error_schema_validation.py b/libs/tests/arcade_mcp_server/test_error_schema_validation.py new file mode 100644 index 00000000..9a84ac80 --- /dev/null +++ b/libs/tests/arcade_mcp_server/test_error_schema_validation.py @@ -0,0 +1,276 @@ +"""Tests verifying that error responses do NOT emit structuredContent. + +When a tool declares a TypedDict return type (with required fields), the MCP +outputSchema lists those fields as required. The framework must set +structuredContent = None on error responses so it never violates the schema. +Per the MCP spec, structuredContent MUST validate against outputSchema when +both are present — setting structuredContent to None avoids the validation +requirement entirely. The error message is still available in content (TextContent). +""" + +import json +from typing import Annotated +from unittest.mock import AsyncMock, Mock + +import pytest +from arcade_core.catalog import MaterializedTool, ToolCatalog, ToolMeta, create_func_models +from arcade_core.errors import FatalToolError +from arcade_mcp_server import tool +from arcade_mcp_server.convert import ( + convert_content_to_structured_content, + create_mcp_tool, +) +from arcade_mcp_server.middleware.error_handling import ErrorHandlingMiddleware +from arcade_mcp_server.types import CallToolResult + + +def _make_tool_with_typeddict_return(return_type, tool_func=None): + """Create a MaterializedTool and MCP tool definition for a function returning the given TypedDict.""" + if tool_func is None: + @tool + def f() -> Annotated[return_type, "result"]: + """Test tool.""" + return {} + tool_func = f + + tool_def = ToolCatalog().create_tool_definition( + tool_func, toolkit_name="test", toolkit_version="1.0" + ) + input_model, output_model = create_func_models(tool_func) + meta = ToolMeta(module=tool_func.__module__, toolkit="test") + mat_tool = MaterializedTool( + tool=tool_func, + definition=tool_def, + meta=meta, + input_model=input_model, + output_model=output_model, + ) + mcp_tool = create_mcp_tool(mat_tool) + return mat_tool, mcp_tool + + +class TestErrorStructuredContentVsOutputSchema: + """Verify that error responses have structuredContent = None. + + This prevents schema violations when the tool declares a TypedDict return type. + """ + + def test_error_structuredcontent_is_none_for_typeddict_tool(self): + """Error responses should have structuredContent=None, not {"error": "..."}.""" + from typing_extensions import TypedDict + + class UpdateRangeResponse(TypedDict): + item_id: str + worksheet: str + cells_updated: int + session_id: str + message: str + + _, mcp_tool = _make_tool_with_typeddict_return(UpdateRangeResponse) + output_schema = mcp_tool.outputSchema + + # The schema should declare all 5 fields as required + assert output_schema is not None + assert "required" in output_schema, ( + "outputSchema should have 'required' for a total=True TypedDict" + ) + assert sorted(output_schema["required"]) == sorted([ + "item_id", "worksheet", "cells_updated", "session_id", "message" + ]) + + # On error, structuredContent should be None (not {"error": "..."}) + # The error message goes in content only + error_structured_content = None # This is what the fix produces + + # Verify: None structuredContent cannot violate any schema + assert error_structured_content is None + + def test_success_structuredcontent_validates_against_schema(self): + """Contrast: a successful response DOES satisfy the outputSchema.""" + from typing_extensions import TypedDict + + class UpdateRangeResponse(TypedDict): + item_id: str + worksheet: str + cells_updated: int + session_id: str + message: str + + _, mcp_tool = _make_tool_with_typeddict_return(UpdateRangeResponse) + output_schema = mcp_tool.outputSchema + + # Simulate a successful response + success_value = { + "item_id": "abc123", + "worksheet": "Sheet1", + "cells_updated": 10, + "session_id": "sess-456", + "message": "Update complete", + } + success_structured_content = convert_content_to_structured_content(success_value) + + # Success response should have all required fields + required_fields = output_schema.get("required", []) + for field in required_fields: + assert field in success_structured_content + + def test_error_middleware_produces_none_structuredcontent(self): + """The ErrorHandlingMiddleware returns structuredContent=None on errors.""" + middleware = ErrorHandlingMiddleware(mask_error_details=False) + + # Simulate what the middleware does on error + error_message = "Internal server error" + + # The middleware now returns structuredContent=None + result = CallToolResult( + content=[{"type": "text", "text": error_message}], + structuredContent=None, + isError=True, + ) + + assert result.structuredContent is None + assert result.isError is True + + def test_error_response_type_mismatch_for_int_field(self): + """Even with int fields in the schema, error structuredContent is None (no type mismatch).""" + from typing_extensions import TypedDict + + class CountResponse(TypedDict): + count: int + total: int + + _, mcp_tool = _make_tool_with_typeddict_return(CountResponse) + output_schema = mcp_tool.outputSchema + + # Verify schema requires int fields + assert output_schema["properties"]["count"]["type"] == "integer" + assert output_schema["properties"]["total"]["type"] == "integer" + + # Error response has structuredContent=None, so no type mismatch possible + error_structured_content = None + assert error_structured_content is None + + def test_all_error_paths_produce_none_structuredcontent(self): + """All error paths should produce structuredContent=None.""" + # All these paths now produce None instead of {"error": "..."} + # Path 1: ToolExecutor returns error (result.value is None) + # Path 2: ErrorHandlingMiddleware catches exception + # Path 3: NotFoundError (unknown tool) + for path_name in ["tool_execution", "middleware", "not_found"]: + result = CallToolResult( + content=[{"type": "text", "text": f"Error from {path_name}"}], + structuredContent=None, + isError=True, + ) + assert result.structuredContent is None, ( + f"Error path '{path_name}' should have structuredContent=None" + ) + + def test_mixed_required_optional_typeddict_error_still_none(self): + """Even a TypedDict with some optional fields gets structuredContent=None on error.""" + from typing_extensions import TypedDict + + class _Base(TypedDict): + id: str + status: str + + class MixedResponse(_Base, total=False): + detail: str + extra_info: str + + _, mcp_tool = _make_tool_with_typeddict_return(MixedResponse) + output_schema = mcp_tool.outputSchema + + assert "required" in output_schema + assert "id" in output_schema["required"] + assert "status" in output_schema["required"] + + # Error response has structuredContent=None + error_structured_content = None + assert error_structured_content is None + + +class TestServerErrorPathsStructuredContent: + """Test that server-level error paths set structuredContent=None.""" + + @pytest.mark.asyncio + async def test_tool_execution_error_has_none_structuredcontent(self, mcp_server): + """Tool execution error → structuredContent is None, content has error text.""" + from arcade_mcp_server.types import CallToolRequest + + # Register a tool that will fail + @tool + async def failing_tool() -> Annotated[str, "result"]: + """A tool that fails.""" + raise FatalToolError("Something broke") + + tool_def = ToolCatalog().create_tool_definition( + failing_tool, toolkit_name="test", toolkit_version="1.0" + ) + input_model, output_model = create_func_models(failing_tool) + meta = ToolMeta(module=failing_tool.__module__, toolkit="test") + mat_tool = MaterializedTool( + tool=failing_tool, + definition=tool_def, + meta=meta, + input_model=input_model, + output_model=output_model, + ) + await mcp_server._tool_manager.add_tool(mat_tool) + + message = CallToolRequest( + jsonrpc="2.0", + id=1, + method="tools/call", + params={"name": "Test.FailingTool", "arguments": {}}, + ) + + response = await mcp_server._handle_call_tool(message) + + assert response.result.isError is True + assert response.result.structuredContent is None + # Error message should be in content + assert len(response.result.content) > 0 + assert any("error" in c.text.lower() or "broke" in c.text.lower() + for c in response.result.content if hasattr(c, "text")) + + @pytest.mark.asyncio + async def test_unknown_tool_error_has_none_structuredcontent(self, mcp_server): + """Unknown tool error → structuredContent is None, content has error text.""" + from arcade_mcp_server.types import CallToolRequest + + message = CallToolRequest( + jsonrpc="2.0", + id=1, + method="tools/call", + params={"name": "NonExistent.Tool", "arguments": {}}, + ) + + response = await mcp_server._handle_call_tool(message) + + assert response.result.isError is True + assert response.result.structuredContent is None + # Content should mention the unknown tool + assert len(response.result.content) > 0 + content_text = response.result.content[0].text + assert "Unknown tool" in content_text + + @pytest.mark.asyncio + async def test_error_content_still_has_message(self, mcp_server): + """Error responses have the error message in content even with structuredContent=None.""" + from arcade_mcp_server.types import CallToolRequest + + message = CallToolRequest( + jsonrpc="2.0", + id=1, + method="tools/call", + params={"name": "DoesNotExist.Tool", "arguments": {}}, + ) + + response = await mcp_server._handle_call_tool(message) + + assert response.result.structuredContent is None + assert response.result.isError is True + # The content list should not be empty — it carries the error info + assert len(response.result.content) > 0 + assert response.result.content[0].text != "" diff --git a/libs/tests/arcade_mcp_server/test_server.py b/libs/tests/arcade_mcp_server/test_server.py index e032772c..0b5387f6 100644 --- a/libs/tests/arcade_mcp_server/test_server.py +++ b/libs/tests/arcade_mcp_server/test_server.py @@ -2,6 +2,7 @@ import asyncio import contextlib +import json from typing import Annotated from unittest.mock import AsyncMock, Mock @@ -323,12 +324,12 @@ class TestMCPServer: assert isinstance(response, JSONRPCResponse) assert response.id == 3 assert isinstance(response.result, CallToolResult) - assert response.result.structuredContent is not None - assert "authorization_url" in response.result.structuredContent - assert response.result.structuredContent["authorization_url"] == "https://example.com/auth" - assert "message" in response.result.structuredContent - assert "Authorization required" in response.result.structuredContent["message"] - assert "needs your permission" in response.result.structuredContent["message"] + assert response.result.structuredContent is None + content_text = response.result.content[0].text + assert "Authorization required" in content_text + assert "needs your permission" in content_text + # The authorization URL is included in the human-readable message + assert "https://example.com/auth" in content_text @pytest.mark.asyncio async def test_handle_call_tool_with_requires_auth_no_api_key(self, mcp_server): @@ -349,13 +350,12 @@ class TestMCPServer: assert isinstance(response, JSONRPCResponse) assert response.id == 3 assert isinstance(response.result, CallToolResult) - assert response.result.structuredContent is not None - assert "message" in response.result.structuredContent - assert "Missing Arcade API key" in response.result.structuredContent["message"] - assert "requires authorization" in response.result.structuredContent["message"] - assert "arcade login" in response.result.structuredContent["message"] - assert "ARCADE_API_KEY" in response.result.structuredContent["message"] - assert "ARCADE_API_KEY" in response.result.structuredContent["llm_instructions"] + assert response.result.structuredContent is None + content_text = response.result.content[0].text + assert "Missing Arcade API key" in content_text + assert "requires authorization" in content_text + assert "arcade login" in content_text + assert "ARCADE_API_KEY" in content_text @pytest.mark.asyncio async def test_handle_call_tool_not_found(self, mcp_server): @@ -371,8 +371,8 @@ class TestMCPServer: assert isinstance(response, JSONRPCResponse) assert response.result.isError - assert "error" in response.result.structuredContent - assert "Unknown tool" in response.result.structuredContent["error"] + assert response.result.structuredContent is None + assert "Unknown tool" in response.result.content[0].text @pytest.mark.asyncio async def test_handle_message_routing(self, mcp_server, initialized_server_session): @@ -606,10 +606,11 @@ class TestMCPServer: assert isinstance(result, JSONRPCResponse) assert isinstance(result.result, CallToolResult) assert result.result.isError is True - assert "Missing Arcade API key" in result.result.structuredContent["message"] - assert "requires authorization" in result.result.structuredContent["message"] - assert "ARCADE_API_KEY" in result.result.structuredContent["message"] - assert "ARCADE_API_KEY" in result.result.structuredContent["llm_instructions"] + content_text = result.result.content[0].text + assert "Missing Arcade API key" in content_text + assert "requires authorization" in content_text + assert "ARCADE_API_KEY" in content_text + assert result.result.structuredContent is None @pytest.mark.asyncio async def test_check_tool_requirements_auth_pending(self, mcp_server): @@ -645,14 +646,21 @@ class TestMCPServer: tool, tool_context, message, "TestToolkit.auth_tool" ) - # Should return error response with authorization URL + # Should return error response with authorization URL in content assert isinstance(result, JSONRPCResponse) assert isinstance(result.result, CallToolResult) assert result.result.isError is True - assert "authorization_url" in result.result.structuredContent - assert result.result.structuredContent["authorization_url"] == "https://example.com/auth" - assert "Authorization required" in result.result.structuredContent["message"] - assert "needs your permission" in result.result.structuredContent["message"] + assert result.result.structuredContent is None + content_text = result.result.content[0].text + assert "Authorization required" in content_text + assert "needs your permission" in content_text + # The authorization URL is included in the human-readable message + assert "https://example.com/auth" in content_text + # Machine-readable fields (authorization_url, llm_instructions) are in content[1] + assert len(result.result.content) >= 2 + extra_data = json.loads(result.result.content[1].text) + assert extra_data["authorization_url"] == "https://example.com/auth" + assert "llm_instructions" in extra_data @pytest.mark.asyncio async def test_check_tool_requirements_auth_completed(self, mcp_server): @@ -732,9 +740,11 @@ class TestMCPServer: assert isinstance(result, JSONRPCResponse) assert isinstance(result.result, CallToolResult) assert result.result.isError is True - assert "Authorization error" in result.result.structuredContent["message"] - assert "failed to authorize" in result.result.structuredContent["message"] - assert "Auth failed" in result.result.structuredContent["message"] + assert result.result.structuredContent is None + content_text = result.result.content[0].text + assert "Authorization error" in content_text + assert "failed to authorize" in content_text + assert "Auth failed" in content_text @pytest.mark.asyncio async def test_check_tool_requirements_secrets_missing(self, mcp_server): @@ -768,10 +778,11 @@ class TestMCPServer: assert isinstance(result, JSONRPCResponse) assert isinstance(result.result, CallToolResult) assert result.result.isError is True - assert "Missing secret" in result.result.structuredContent["message"] - assert "API_KEY, DATABASE_URL" in result.result.structuredContent["message"] - assert ".env file" in result.result.structuredContent["message"] - assert ".env file" in result.result.structuredContent["llm_instructions"] + assert result.result.structuredContent is None + content_text = result.result.content[0].text + assert "Missing secret" in content_text + assert "API_KEY, DATABASE_URL" in content_text + assert ".env file" in content_text @pytest.mark.asyncio async def test_check_tool_requirements_secrets_partial_missing(self, mcp_server): @@ -812,8 +823,10 @@ class TestMCPServer: assert isinstance(result, JSONRPCResponse) assert isinstance(result.result, CallToolResult) assert result.result.isError is True - assert "DATABASE_URL" in result.result.structuredContent["message"] - assert "API_KEY" not in result.result.structuredContent["message"] + assert result.result.structuredContent is None + content_text = result.result.content[0].text + assert "DATABASE_URL" in content_text + assert "API_KEY" not in content_text @pytest.mark.asyncio async def test_check_tool_requirements_secrets_available(self, mcp_server): @@ -942,7 +955,10 @@ class TestMCPServer: assert isinstance(result, JSONRPCResponse) assert isinstance(result.result, CallToolResult) assert result.result.isError is True - assert "authorization_url" in result.result.structuredContent + assert result.result.structuredContent is None + content_text = result.result.content[0].text + # The authorization URL appears in the human-readable message text + assert "https://example.com/auth" in content_text @pytest.mark.asyncio async def test_http_transport_blocks_tool_with_auth( @@ -967,7 +983,9 @@ class TestMCPServer: assert isinstance(response, JSONRPCResponse) assert isinstance(response.result, CallToolResult) assert response.result.isError is True - assert "HTTP transport" in response.result.structuredContent["message"] + assert response.result.structuredContent is None + content_text = response.result.content[0].text + assert "HTTP transport" in content_text @pytest.mark.asyncio async def test_http_transport_blocks_tool_with_secrets(self, mcp_server): @@ -1032,8 +1050,10 @@ class TestMCPServer: assert isinstance(response, JSONRPCResponse) assert isinstance(response.result, CallToolResult) assert response.result.isError is True - assert "HTTP transport" in response.result.structuredContent["message"] - assert "secrets" in response.result.structuredContent["message"] + assert response.result.structuredContent is None + content_text = response.result.content[0].text + assert "HTTP transport" in content_text + assert "secrets" in content_text @pytest.mark.asyncio async def test_http_transport_blocks_tool_with_both_auth_and_secrets(self, mcp_server): @@ -1110,9 +1130,11 @@ class TestMCPServer: assert isinstance(response, JSONRPCResponse) assert isinstance(response.result, CallToolResult) assert response.result.isError is True - assert "Unsupported transport" in response.result.structuredContent["message"] - assert "HTTP transport" in response.result.structuredContent["message"] - assert "authorization" in response.result.structuredContent["message"] + assert response.result.structuredContent is None + content_text = response.result.content[0].text + assert "Unsupported transport" in content_text + assert "HTTP transport" in content_text + assert "authorization" in content_text @pytest.mark.asyncio async def test_stdio_transport_allows_tool_with_auth(