From 9fa8a34e93fb4c7944b0f14fb93b98e08034909c Mon Sep 17 00:00:00 2001 From: Achraf Ez Date: Mon, 10 Aug 2026 16:07:51 +0000 Subject: [PATCH] fix(smolagents): restore nullable propagation and take the type from anyOf SmolAgentsAdapter.adapt() copies inputSchema["properties"] into the smolagents tool without reading inputSchema["required"], so the schema the model sees marks every MCP parameter mandatory, including ones the server declared optional. ToolCallingAgent then refuses a call that omits a parameter the server said was optional. This is a regression. _generate_tool_inputs set inputs[k]["nullable"] in #11; 900880c1 (#23) replaced it with a direct properties copy and the nullable handling went with it. test_optional_sync kept passing because it asserts on call results, and direct calls still work -- only the advertised schema is wrong. Compute the required set from the resolved inputSchema and mark every property outside it nullable. Where a property has no top-level "type", take it from the first non-null anyOf branch instead of defaulting to "string", carrying that branch's enum. That also covers #68: a Literal[...] | None parameter currently arrives as a bare "string" with its enum buried in anyOf. Three tests assert on tool.inputs directly rather than on call results. --- src/mcpadapt/smolagents_adapter.py | 23 ++++++-- tests/test_smolagents_adapter.py | 84 ++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 3 deletions(-) diff --git a/src/mcpadapt/smolagents_adapter.py b/src/mcpadapt/smolagents_adapter.py index d48b542..1bb464f 100644 --- a/src/mcpadapt/smolagents_adapter.py +++ b/src/mcpadapt/smolagents_adapter.py @@ -99,12 +99,29 @@ def adapt( if k != "$defs" } - # make sure mandatory `description` and `type` is provided for each arguments: + # make sure mandatory `description` and `type` is provided for each argument, + # and propagate the JSON Schema `required` list as smolagents' own `nullable` + # key so that optional MCP parameters are not advertised to the model as + # mandatory. `nullable` propagation was added in #11 and lost in #23 when + # `_generate_tool_inputs` was replaced by a direct `properties` copy. + # Resolving `type` from `anyOf` also covers #68. + required = set(input_schema.get("required", [])) for k, v in input_schema["properties"].items(): if "description" not in v: - input_schema["properties"][k]["description"] = "see tool description" + v["description"] = "see tool description" if "type" not in v: - input_schema["properties"][k]["type"] = "string" + # union-typed (anyOf) parameter with no top-level `type`: take the + # first non-null branch's type instead of defaulting to "string". + non_null_options = [ + opt for opt in v.get("anyOf", []) if opt.get("type") != "null" + ] + v["type"] = ( + non_null_options[0]["type"] if non_null_options else "string" + ) + if non_null_options and "enum" in non_null_options[0]: + v["enum"] = non_null_options[0]["enum"] + if k not in required: + v["nullable"] = True # Extract and resolve outputSchema if present (only if structured_output=True) output_schema = None diff --git a/tests/test_smolagents_adapter.py b/tests/test_smolagents_adapter.py index a1ccb2b..2046177 100644 --- a/tests/test_smolagents_adapter.py +++ b/tests/test_smolagents_adapter.py @@ -135,6 +135,90 @@ def test_optional_sync(echo_server_optional_script): assert tools[2](text="hello") == "Echo: hello" +def test_optional_sync_propagates_schema_optionality_and_type( + echo_server_optional_script, +): + """The tool.inputs schema handed to the model must reflect the MCP schema's + own `required` list, not silently mark every parameter as required. + + Regression guard: `nullable` propagation arrived in #11 and was dropped in + #23. It went unnoticed because `test_optional_sync` asserts on call results, + and direct calls keep working -- only the schema shown to the model is wrong. + """ + with MCPAdapt( + StdioServerParameters( + command="uv", args=["run", "python", "-c", echo_server_optional_script] + ), + SmolAgentsAdapter(), + ) as tools: + # echo_tool_optional: `text: str | None = None` -> optional, must be nullable + assert tools[0].inputs["text"].get("nullable") is True + # echo_tool_default_value: `text: str = "empty"` -> optional, must be nullable + assert tools[1].inputs["text"].get("nullable") is True + # echo_tool_union_none: `text: str | None` (no default) -> still required + assert not tools[2].inputs["text"].get("nullable", False) + + +def test_union_typed_parameter_keeps_its_real_type(): + """An `int | None` MCP parameter must not be advertised to the model as + "string" just because pydantic emitted no top-level `type` key (#68).""" + mcp_server_script = dedent( + ''' + from mcp.server.fastmcp import FastMCP + + mcp = FastMCP("Numeric Server") + + @mcp.tool() + def numeric(count: int | None = None) -> str: + """A numeric tool with an optional int parameter""" + return str(count) + + mcp.run() + ''' + ) + with MCPAdapt( + StdioServerParameters( + command="uv", args=["run", "python", "-c", mcp_server_script] + ), + SmolAgentsAdapter(), + ) as tools: + assert tools[0].inputs["count"]["type"] == "integer" + assert tools[0].inputs["count"].get("nullable") is True + + +def test_literal_union_none_keeps_type_and_enum(): + """`Literal[...] | None` -- the exact shape reported in #68 -- must reach the + model as a string with its `enum` intact, not as a bare string.""" + mcp_server_script = dedent( + ''' + from typing import Literal + + from mcp.server.fastmcp import FastMCP + + mcp = FastMCP("Lifecycle Server") + + @mcp.tool() + def lifecycle( + stage: Literal["development", "production", "retired"] | None = None, + ) -> str: + """A tool with an optional Literal parameter""" + return str(stage) + + mcp.run() + ''' + ) + with MCPAdapt( + StdioServerParameters( + command="uv", args=["run", "python", "-c", mcp_server_script] + ), + SmolAgentsAdapter(), + ) as tools: + stage = tools[0].inputs["stage"] + assert stage["type"] == "string" + assert stage["enum"] == ["development", "production", "retired"] + assert stage.get("nullable") is True + + def test_tool_name_with_dashes(): mcp_server_script = dedent( '''