From 112b91f5211e598485a5e2da11c2dd93c6b2a652 Mon Sep 17 00:00:00 2001 From: Jeremiah Lowin <153965+jlowin@users.noreply.github.com> Date: Tue, 6 May 2025 13:21:38 -0400 Subject: [PATCH] create a setting for legacy parsing --- docs/servers/tools.mdx | 10 +++ src/fastmcp/settings.py | 13 ++++ src/fastmcp/tools/tool.py | 39 +++++++---- src/fastmcp/utilities/tests.py | 41 +++++++++++ tests/server/test_server_interactions.py | 75 -------------------- tests/tools/test_tool.py | 87 +++++++++++++++++++++++- tests/tools/test_tool_manager.py | 59 ++++++++++------ tests/utilities/test_tests.py | 9 +++ 8 files changed, 222 insertions(+), 111 deletions(-) create mode 100644 src/fastmcp/utilities/tests.py create mode 100644 tests/utilities/test_tests.py diff --git a/docs/servers/tools.mdx b/docs/servers/tools.mdx index 66c6c982f..2d2399fb8 100644 --- a/docs/servers/tools.mdx +++ b/docs/servers/tools.mdx @@ -708,3 +708,13 @@ The duplicate behavior options are: - `"error"`: Raises a `ValueError`, preventing the duplicate registration. - `"replace"`: Silently replaces the existing tool with the new one. - `"ignore"`: Keeps the original tool and ignores the new registration attempt. + +### Legacy JSON Parsing + + + +FastMCP 1.0 and < 2.2.10 relied on a crutch that attempted to work around LLM limitations by automatically parsing stringified JSON in tool arguments (e.g., converting `"[1,2,3]"` to `[1,2,3]`). As of FastMCP 2.2.10, this behavior is disabled by default because it circumvents type validation and can lead to unexpected type coercion issues (e.g. parsing "true" as a bool and attempting to call a tool that expected a string, which would fail type validation). + +Most modern LLMs correctly format JSON, but if working with models that unnecessarily stringify JSON (as was the case with Claude Desktop in late 2024), you can re-enable this behavior on your server by setting the environment variable `FASTMCP_TOOL_ATTEMPT_PARSE_JSON_ARGS=1`. + +We strongly recommend leaving this disabled unless necessary. diff --git a/src/fastmcp/settings.py b/src/fastmcp/settings.py index 30870e1c7..808f8a2a9 100644 --- a/src/fastmcp/settings.py +++ b/src/fastmcp/settings.py @@ -27,6 +27,16 @@ class Settings(BaseSettings): test_mode: bool = False log_level: LOG_LEVEL = "INFO" + tool_attempt_parse_json_args: bool = Field( + default=False, + description=""" + Note: this enables a legacy behavior. If True, will attempt to parse + stringified JSON lists and objects strings in tool arguments before + passing them to the tool. This is an old behavior that can create + unexpected type coercion issues, but may be helpful for less powerful + LLMs that stringify JSON instead of passing actual lists and objects. + Defaults to False.""", + ) class ServerSettings(BaseSettings): @@ -83,3 +93,6 @@ class ClientSettings(BaseSettings): ) log_level: LOG_LEVEL = Field(default_factory=lambda: Settings().log_level) + + +settings = Settings() diff --git a/src/fastmcp/tools/tool.py b/src/fastmcp/tools/tool.py index 2bdfaac2c..b039f2775 100644 --- a/src/fastmcp/tools/tool.py +++ b/src/fastmcp/tools/tool.py @@ -10,6 +10,7 @@ from mcp.types import EmbeddedResource, ImageContent, TextContent, ToolAnnotatio from mcp.types import Tool as MCPTool from pydantic import BaseModel, BeforeValidator, Field +import fastmcp from fastmcp.exceptions import ToolError from fastmcp.utilities.json_schema import prune_params from fastmcp.utilities.logging import get_logger @@ -107,6 +108,7 @@ class Tool(BaseModel): context: Context[ServerSessionT, LifespanContextT] | None = None, ) -> list[TextContent | ImageContent | EmbeddedResource]: """Run the tool with arguments.""" + try: injected_args = ( {self.context_kwarg: context} if self.context_kwarg is not None else {} @@ -114,22 +116,29 @@ class Tool(BaseModel): parsed_args = arguments.copy() - # Pre-parse data from JSON in order to handle cases like `["a", "b", "c"]` - # being passed in as JSON inside a string rather than an actual list. - # - # Claude desktop is prone to this - in fact it seems incapable of NOT doing - # this. For sub-models, it tends to pass dicts (JSON objects) as JSON strings, - # which can be pre-parsed here. - for param_name in self.parameters["properties"]: - if isinstance(parsed_args.get(param_name, None), str): - try: - parsed_args[param_name] = json.loads(parsed_args[param_name]) - except json.JSONDecodeError: - pass + if fastmcp.settings.settings.tool_attempt_parse_json_args: + # Pre-parse data from JSON in order to handle cases like `["a", "b", "c"]` + # being passed in as JSON inside a string rather than an actual list. + # + # Claude desktop is prone to this - in fact it seems incapable of NOT doing + # this. For sub-models, it tends to pass dicts (JSON objects) as JSON strings, + # which can be pre-parsed here. + signature = inspect.signature(self.fn) + for param_name in self.parameters["properties"]: + if param_name not in signature.parameters: + continue + arg = parsed_args.get(param_name, None) + if isinstance(arg, str) and signature.parameters[ + param_name + ].annotation not in (int, float, bool): + # if arg.strip().startswith("{") or arg.strip().startswith("["): + try: + parsed_args[param_name] = json.loads(arg) - type_adapter = get_cached_typeadapter( - self.fn, config=frozenset([("coerce_numbers_to_str", True)]) - ) + except json.JSONDecodeError: + pass + + type_adapter = get_cached_typeadapter(self.fn) result = type_adapter.validate_python(parsed_args | injected_args) if inspect.isawaitable(result): result = await result diff --git a/src/fastmcp/utilities/tests.py b/src/fastmcp/utilities/tests.py new file mode 100644 index 000000000..773845e4d --- /dev/null +++ b/src/fastmcp/utilities/tests.py @@ -0,0 +1,41 @@ +import copy +from contextlib import contextmanager +from typing import Any + +from fastmcp.settings import settings + + +@contextmanager +def temporary_settings(**kwargs: Any): + """ + Temporarily override ControlFlow setting values. + + Args: + **kwargs: The settings to override, including nested settings. + + Example: + Temporarily override a setting: + ```python + import fastmcp + from fastmcp.utilities.tests import temporary_settings + + with temporary_settings(log_level='DEBUG'): + assert fastmcp.settings.settings.log_level == 'DEBUG' + assert fastmcp.settings.settings.log_level == 'INFO' + ``` + """ + old_settings = copy.deepcopy(settings.model_dump()) + + try: + # apply the new settings + for attr, value in kwargs.items(): + if not hasattr(settings, attr): + raise AttributeError(f"Setting {attr} does not exist.") + setattr(settings, attr, value) + yield + + finally: + # restore the old settings + for attr in kwargs: + if hasattr(settings, attr): + setattr(settings, attr, old_settings[attr]) diff --git a/tests/server/test_server_interactions.py b/tests/server/test_server_interactions.py index 1b95e20b7..37b82847f 100644 --- a/tests/server/test_server_interactions.py +++ b/tests/server/test_server_interactions.py @@ -349,81 +349,6 @@ class TestToolParameters: assert isinstance(result[0], TextContent) assert result[0].text == "true" - async def test_tool_list_coercion(self): - """Test JSON string to collection type coercion.""" - mcp = FastMCP() - - @mcp.tool() - def process_list(items: list[int]) -> int: - return sum(items) - - async with Client(mcp) as client: - # JSON array string should be coerced to list - result = await client.call_tool( - "process_list", {"items": "[1, 2, 3, 4, 5]"} - ) - assert isinstance(result[0], TextContent) - assert result[0].text == "15" - - async def test_tool_list_coercion_error(self): - """Test that a list coercion error is raised if the input is not a valid list.""" - mcp = FastMCP() - - @mcp.tool() - def process_list(items: list[int]) -> int: - return sum(items) - - async with Client(mcp) as client: - with pytest.raises( - ClientError, - match="Input should be a valid list", - ): - await client.call_tool("process_list", {"items": "['a', 'b', 3]"}) - - async def test_tool_dict_coercion(self): - """Test JSON string to dict type coercion.""" - mcp = FastMCP() - - @mcp.tool() - def process_dict(data: dict[str, int]) -> int: - return sum(data.values()) - - async with Client(mcp) as client: - # JSON object string should be coerced to dict - result = await client.call_tool( - "process_dict", {"data": '{"a": 1, "b": "2", "c": 3}'} - ) - assert isinstance(result[0], TextContent) - assert result[0].text == "6" - - async def test_tool_set_coercion(self): - """Test JSON string to set type coercion.""" - mcp = FastMCP() - - @mcp.tool() - def process_set(items: set[int]) -> int: - assert isinstance(items, set) - return sum(items) - - async with Client(mcp) as client: - result = await client.call_tool("process_set", {"items": "[1, 2, 3, 4, 5]"}) - assert isinstance(result[0], TextContent) - assert result[0].text == "15" - - async def test_tool_tuple_coercion(self): - """Test JSON string to tuple type coercion.""" - mcp = FastMCP() - - @mcp.tool() - def process_tuple(items: tuple[int, str]) -> int: - assert isinstance(items, tuple) - return items[0] + len(items[1]) - - async with Client(mcp) as client: - result = await client.call_tool("process_tuple", {"items": '["1", "two"]'}) - assert isinstance(result[0], TextContent) - assert result[0].text == "4" - async def test_annotated_field_validation(self): mcp = FastMCP() diff --git a/tests/tools/test_tool.py b/tests/tools/test_tool.py index bc1e452cf..249c681be 100644 --- a/tests/tools/test_tool.py +++ b/tests/tools/test_tool.py @@ -2,8 +2,11 @@ import pytest from mcp.types import ImageContent, TextContent from pydantic import BaseModel -from fastmcp import Image +from fastmcp import FastMCP, Image +from fastmcp.client import Client +from fastmcp.exceptions import ClientError from fastmcp.tools.tool import Tool +from fastmcp.utilities.tests import temporary_settings class TestToolFromFunction: @@ -150,9 +153,14 @@ class TestToolFromFunction: x: int = 10 -class TestToolJsonParsing: +class TestLegacyToolJsonParsing: """Tests for Tool's JSON pre-parsing functionality.""" + @pytest.fixture(autouse=True) + def enable_legacy_json_parsing(self): + with temporary_settings(tool_attempt_parse_json_args=True): + yield + async def test_json_string_arguments(self): """Test that JSON string arguments are parsed and validated correctly""" @@ -264,3 +272,78 @@ class TestToolJsonParsing: invalid_json = '{"x": 1, "y": {"invalid": "hello"}}' with pytest.raises(Exception): await tool.run({"data": invalid_json}) + + async def test_tool_list_coercion(self): + """Test JSON string to collection type coercion.""" + mcp = FastMCP() + + @mcp.tool() + def process_list(items: list[int]) -> int: + return sum(items) + + async with Client(mcp) as client: + # JSON array string should be coerced to list + result = await client.call_tool( + "process_list", {"items": "[1, 2, 3, 4, 5]"} + ) + assert isinstance(result[0], TextContent) + assert result[0].text == "15" + + async def test_tool_list_coercion_error(self): + """Test that a list coercion error is raised if the input is not a valid list.""" + mcp = FastMCP() + + @mcp.tool() + def process_list(items: list[int]) -> int: + return sum(items) + + async with Client(mcp) as client: + with pytest.raises( + ClientError, + match="Input should be a valid list", + ): + await client.call_tool("process_list", {"items": "['a', 'b', 3]"}) + + async def test_tool_dict_coercion(self): + """Test JSON string to dict type coercion.""" + mcp = FastMCP() + + @mcp.tool() + def process_dict(data: dict[str, int]) -> int: + return sum(data.values()) + + async with Client(mcp) as client: + # JSON object string should be coerced to dict + result = await client.call_tool( + "process_dict", {"data": '{"a": 1, "b": "2", "c": 3}'} + ) + assert isinstance(result[0], TextContent) + assert result[0].text == "6" + + async def test_tool_set_coercion(self): + """Test JSON string to set type coercion.""" + mcp = FastMCP() + + @mcp.tool() + def process_set(items: set[int]) -> int: + assert isinstance(items, set) + return sum(items) + + async with Client(mcp) as client: + result = await client.call_tool("process_set", {"items": "[1, 2, 3, 4, 5]"}) + assert isinstance(result[0], TextContent) + assert result[0].text == "15" + + async def test_tool_tuple_coercion(self): + """Test JSON string to tuple type coercion.""" + mcp = FastMCP() + + @mcp.tool() + def process_tuple(items: tuple[int, str]) -> int: + assert isinstance(items, tuple) + return items[0] + len(items[1]) + + async with Client(mcp) as client: + result = await client.call_tool("process_tuple", {"items": '["1", "two"]'}) + assert isinstance(result[0], TextContent) + assert result[0].text == "4" diff --git a/tests/tools/test_tool_manager.py b/tests/tools/test_tool_manager.py index d0997d050..5a4f3d122 100644 --- a/tests/tools/test_tool_manager.py +++ b/tests/tools/test_tool_manager.py @@ -14,6 +14,7 @@ from fastmcp import Context, FastMCP, Image from fastmcp.exceptions import NotFoundError, ToolError from fastmcp.tools import ToolManager from fastmcp.tools.tool import Tool +from fastmcp.utilities.tests import temporary_settings class TestAddTools: @@ -320,14 +321,6 @@ class TestCallTools: manager = ToolManager() manager.add_tool_from_fn(sum_vals) - # Try both with plain list and with JSON list - - result = await manager.call_tool("sum_vals", {"vals": "[1, 2, 3]"}) - assert isinstance(result, list) - assert len(result) == 1 - assert isinstance(result[0], TextContent) - assert result[0].text == "6" - assert json.loads(result[0].text) == 6 result = await manager.call_tool("sum_vals", {"vals": [1, 2, 3]}) assert isinstance(result, list) @@ -336,6 +329,24 @@ class TestCallTools: assert result[0].text == "6" assert json.loads(result[0].text) == 6 + async def test_call_tool_with_list_int_input_legacy_behavior(self): + """Legacy behavior -- parse a stringified JSON object""" + + def sum_vals(vals: list[int]) -> int: + return sum(vals) + + manager = ToolManager() + manager.add_tool_from_fn(sum_vals) + # Try both with plain list and with JSON list + + with temporary_settings(tool_attempt_parse_json_args=True): + result = await manager.call_tool("sum_vals", {"vals": "[1, 2, 3]"}) + assert isinstance(result, list) + assert len(result) == 1 + assert isinstance(result[0], TextContent) + assert result[0].text == "6" + assert json.loads(result[0].text) == 6 + async def test_call_tool_with_list_str_or_str_input(self): def concat_strs(vals: list[str] | str) -> str: return vals if isinstance(vals, str) else "".join(vals) @@ -350,23 +361,33 @@ class TestCallTools: assert isinstance(result[0], TextContent) assert result[0].text == "abc" - result = await manager.call_tool("concat_strs", {"vals": '["a", "b", "c"]'}) - assert isinstance(result, list) - assert len(result) == 1 - assert isinstance(result[0], TextContent) - assert result[0].text == "abc" - result = await manager.call_tool("concat_strs", {"vals": "a"}) assert isinstance(result, list) assert len(result) == 1 assert isinstance(result[0], TextContent) assert result[0].text == "a" - result = await manager.call_tool("concat_strs", {"vals": '"a"'}) - assert isinstance(result, list) - assert len(result) == 1 - assert isinstance(result[0], TextContent) - assert result[0].text == "a" + async def test_call_tool_with_list_str_or_str_input_legacy_behavior(self): + """Legacy behavior -- parse a stringified JSON object""" + + def concat_strs(vals: list[str] | str) -> str: + return vals if isinstance(vals, str) else "".join(vals) + + manager = ToolManager() + manager.add_tool_from_fn(concat_strs) + + with temporary_settings(tool_attempt_parse_json_args=True): + result = await manager.call_tool("concat_strs", {"vals": '["a", "b", "c"]'}) + assert isinstance(result, list) + assert len(result) == 1 + assert isinstance(result[0], TextContent) + assert result[0].text == "abc" + + result = await manager.call_tool("concat_strs", {"vals": '"a"'}) + assert isinstance(result, list) + assert len(result) == 1 + assert isinstance(result[0], TextContent) + assert result[0].text == "a" async def test_call_tool_with_complex_model(self): class MyShrimpTank(BaseModel): diff --git a/tests/utilities/test_tests.py b/tests/utilities/test_tests.py new file mode 100644 index 000000000..9d4ea8822 --- /dev/null +++ b/tests/utilities/test_tests.py @@ -0,0 +1,9 @@ +import fastmcp +from fastmcp.utilities.tests import temporary_settings + + +class TestTemporarySettings: + def test_temporary_settings(self): + with temporary_settings(log_level="DEBUG"): + assert fastmcp.settings.settings.log_level == "DEBUG" + assert fastmcp.settings.settings.log_level == "INFO"