Added MCP metadata and annotations to ToolDefinition.metadata

[ChuckJonas]

Fixes #2875

[DouweM]

Let's link to the FilteredToolset docs

[DouweM]

I'd rather have this in test_mcp.py

[ChuckJonas]

While moving this over, I noticed ya'll already have an tests/mcp_server.py so decided I would just add some metadata to these tools to test with...

But it seems like it's running an older version of FastMCP that gets imported from the official MCP python SDK (I don't really understand what that's about)?

This version doesn't seem to have the meta or tags properties on the @tool annotation.

[ChuckJonas]

Looks like mcp.server.fastmcp is the original version and fastMCP 2.0 has been moved to it's own project (I've been using FastMCP 2.0 but didn't realize this).

But also seems like mcp.server.fastmcp is still getting updated?

For now, seems like the cleanest thing I can do is just test that the annotations and output_schema get set on the metadata since meta isn't available.

[DouweM]

@ChuckJonas Yeah the fact that there are now 2 FastMCPs is very confusing. Our test server uses the "official" one from mcp, but we also have an open PR to add a FastMCPToolset that uses the independent FastMCP's client library: #2784. (I've already commented there to suggest supporting ToolDefinition.metadata well).

For now, seems like the cleanest thing I can do is just test that the annotations and output_schema get set on the metadata since meta isn't available.

Makes sense

[strawgate]

In the near future the FastMCP v1 that is in the sdk will be renamed, I think to MCPServer and FastMCP v2 will be the only FastMCP going forward

[ChuckJonas]

@DouweM I think I made the suggested changes

There are a couple things I'm not 100% sure about.

1. The type of metadata. Currently it's just Dict[str, Any]. It would be nice if it was typed somehow

class MCPToolMetadata(TypedDict):
    meta: dict[str, Any] | None
    annotations: dict[str, Any] | None
    output_schema: dict[str, Any] | None

But I think this would require making ToolDefinition generic or just locking it into the MCP structure.

2. It feels like these properties should be available one the process_tool_call... But right now it just returns the tool_name and not the definition. But maybe that's a ticket for another day...

[DouweM]

@ChuckJonas

1. Hmm yeah, I agree it would be nice to have it be typed.

   I spent some time playing around with making ToolDefinition generic in a ToolMetadataT and carrying that through all the necessary methods so that MCPServer(...).filtered(lambda tool_def: ...) can treat tool_def.metadata as a MCPToolMetadata, but I couldn't get it to work because some places require it to be contravariant while others require covariance :/ That's possibly something we could fix with some effort, but not without a breaking change which we can't do until v2.

   Another option would be to define metadata as a union of a MCPToolMetadata dataclass and dict[str, Any], but it'd be annoying for non-MCP users who write custom metadata to have to always do an isinstance before they can read values off it...
   (In case we do go this route: MCPToolMetadata would need a kind: Literal['mcp-tool-metadata'] = 'mcp-tool-metadata' field for union type discrimination, we'd define ToolMetadata: TypeAlias = MCPToolMetadata | dict[str, Any] | None annotated with a pydantic.Discriminator (there are some examples in the repo), and then use that for ToolDefinition.metadata -- we'd also need to have a test to make sure that ToolDefinition round-trip serialization (i.e. object to dict and back to object) works correctly for MCP tool metadata, plain dicts, and None.)

   So I'm thinking keeping it a dict[str, Any] and documenting what fields are available in the case of MCP is the best we can do without a more significant breaking-change ToolDefinition redesign to be generic.

2. Yeah that's worth another issue, as long as we can keep it backward-compatible with older process_tool_call functions we can easily pass the tool_def as well.

Thinking this through now, we'll want to allow users to define tool metadata as well when registering their own tool functions. Can you add the metadata attribute to all the tool registration spots we have currently? You can search for require_parameter_descriptions: to find them.

[ChuckJonas]

@DouweM Should be good to go!

I think I completed all requested changes and CI seems to be passing.

I also tested by running locally with my FastMCP2.0 server and the _meta came through as expected.

LMK if there's anything else.

[DouweM]

@ChuckJonas Any chance you can also do this final request from my comment?

Thinking this through now, we'll want to allow users to define tool metadata as well when registering their own tool functions. Can you add the metadata attribute to all the tool registration spots we have currently? You can search for require_parameter_descriptions: to find them.

Just to make this more generally useful as a feature.

[ChuckJonas]

@DouweM sorry I somehow missed that... I'm not 100% sure I got everything. Also, I came across what I think was an unrelated bug (Added a comment to the line)...

[ChuckJonas]

@DouweM I think was this a bug? It seemed like we were 1 parameter off (requires_approval was being passed into sequential). I assume False is the reasonable default.

Might be worth refactoring these to name parameters in the future to help prevent this type of miss!

[DouweM]

Hmm, I guess there are two issues here:

1. sequential should've been added to these decorators as well
2. as you said, the no-kwargs style makes it way too easy to mess this up.

I'll submit a PR to fix both. Thanks for this feature!

[DouweM]

Hmm, I guess there are two issues here:

1. sequential should've been added to these decorators as well
2. as you said, the no-kwargs style makes it way too easy to mess this up.

I'll submit a PR to fix both. Thanks for this feature!