From 40e6cbfec76d9fa5baa26caf18f6f013e4cb6ac1 Mon Sep 17 00:00:00 2001 From: Christian Sidak Date: Thu, 16 Apr 2026 21:42:15 -0700 Subject: [PATCH 1/3] fix: skip JSON pre-parsing for str union annotations in pre_parse_json The `pre_parse_json` method used an identity check (`field_info.annotation is not str`) to decide whether to attempt `json.loads` on string values. For union types like `str | None`, this check passed because `str | None` is not identical to `str`, causing string values to be unnecessarily JSON-parsed. This could corrupt string identifiers (UUIDs, etc.) when the parsed result was a non-scalar type like a dict or list. Replace the identity check with a `_should_pre_parse_json` helper that inspects union annotations. Unions containing only simple scalar types (str, int, float, bool, NoneType) skip pre-parsing entirely, while complex unions like `list[str] | None` still benefit from it. Closes #1873 --- .../mcpserver/utilities/func_metadata.py | 26 +++++++- tests/server/mcpserver/test_func_metadata.py | 64 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/src/mcp/server/mcpserver/utilities/func_metadata.py b/src/mcp/server/mcpserver/utilities/func_metadata.py index 062b47d0f..678d12789 100644 --- a/src/mcp/server/mcpserver/utilities/func_metadata.py +++ b/src/mcp/server/mcpserver/utilities/func_metadata.py @@ -148,7 +148,7 @@ def pre_parse_json(self, data: dict[str, Any]) -> dict[str, Any]: continue field_info = key_to_field_info[data_key] - if isinstance(data_value, str) and field_info.annotation is not str: + if isinstance(data_value, str) and _should_pre_parse_json(field_info.annotation): try: pre_parsed = json.loads(data_value) except json.JSONDecodeError: @@ -416,6 +416,30 @@ def _try_create_model_and_schema( return None, None, False + +_SIMPLE_TYPES: frozenset[type] = frozenset({str, int, float, bool, type(None)}) + + +def _should_pre_parse_json(annotation: Any) -> bool: + """Return True if the annotation may benefit from JSON pre-parsing. + + For unions containing only simple scalar types (str, int, float, bool, None), + pre-parsing is skipped because json.loads would corrupt string values that + happen to look like JSON objects or arrays -- e.g. a UUID passed as a string + should stay a string even if the annotation is ``str | None``. + + Complex unions like ``list[str] | None`` still need pre-parsing so that a + JSON-encoded list arriving as a string can be deserialized before Pydantic + validation. + """ + if annotation is str: + return False + origin = get_origin(annotation) + if origin is not None and is_union_origin(origin): + return any(arg not in _SIMPLE_TYPES for arg in get_args(annotation)) + return True + + _no_default = object() diff --git a/tests/server/mcpserver/test_func_metadata.py b/tests/server/mcpserver/test_func_metadata.py index c57d1ee9f..ab8f7cdac 100644 --- a/tests/server/mcpserver/test_func_metadata.py +++ b/tests/server/mcpserver/test_func_metadata.py @@ -551,6 +551,70 @@ def handle_json_payload(payload: str, strict_mode: bool = False) -> str: assert result == f"Handled payload of length {len(json_array_payload)}" + +def test_str_union_pre_parse_preserves_strings(): + """Regression test for #1873: pre_parse_json must not JSON-parse strings + when the annotation is a union of simple types like str | None. + + UUIDs and other string identifiers that start with digits were being + corrupted because json.loads would partially parse them as numbers. + """ + + def func_optional_str(value: str | None = None) -> str: # pragma: no cover + return str(value) + + meta = func_metadata(func_optional_str) + + # A JSON object string must be preserved as-is for str | None + json_obj = '{"database": "postgres", "port": 5432}' + assert meta.pre_parse_json({"value": json_obj})["value"] == json_obj + + # A JSON array string must be preserved as-is for str | None + json_array = '["item1", "item2"]' + assert meta.pre_parse_json({"value": json_array})["value"] == json_array + + # Plain strings are unaffected + assert meta.pre_parse_json({"value": "hello"})["value"] == "hello" + + # UUID-like strings must never be corrupted + uuid_val = "58aa9efd-faad-4901-89e8-99e807a1a2d6" + assert meta.pre_parse_json({"value": uuid_val})["value"] == uuid_val + + # UUID with scientific-notation-like prefix must be preserved + uuid_sci = "3400e37e-b251-49d9-91b0-f8dd8602ff7e" + assert meta.pre_parse_json({"value": uuid_sci})["value"] == uuid_sci + + +def test_complex_union_still_pre_parses(): + """Ensure complex unions like list[str] | None still benefit from + JSON pre-parsing so that serialized lists are deserialized properly. + """ + + def func_optional_list(items: list[str] | None = None) -> str: # pragma: no cover + return str(items) + + meta = func_metadata(func_optional_list) + assert meta.pre_parse_json({"items": '["a", "b", "c"]'})["items"] == ["a", "b", "c"] + + +@pytest.mark.anyio +async def test_str_union_uuid_end_to_end(): + """End-to-end test: a str | None parameter receives the exact UUID string.""" + + def update_task(task_id: str | None = None) -> str: # pragma: no cover + return f"got {task_id}" + + meta = func_metadata(update_task) + uuid_val = "58aa9efd-faad-4901-89e8-99e807a1a2d6" + result = await meta.call_fn_with_arg_validation( + update_task, + fn_is_async=False, + arguments_to_validate={"task_id": uuid_val}, + arguments_to_pass_directly=None, + ) + assert result == f"got {uuid_val}" + + # Tests for structured output functionality From 6acaa25339325af92823d2cf06bdc18b44d2636f Mon Sep 17 00:00:00 2001 From: Christian Sidak Date: Thu, 16 Apr 2026 21:53:45 -0700 Subject: [PATCH 2/3] Fix CI: remove extra blank lines and unnecessary pragma comments Remove extra blank line before _SIMPLE_TYPES and between test functions to satisfy pre-commit linting. Remove # pragma: no cover from test inner functions that are exercised by the tests. --- src/mcp/server/mcpserver/utilities/func_metadata.py | 1 - tests/server/mcpserver/test_func_metadata.py | 7 +++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/mcp/server/mcpserver/utilities/func_metadata.py b/src/mcp/server/mcpserver/utilities/func_metadata.py index 678d12789..621b5e3bb 100644 --- a/src/mcp/server/mcpserver/utilities/func_metadata.py +++ b/src/mcp/server/mcpserver/utilities/func_metadata.py @@ -416,7 +416,6 @@ def _try_create_model_and_schema( return None, None, False - _SIMPLE_TYPES: frozenset[type] = frozenset({str, int, float, bool, type(None)}) diff --git a/tests/server/mcpserver/test_func_metadata.py b/tests/server/mcpserver/test_func_metadata.py index ab8f7cdac..51a16c4a8 100644 --- a/tests/server/mcpserver/test_func_metadata.py +++ b/tests/server/mcpserver/test_func_metadata.py @@ -551,7 +551,6 @@ def handle_json_payload(payload: str, strict_mode: bool = False) -> str: assert result == f"Handled payload of length {len(json_array_payload)}" - def test_str_union_pre_parse_preserves_strings(): """Regression test for #1873: pre_parse_json must not JSON-parse strings when the annotation is a union of simple types like str | None. @@ -560,7 +559,7 @@ def test_str_union_pre_parse_preserves_strings(): corrupted because json.loads would partially parse them as numbers. """ - def func_optional_str(value: str | None = None) -> str: # pragma: no cover + def func_optional_str(value: str | None = None) -> str: return str(value) meta = func_metadata(func_optional_str) @@ -590,7 +589,7 @@ def test_complex_union_still_pre_parses(): JSON pre-parsing so that serialized lists are deserialized properly. """ - def func_optional_list(items: list[str] | None = None) -> str: # pragma: no cover + def func_optional_list(items: list[str] | None = None) -> str: return str(items) meta = func_metadata(func_optional_list) @@ -601,7 +600,7 @@ def func_optional_list(items: list[str] | None = None) -> str: # pragma: no cov async def test_str_union_uuid_end_to_end(): """End-to-end test: a str | None parameter receives the exact UUID string.""" - def update_task(task_id: str | None = None) -> str: # pragma: no cover + def update_task(task_id: str | None = None) -> str: return f"got {task_id}" meta = func_metadata(update_task) From 819130594cfbd1e9007d20d6196e9a9bf643b26a Mon Sep 17 00:00:00 2001 From: Christian Sidak Date: Thu, 16 Apr 2026 22:08:06 -0700 Subject: [PATCH 3/3] Add pragma: no cover to inner test functions not executed by coverage The inner functions func_optional_str and func_optional_list are only used for metadata extraction and pre_parse_json testing -- their bodies are never actually called, so coverage cannot track them. --- tests/server/mcpserver/test_func_metadata.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/server/mcpserver/test_func_metadata.py b/tests/server/mcpserver/test_func_metadata.py index 51a16c4a8..8b5a97cd6 100644 --- a/tests/server/mcpserver/test_func_metadata.py +++ b/tests/server/mcpserver/test_func_metadata.py @@ -559,7 +559,7 @@ def test_str_union_pre_parse_preserves_strings(): corrupted because json.loads would partially parse them as numbers. """ - def func_optional_str(value: str | None = None) -> str: + def func_optional_str(value: str | None = None) -> str: # pragma: no cover return str(value) meta = func_metadata(func_optional_str) @@ -589,7 +589,7 @@ def test_complex_union_still_pre_parses(): JSON pre-parsing so that serialized lists are deserialized properly. """ - def func_optional_list(items: list[str] | None = None) -> str: + def func_optional_list(items: list[str] | None = None) -> str: # pragma: no cover return str(items) meta = func_metadata(func_optional_list)