Skip to content

fix: preserve optional string JSON arguments#2620

Open
pragnyanramtha wants to merge 1 commit into
modelcontextprotocol:mainfrom
pragnyanramtha:pragnyan/preserve-optional-string-json-arguments-1873
Open

fix: preserve optional string JSON arguments#2620
pragnyanramtha wants to merge 1 commit into
modelcontextprotocol:mainfrom
pragnyanramtha:pragnyan/preserve-optional-string-json-arguments-1873

Conversation

@pragnyanramtha
Copy link
Copy Markdown

Summary

  • skip JSON pre-parsing for simple scalar union annotations such as str | None
  • keep JSON pre-parsing for structured optional types such as list[str] | None
  • add FastMCP metadata regressions for optional string and optional list arguments

Fixes #1873.

Context

FastMCP already preserves JSON-looking values for plain str arguments, but str | None still went through json.loads(...) during argument pre-parsing. That means an optional string receiving a JSON object or array string was converted to a dict/list before Pydantic validation, then rejected or corrupted instead of reaching the tool as the annotated string value.

This keeps structured annotations parseable while treating unions of simple scalar types like simple scalar fields.

Validation

  • uv run --frozen pytest tests/server/mcpserver/test_func_metadata.py -> 36 passed
  • uv run --frozen pytest tests/server/mcpserver/test_tool_manager.py -> 50 passed
  • uv run --frozen ruff check src/mcp/server/mcpserver/utilities/func_metadata.py tests/server/mcpserver/test_func_metadata.py -> passed
  • uv run --frozen pyright src/mcp/server/mcpserver/utilities/func_metadata.py tests/server/mcpserver/test_func_metadata.py -> 0 errors
  • git diff --check -> passed

Copilot AI review requested due to automatic review settings May 16, 2026 16:44
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: String parameters starting with digits coerced to numbers, causing UUID data loss

2 participants