From 8f9f1b5f051e1ff95036d72a360de4075da99ac4 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 4 May 2026 01:18:57 +0000 Subject: [PATCH 1/2] fix(server): populate structuredContent.adcp_error on MCP error results When an ADCPError is raised from a handler, return a CallToolResult directly instead of raising ToolError. Returning bypasses FastMCP's _make_error_result path, which drops structuredContent, so both isError=true and structuredContent.adcp_error.code are now propagated to the wire. Adds _extract_adcp_error_fields() to translate.py to extract the structured {code, message, recovery, field?, details?} dict from all ADCPError variants (ADCPError, ADCPTaskError, Error Pydantic model, DecisioningAdcpError). Text fallback in content[] is preserved for backward compatibility. Closes #509 https://claude.ai/code/session_01V4QPiByXeHu2p18TsxjWDF --- src/adcp/server/serve.py | 30 ++++++++---- src/adcp/server/translate.py | 58 +++++++++++++++++++++++ tests/test_server_framework.py | 75 +++++++++++++++++++++++++++++ tests/test_translate.py | 86 +++++++++++++++++++++++++++++++++- 4 files changed, 240 insertions(+), 9 deletions(-) diff --git a/src/adcp/server/serve.py b/src/adcp/server/serve.py index 63215e6fb..b49a494f9 100644 --- a/src/adcp/server/serve.py +++ b/src/adcp/server/serve.py @@ -867,10 +867,26 @@ def _register_tool( """ from mcp.server.fastmcp.tools import Tool from mcp.server.fastmcp.utilities.func_metadata import ArgModelBase, FuncMetadata + from mcp.types import CallToolResult, TextContent from pydantic import ConfigDict from adcp.exceptions import ADCPError - from adcp.server.translate import translate_error + from adcp.server.translate import _extract_adcp_error_fields, translate_error + + def _adcp_exc_to_call_tool_result(exc: ADCPError) -> CallToolResult: + """Return a CallToolResult with isError=True and structuredContent.adcp_error. + + Returning (not raising) a CallToolResult bypasses FastMCP's _make_error_result + path, which would drop structuredContent. The lowlevel server's + isinstance(results, CallToolResult) check passes it through unchanged. + """ + tool_error = translate_error(exc, protocol="mcp") + adcp_error = _extract_adcp_error_fields(exc) + return CallToolResult( + isError=True, + structuredContent={"adcp_error": adcp_error}, + content=[TextContent(type="text", text=str(tool_error))], + ) async def fn(**kwargs: Any) -> dict[str, Any]: # Caller identity: FastMCP does not expose an authenticated principal @@ -911,13 +927,11 @@ async def _call_handler() -> Any: else: result = await _call_handler() except ADCPError as exc: - # Translate AdCP-typed exceptions (IdempotencyConflictError, - # ADCPTaskError with a spec code, etc.) into a ToolError so FastMCP - # surfaces ``is_error=true`` with the spec error code in the - # message text. Clients per AdCP §transport-errors will extract - # the code via either structuredContent.adcp_error (if populated) - # or the text-fallback path. - raise translate_error(exc, protocol="mcp") from exc + # Return CallToolResult directly so FastMCP preserves structuredContent. + # Raising ToolError goes through _make_error_result which drops + # structuredContent; returning bypasses that path (lowlevel server + # checks isinstance(results, CallToolResult) and passes through). + return _adcp_exc_to_call_tool_result(exc) # type: ignore[return-value] if hasattr(result, "model_dump"): return result.model_dump(mode="json", exclude_none=True) # type: ignore[no-any-return] if isinstance(result, dict): diff --git a/src/adcp/server/translate.py b/src/adcp/server/translate.py index 847a1882d..ce520e0bb 100644 --- a/src/adcp/server/translate.py +++ b/src/adcp/server/translate.py @@ -99,6 +99,64 @@ def _build_error_data( return data +def _extract_adcp_error_fields( + exc: ADCPError | Error, +) -> dict[str, Any]: + """Extract the spec-defined adcp_error wire fields from an exception. + + Returns a dict with ``code``, ``message``, ``recovery``, and optional + ``field`` / ``details``. Used to populate + ``CallToolResult.structuredContent["adcp_error"]`` on MCP error results + so the storyboard runner's ``/adcp_error/code`` JSON-pointer assertions + can resolve. + + Mirrors the field-extraction logic in :func:`translate_error` but returns + the structured dict instead of constructing a ``ToolError`` text payload. + """ + try: + from adcp.decisioning.types import AdcpError as DecisioningAdcpError # type: ignore[import-not-found] # noqa: N813 + except Exception: + DecisioningAdcpError = None # type: ignore[assignment,misc,unused-ignore] # noqa: N806 + + field: str | None = None + details: dict[str, Any] | None = None + + if isinstance(exc, Error): + code = exc.code + message = exc.message + recovery = _recovery_for_code(code) + field = exc.field + details = exc.details + elif DecisioningAdcpError is not None and isinstance(exc, DecisioningAdcpError): + code = exc.code + message = exc.args[0] if exc.args else "" + recovery = exc.recovery + field = exc.field + details = exc.details or None + elif isinstance(exc, ADCPError): + code = _error_code_for_exception(exc) + message = exc.message + recovery = _recovery_for_code(code) + errors = getattr(exc, "errors", None) + if errors: + first = errors[0] + field = getattr(first, "field", None) + details = getattr(first, "details", None) + else: + raise TypeError(f"Expected ADCPError or Error, got {type(exc).__name__}") + + result: dict[str, Any] = { + "code": code, + "message": message, + "recovery": recovery, + } + if field: + result["field"] = field + if details: + result["details"] = details + return result + + def translate_error( exc: ADCPError | Error, protocol: Literal["mcp", "a2a"] | Protocol, diff --git a/tests/test_server_framework.py b/tests/test_server_framework.py index c9398b823..09e2c93a1 100644 --- a/tests/test_server_framework.py +++ b/tests/test_server_framework.py @@ -646,6 +646,81 @@ async def test_call_unknown_tool_raises(self): with pytest.raises(KeyError, match="Unknown tool"): await tools.call_tool("nonexistent_tool", {}) + @pytest.mark.asyncio + async def test_adcp_error_returns_structured_call_tool_result(self): + """ADCPError from a handler produces CallToolResult with structuredContent. + + The storyboard runner asserts /adcp_error/code via JSON-pointer on + CallToolResult.structuredContent; this verifies the field is populated. + """ + from unittest.mock import MagicMock + + from mcp.types import CallToolResult + + from adcp.exceptions import ADCPTaskError + from adcp.server.serve import _register_tool + from adcp.types import Error + + err = Error(code="MEDIA_BUY_NOT_FOUND", message="Media buy not found") + exc = ADCPTaskError("get_media_buy", [err]) + + async def caller(kwargs, context=None): + raise exc + + mcp_mock = MagicMock() + mcp_mock._tool_manager._tools = {} + + _register_tool( + mcp_mock, + name="get_media_buy", + description="Get a media buy", + input_schema={"type": "object", "properties": {}}, + caller=caller, + ) + + tool = mcp_mock._tool_manager._tools["get_media_buy"] + result = await tool.fn() + + assert isinstance(result, CallToolResult) + assert result.isError is True + assert result.structuredContent is not None + assert result.structuredContent["adcp_error"]["code"] == "MEDIA_BUY_NOT_FOUND" + assert "Media buy not found" in result.structuredContent["adcp_error"]["message"] + assert len(result.content) > 0 + assert "MEDIA_BUY_NOT_FOUND" in result.content[0].text + + @pytest.mark.asyncio + async def test_adcp_error_text_fallback_preserved(self): + """Text content is preserved alongside structuredContent for backward compat.""" + from unittest.mock import MagicMock + + from adcp.exceptions import ADCPAuthenticationError + from adcp.server.serve import _register_tool + + exc = ADCPAuthenticationError("Token expired", agent_id="agent@example.com") + + async def caller(kwargs, context=None): + raise exc + + mcp_mock = MagicMock() + mcp_mock._tool_manager._tools = {} + + _register_tool( + mcp_mock, + name="get_products", + description="Get products", + input_schema={"type": "object", "properties": {}}, + caller=caller, + ) + + tool = mcp_mock._tool_manager._tools["get_products"] + result = await tool.fn() + + # Text fallback: code appears in content text + assert "AUTH_REQUIRED" in result.content[0].text + # Structured content also set + assert result.structuredContent["adcp_error"]["code"] == "AUTH_REQUIRED" + class TestServerModuleExports: """Test that server module exports are correct.""" diff --git a/tests/test_translate.py b/tests/test_translate.py index 2b4836d0b..7ae42c4da 100644 --- a/tests/test_translate.py +++ b/tests/test_translate.py @@ -13,7 +13,7 @@ ADCPTaskError, ADCPTimeoutError, ) -from adcp.server.translate import normalize_request, translate_error +from adcp.server.translate import _extract_adcp_error_fields, normalize_request, translate_error from adcp.types import Error from adcp.types.core import Protocol @@ -185,6 +185,90 @@ def test_accepts_uppercase_protocol_string(self): assert isinstance(result, ToolError) +# ============================================================================ +# _extract_adcp_error_fields +# ============================================================================ + + +class TestExtractAdcpErrorFields: + """Tests for _extract_adcp_error_fields — the structured dict extractor.""" + + def test_adcp_error_returns_code_message_recovery(self): + """ADCPError produces a dict with code, message, and recovery.""" + exc = ADCPError("something went wrong") + result = _extract_adcp_error_fields(exc) + + assert result["code"] == "INTERNAL_ERROR" + assert result["message"] == "something went wrong" + assert "recovery" in result + + def test_auth_error_code_and_recovery(self): + """ADCPAuthenticationError maps to AUTH_REQUIRED / terminal.""" + exc = ADCPAuthenticationError("Forbidden", agent_id="agent@example.com") + result = _extract_adcp_error_fields(exc) + + assert result["code"] == "AUTH_REQUIRED" + assert result["recovery"] == "terminal" + + def test_timeout_error_code_and_recovery(self): + """ADCPTimeoutError maps to SERVICE_UNAVAILABLE / transient.""" + exc = ADCPTimeoutError("Timed out", timeout=30.0) + result = _extract_adcp_error_fields(exc) + + assert result["code"] == "SERVICE_UNAVAILABLE" + assert result["recovery"] == "transient" + + def test_task_error_uses_first_code(self): + """ADCPTaskError with errors list uses the first error code.""" + err = Error(code="MEDIA_BUY_NOT_FOUND", message="Not found") + exc = ADCPTaskError("get_media_buy", [err]) + result = _extract_adcp_error_fields(exc) + + assert result["code"] == "MEDIA_BUY_NOT_FOUND" + + def test_task_error_lifts_field_from_first_error(self): + """ADCPTaskError lifts the first error's field path.""" + err = Error(code="VALIDATION_ERROR", message="Bad value", field="packages[0].budget") + exc = ADCPTaskError("create_media_buy", [err]) + result = _extract_adcp_error_fields(exc) + + assert result["field"] == "packages[0].budget" + + def test_error_model_direct(self): + """Error Pydantic model is extracted directly.""" + err = Error( + code="BUDGET_TOO_LOW", + message="Below minimum", + field="packages[0].budget", + details={"minimum": 100}, + ) + result = _extract_adcp_error_fields(err) + + assert result["code"] == "BUDGET_TOO_LOW" + assert result["message"] == "Below minimum" + assert result["field"] == "packages[0].budget" + assert result["details"] == {"minimum": 100} + + def test_no_field_when_absent(self): + """field key is absent when not set on the exception.""" + exc = ADCPError("plain error") + result = _extract_adcp_error_fields(exc) + + assert "field" not in result + + def test_no_details_when_absent(self): + """details key is absent when not set.""" + exc = ADCPError("plain error") + result = _extract_adcp_error_fields(exc) + + assert "details" not in result + + def test_rejects_non_adcp_exception(self): + """Non-ADCPError/Error input raises TypeError.""" + with pytest.raises(TypeError, match="Expected ADCPError or Error"): + _extract_adcp_error_fields(ValueError("not adcp")) # type: ignore[arg-type] + + # ============================================================================ # normalize_request — structural transforms # ============================================================================ From e2f6db7fdeddb2ecaee37077f840879d26b2f20f Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 4 May 2026 01:26:22 +0000 Subject: [PATCH 2/2] refine(server): add suggestion to structured error, narrow ImportError, fix mypy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add suggestion field to _extract_adcp_error_fields() for all branches (prevents silent drop on MCP→A2A proxy round-trips) - Narrow except Exception → except ImportError for optional decisioning import - Add pyproject.toml mypy override for adcp.decisioning (optional component) https://claude.ai/code/session_01V4QPiByXeHu2p18TsxjWDF --- pyproject.toml | 6 ++++++ src/adcp/server/translate.py | 14 ++++++++++---- tests/test_translate.py | 14 ++++++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 69f376bd1..d10905fef 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -182,6 +182,12 @@ ignore_missing_imports = true module = ["jsonschema", "jsonschema.*"] ignore_missing_imports = true +# adcp.decisioning is an optional component not bundled with the base SDK; +# its types are imported defensively (try/except) where needed. +[[tool.mypy.overrides]] +module = ["adcp.decisioning", "adcp.decisioning.*"] +ignore_missing_imports = true + [tool.pytest.ini_options] testpaths = ["tests"] asyncio_mode = "auto" diff --git a/src/adcp/server/translate.py b/src/adcp/server/translate.py index ce520e0bb..0a035c67b 100644 --- a/src/adcp/server/translate.py +++ b/src/adcp/server/translate.py @@ -105,7 +105,7 @@ def _extract_adcp_error_fields( """Extract the spec-defined adcp_error wire fields from an exception. Returns a dict with ``code``, ``message``, ``recovery``, and optional - ``field`` / ``details``. Used to populate + ``field`` / ``details`` / ``suggestion``. Used to populate ``CallToolResult.structuredContent["adcp_error"]`` on MCP error results so the storyboard runner's ``/adcp_error/code`` JSON-pointer assertions can resolve. @@ -114,12 +114,13 @@ def _extract_adcp_error_fields( the structured dict instead of constructing a ``ToolError`` text payload. """ try: - from adcp.decisioning.types import AdcpError as DecisioningAdcpError # type: ignore[import-not-found] # noqa: N813 - except Exception: - DecisioningAdcpError = None # type: ignore[assignment,misc,unused-ignore] # noqa: N806 + from adcp.decisioning.types import AdcpError as DecisioningAdcpError # noqa: N813 + except ImportError: + DecisioningAdcpError = None # noqa: N806 field: str | None = None details: dict[str, Any] | None = None + suggestion: str | None = None if isinstance(exc, Error): code = exc.code @@ -127,16 +128,19 @@ def _extract_adcp_error_fields( recovery = _recovery_for_code(code) field = exc.field details = exc.details + suggestion = exc.suggestion elif DecisioningAdcpError is not None and isinstance(exc, DecisioningAdcpError): code = exc.code message = exc.args[0] if exc.args else "" recovery = exc.recovery field = exc.field details = exc.details or None + suggestion = getattr(exc, "suggestion", None) elif isinstance(exc, ADCPError): code = _error_code_for_exception(exc) message = exc.message recovery = _recovery_for_code(code) + suggestion = exc.suggestion errors = getattr(exc, "errors", None) if errors: first = errors[0] @@ -154,6 +158,8 @@ def _extract_adcp_error_fields( result["field"] = field if details: result["details"] = details + if suggestion: + result["suggestion"] = suggestion return result diff --git a/tests/test_translate.py b/tests/test_translate.py index 7ae42c4da..0bcfe862b 100644 --- a/tests/test_translate.py +++ b/tests/test_translate.py @@ -263,6 +263,20 @@ def test_no_details_when_absent(self): assert "details" not in result + def test_suggestion_included_when_present(self): + """suggestion is included when the exception carries one.""" + exc = ADCPError("bad value", suggestion="Set the budget field") + result = _extract_adcp_error_fields(exc) + + assert result["suggestion"] == "Set the budget field" + + def test_no_suggestion_when_absent(self): + """suggestion key is absent when not set.""" + exc = ADCPError("plain error") + result = _extract_adcp_error_fields(exc) + + assert "suggestion" not in result + def test_rejects_non_adcp_exception(self): """Non-ADCPError/Error input raises TypeError.""" with pytest.raises(TypeError, match="Expected ADCPError or Error"):