Repository navigation
FuncMetadata.pre_parse_json mis-detects str | None as non-string and corrupts JSON-looking string arguments #3055
Description
Activity
I hit this too and confirmed it still reproduces on v2 (main), not just v1 — a str | None tool parameter receiving a JSON-looking string (e.g. '{"blocks": [...]}') gets replaced with a dict before validation and fails with Input should be a valid string. The buggy check is field_info.annotation is not str in pre_parse_json, which is also True for Optional[str].
I'd like to take this. I have a fix + tests ready (full suite passing at 100% coverage locally).
One heads-up on approach: the fix suggested above — "skip if any union member is str" — would regress existing behavior. str | list[str] is deliberately pre-parsed (there's a test for it) so a stringified JSON array can populate the list arm. So I scoped it narrower: skip pre-parsing only when the field is str or a union of only str/None — i.e. no container arm a parsed value could go into. Unions that include a container still pre-parse as before. Happy to go with the broader approach instead if you'd prefer.
Disclosure: I used AI assistance (Claude Code) to help locate the root cause and draft the change; I've reviewed it and understand it.
Thanks for confirming the repro on main — and good to see we converged on the same scoping independently (skip pre-parsing only when the union's non-None arm is exactly str, so str | list[str] etc. still pre-parse as before).
I already have a PR open for this: #3056. Feel free to take a look — no need to duplicate the work. It also just picked up a fix for Annotated[str, Field(...)] | None (idiomatic FastMCP style for described params), flagged by another commenter on the PR, in case that's useful for your version too.
Title:
FuncMetadata.pre_parse_jsonmis-detectsstr | Noneas non-string and corrupts JSON-looking string argumentsEnvironment
mcpversion: 1.27.0Summary
FuncMetadata.pre_parse_json()inmcp/server/fastmcp/utilities/func_metadata.pydecides whether tojson.loads()a string argument based on:This check is meant to catch cases where a client (e.g. Claude Desktop) stringifies a list/dict argument that should really be a Python object. But
field_info.annotation is not strisTrueforOptional[str]/str | Noneas well, since that annotation is not literallystr. So any optional string parameter gets the same treatment as alist/dict/model parameter.If the caller passes a valid string value for such a parameter that also happens to parse as a JSON object or array — e.g. a JSON-serialized template body like
'{"blocks": [...]}'— the value silently gets replaced with adict/listbefore the pydantic argument model is validated. Validation then fails with something like:...even though the caller sent a perfectly valid string and the tool signature explicitly declares
body: str | None.Minimal repro
Expected behavior
A parameter typed
str | None(or anyUnionthat includesstr) should not have its string value re-interpreted as JSON, since the raw string is already a valid value for that field. Pre-parsing should only kick in when a plainstrcould never satisfy the annotation (e.g.list[str],dict[str, Any], a Pydantic model,int, etc.).Suggested fix
Replace the identity check with one that walks
Union/X | Ymembers:Impact
Any FastMCP tool with an
Optional[str](orstr | None) parameter breaks whenever a caller passes a string value that happens to be valid JSON for an object/array (JSON-in-a-string payloads: template bodies, block-based editor content, serialized configs, etc.). We hit this in production with a Unisender email-template MCP server wherebody: str | Noneholds a JSON block structure — everyupdate_email_template/create_email_templatecall with a block-based template failed validation until we monkey-patchedFuncMetadata.pre_parse_jsonlocally with the fix above.