Skip to content

Conversation

@dsfaccini
Copy link
Contributor

@dsfaccini dsfaccini commented Nov 25, 2025

Fixes #3218 (redundant tools/list calls before every model request)

This PR implements smart caching with proper invalidation for MCP server tools and resources:

  • Only caches when servers advertise capabilities.tools.listChanged / capabilities.resources.listChanged (checks each independently)
  • Listens for notifications/tools/list_changed and notifications/resources/list_changed to invalidate caches when tools change dynamically
  • Clears caches on server reconnection to ensure fresh state

Context:
#3227 added naive caching but was reverted via #3228 because it broke MCP servers that change tools dynamically based on state (per @phemmer's feedback).
This implementation follows @DouweM's recommended approach using the MCP protocol's built-in change notification system.

Changes: Updated MCPServer class to track capabilities, cache tools/resources, handle notifications via message_handler, and invalidate caches accordingly.

@DouweM DouweM self-assigned this Nov 26, 2025
resources=resources_cap is not None,
tools=tools_cap is not None,
tools_list_changed=bool(tools_cap.listChanged) if tools_cap else False,
resources_list_changed=bool(resources_cap.listChanged) if resources_cap else False,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's do this for do this for mcp_capabilities.prompts.listChanged as well, even if we don't support listing prompts yet

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure to what extent beyond adding the field, but I added the same fields and checks as for the other two (resources/tools)

self._tools_cache_valid = True
return result.tools
else:
# Server doesn't support notifications, always fetch fresh
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think typically, if a server doesn't support notifications, that means the tools will never change, so we should cache.

Although @phemmer said on the other PR, "a flag might be good for broken mcp servers which change tools without notifications", which I agree with: cache_tools set to True by default, and I suppose cache_resources as well.

Then we should always read/write the cache unless that flag is disabled, with no branching on the server capability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to this now

@dsfaccini dsfaccini requested a review from DouweM November 27, 2025 23:03
elif isinstance(message, mcp_types.ResourceListChangedNotification):
self._cached_resources = None
elif isinstance(message, mcp_types.PromptListChangedNotification):
self._cached_prompts = None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's remove the cached_prompts stuff because we're not actually loading prompts at all yet

max_retries: The maximum number of times to retry a tool call.
elicitation_callback: Callback function to handle elicitation requests from the server.
cache_tools: Whether to cache the list of tools.
cache_resources: Whether to cache the list of resources.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's link to the full description as it contains very crucial information about when to use this and its behavior. You can use the [...][pydantic_ai.mcp.MCPServer.cache_tools] format

# ============================================================================


async def test_tools_caching_enabled_by_default(mcp_server: MCPServerStdio) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of mocking, can we test against a real MCP server, specifically tests.mcp_server like the rest of this file? You could give it a tool that is hidden by default, plus a special enable_hidden_tool tool that will cause the tool to be able and the notification to be sent.

I assume FastMCP 1.0, which we use in that tests.mcp_server, supports something like that

Copy link
Contributor Author

@dsfaccini dsfaccini left a comment

Choose a reason for hiding this comment

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

I included the live test to trigger the notification that enables the hidden tool.

Looking at mcp.py I stumbled upon the NOTE about the support for audio and the TODO in _mcp.py to support audio. The referenced issue in the FastMCP repo has been closed since then so I added the branch to handle audio, and the respective tests for both.

"""Enable the hidden tool, triggering a ToolListChangedNotification."""
mcp._tool_manager.add_tool(hidden_tool) # pyright: ignore[reportPrivateUsage]
await ctx.session.send_tool_list_changed()
return 'Hidden tool enabled'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a tool to enable the hidden tool



def test_map_from_pai_messages_with_binary_content():
"""Test that map_from_pai_messages correctly converts image and audio content to MCP format."""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

tests mcp image and audio support

type='image',
data=base64.b64decode(chunk.data).decode(),
data=base64.b64encode(chunk.data).decode(),
mimeType=chunk.media_type,
Copy link
Contributor Author

@dsfaccini dsfaccini Nov 30, 2025

Choose a reason for hiding this comment

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

I thought b64decode was wrong here, because we generally use BinaryContent.data to store arbitrary bytes, not base64-encoded bytes. On closer look it's actually right, there are some tests setting BinaryContent.data = base64_bytes, for example, in test_anthropic and test_mistral.

So an idea: should we have a mechanism to avoid this confusion? Like

  1. enforcing BinaryContent.data to be strictly non-base64 and instead having a property that returns it encoded?

would help dumb humans (like me) and llms (currently equivalent to dumb humans) more easily navigagte and extend the functionality in the future.

return messages.BinaryContent(
data=base64.b64decode(part.data), media_type=part.mimeType
) # pragma: no cover
return messages.BinaryContent(data=base64.b64decode(part.data), media_type=part.mimeType)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

'user',
mcp_types.AudioContent(
type='audio',
data=base64.b64encode(chunk.data).decode(),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

FastMCP supports audio now, see comment further down


@dataclass(repr=False, kw_only=True)
class Resource(BaseResource):
"""A resource that can be read from an MCP server.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just a curious question, so far it doesn't look like we're adding any pydantic-ai-functionality, wouldn't it be okay to use the respective mcp_types.* themselves?

tool_names1 = [t.name for t in tools1]
assert 'hidden_tool' not in tool_names1
assert 'enable_hidden_tool' in tool_names1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the live test that uses the enable_hidden_tool to show the hidden_tool

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Agent run calling MCP list tools every time before calling the model provider

2 participants