Proper handle extra data in MCP objects by Kludex · Pull Request #1937 · modelcontextprotocol/python-sdk

Kludex · 2026-01-23T10:46:19Z

Now properly matching the specification.

Kludex · 2026-01-23T10:46:45Z

src/mcp/types/_types.py

 class GetTaskPayloadResult(Result):
    """The response to a tasks/result request.
+
    The structure matches the result type of the original request.
    For example, a tools/call task would return the CallToolResult structure.
    """

+    model_config = ConfigDict(extra="allow")


This matches the additionalProperties in the specification.

Kludex · 2026-01-23T10:47:03Z

src/mcp/types/_types.py


-    # TODO(Marcelo): The extra="allow" should be only on specific types e.g. `Meta`, not on the base class.
-    model_config = ConfigDict(extra="allow", alias_generator=to_camel, populate_by_name=True)
+    model_config = ConfigDict(alias_generator=to_camel, populate_by_name=True)


This is now properly done.

(quick comment before I review) just to link related things, there's ongoing discussions around this here: modelcontextprotocol/modelcontextprotocol#2084

Ideally we should have it just ignore extra fields. Wouldn't this cause errors if a newer server sends back extra stuff in an object?

This is ignoring extra fields now. That's what this PR is doing.

There are 3 options: extra being allow, ignore or forbid. By default it's ignore - which is now.

ohh I thought default was forbid

Kludex · 2026-01-23T10:48:20Z

pyproject.toml

    "httpx-sse>=0.4",
-    "pydantic>=2.12.0; python_version >= '3.14'",
-    "pydantic>=2.11.0; python_version < '3.14'",
+    "pydantic>=2.12.0",


We need to bump it for this to work because the extra="allow" behavior doesn't work properly in TypedDicts on previous versions. Given that we are forced to do it on 3.14, I think it makes sense to change this to have a clean state here.

claude · 2026-01-23T10:53:59Z

Code Review - Issue 1: model_config overwrites parent configuration

File: src/mcp/types/_types.py (line 479)

Issue: The model_config in GetTaskPayloadResult completely replaces the parent configuration instead of extending it.

In Pydantic v2, when a child class defines model_config, it completely replaces the parent's config rather than merging. The parent class MCPModel defines:

model_config = ConfigDict(alias_generator=to_camel, populate_by_name=True)

See:

python-sdk/src/mcp/types/_types.py

Lines 33 to 36 in 701779a

    
           """Base class for all MCP protocol types. Allows extra fields for forward compatibility.""" 
        
           model_config = ConfigDict(alias_generator=to_camel, populate_by_name=True)

By setting model_config = ConfigDict(extra="allow") in GetTaskPayloadResult, you lose:

alias_generator=to_camel - converts Python snake_case to JSON camelCase
populate_by_name=True - allows using either the alias or field name

This breaks serialization consistency for this class compared to all other MCP types.

Fix: Merge the configs instead of replacing:

    model_config = ConfigDict(extra="allow", alias_generator=to_camel, populate_by_name=True)

claude · 2026-01-23T10:54:10Z

Code Review - Issue 2: Missing tests for behavior change

Reference: CLAUDE.md Testing Requirements

This PR removes extra="allow" from the base MCPModel class, which is a significant behavior change affecting ~60 model classes. The change impacts:

Data validation: Models now reject/ignore extra fields instead of accepting them
Forward compatibility: The docstring promises "Allows extra fields for forward compatibility" - this behavior is being removed
Storage behavior: Extra fields are no longer preserved on model instances

According to CLAUDE.md:

New features require tests
Bug fixes require regression tests

No tests were added in this PR to verify:

GetTaskPayloadResult still correctly accepts extra fields
Other models properly handle extra fields per the new behavior
Forward compatibility scenarios work as intended
Existing functionality is preserved

Please add tests to cover this validation behavior change.

claude · 2026-01-23T10:54:20Z

Code Review - Issue 3: Breaking changes not documented in migration guide

Reference: CLAUDE.md Breaking Changes

This PR introduces two breaking changes that require migration documentation:

1. Removal of `extra="allow"` from MCPModel

Impact: All MCP protocol types will no longer accept extra fields by default (except GetTaskPayloadResult). This breaks:

Forward compatibility with future protocol versions that add fields
Any code relying on extra fields being preserved
The documented behavior in MCPModel's docstring

2. Pydantic version requirement increase (2.11.0 → 2.12.0)

Impact: Users currently on Pydantic 2.11.0 will be forced to upgrade, which could break their dependency graphs.

According to CLAUDE.md:

When making breaking changes, document them in docs/migration.md. Include:

What changed

Why it changed

How to migrate existing code

Please add a section to docs/migration.md documenting both breaking changes and providing migration guidance for users.

src/mcp/types/_types.py

…hon-sdk into fix-loose-extra

Kludex · 2026-01-23T11:18:59Z

Ah, now I understand what the comment about: Allows extra fields for forward compatibility. means. But that doesn't matter, because if the field is not being handled, it can be just ignored. Isn't that the case?

Kludex · 2026-01-23T11:28:31Z

Ah, now I understand what the comment about: Allows extra fields for forward compatibility. means. But that doesn't matter, because if the field is not being handled, it can be just ignored. Isn't that the case?

Answering my own question: I checked the C# SDK, and they just ignore the fields in older SDK versions, which makes completely sense since you can't handle those fields - but still doesn't error, which is what we want here!

maxisbey · 2026-01-23T14:29:17Z

Ah, now I understand what the comment about: Allows extra fields for forward compatibility. means. But that doesn't matter, because if the field is not being handled, it can be just ignored. Isn't that the case?

Answering my own question: I checked the C# SDK, and they just ignore the fields in older SDK versions, which makes completely sense since you can't handle those fields - but still doesn't error, which is what we want here!

this follows the typescript approach as well, so sounds good to me too

Proper handle extra data in MCP objects

e448b42

Kludex commented Jan 23, 2026

View reviewed changes

Kludex requested a review from maxisbey January 23, 2026 10:48

Merge branch 'main' into fix-loose-extra

701779a

Kludex commented Jan 23, 2026

View reviewed changes

src/mcp/types/_types.py Outdated Show resolved Hide resolved

Kludex added 3 commits January 23, 2026 12:10

Apply suggestion from @Kludex

6e3d2df

add migration note

6784912

Merge branch 'fix-loose-extra' of github.com:modelcontextprotocol/pyt…

6a7b420

…hon-sdk into fix-loose-extra

ignore fields for forward compatiblity

2e9eda9

maxisbey approved these changes Jan 23, 2026

View reviewed changes

Kludex merged commit a1d330d into main Jan 23, 2026
40 checks passed

Kludex deleted the fix-loose-extra branch January 23, 2026 14:35

Conversation

Kludex commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maxisbey Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

claude bot commented Jan 23, 2026

Code Review - Issue 1: model_config overwrites parent configuration

Uh oh!

claude bot commented Jan 23, 2026

Code Review - Issue 2: Missing tests for behavior change

Uh oh!

claude bot commented Jan 23, 2026

Code Review - Issue 3: Breaking changes not documented in migration guide

1. Removal of extra="allow" from MCPModel

2. Pydantic version requirement increase (2.11.0 → 2.12.0)

Uh oh!

Uh oh!

Kludex commented Jan 23, 2026

Uh oh!

Kludex commented Jan 23, 2026

Uh oh!

maxisbey commented Jan 23, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Kludex commented Jan 23, 2026 •

edited

Loading

maxisbey Jan 23, 2026 •

edited

Loading

1. Removal of `extra="allow"` from MCPModel