diff --git a/src/agents/mcp/server.py b/src/agents/mcp/server.py index cdc8927b55..b1e281f41f 100644 --- a/src/agents/mcp/server.py +++ b/src/agents/mcp/server.py @@ -965,6 +965,7 @@ def __init__( # The cache is always dirty at startup, so that we fetch tools at least once self._cache_dirty = True + self._tools_cache_generation = 0 self._tools_list: list[MCPTool] | None = None self.tool_filter = tool_filter @@ -1101,6 +1102,7 @@ async def __aexit__(self, exc_type, exc_value, traceback): def invalidate_tools_cache(self): """Invalidate the tools cache.""" + self._tools_cache_generation += 1 self._cache_dirty = True def _extract_http_errors_from_exception(self, e: BaseException) -> list[Exception]: @@ -1447,6 +1449,7 @@ async def list_tools( if self.cache_tools_list and not self._cache_dirty and self._tools_list: tools = self._tools_list else: + refresh_generation = self._tools_cache_generation tools = [] cursor: str | None = None seen_cursors: set[str | None] = set() @@ -1495,8 +1498,9 @@ async def fetch_pages() -> bool: cursor = None seen_cursors.clear() del fetch_pages - self._tools_list = tools - self._cache_dirty = False + if refresh_generation == self._tools_cache_generation: + self._tools_list = tools + self._cache_dirty = False # Filter tools based on tool_filter filtered_tools = tools @@ -1596,7 +1600,7 @@ def _validate_required_parameters( self, tool_name: str, arguments: dict[str, Any] | None ) -> None: """Validate required tool parameters from cached MCP tool schemas before invocation.""" - if self._tools_list is None: + if self._cache_dirty or self._tools_list is None: return tool = next((item for item in self._tools_list if item.name == tool_name), None) diff --git a/tests/mcp/test_caching.py b/tests/mcp/test_caching.py index dc30f5d61f..86a80ea878 100644 --- a/tests/mcp/test_caching.py +++ b/tests/mcp/test_caching.py @@ -1,9 +1,11 @@ +import asyncio from unittest.mock import AsyncMock, call, patch import pytest -from mcp.types import PaginatedRequestParams +from mcp.types import CallToolResult, PaginatedRequestParams, TextContent from agents import Agent +from agents.exceptions import UserError from agents.mcp import MCPServerStdio from agents.run_context import RunContextWrapper @@ -64,6 +66,101 @@ async def test_server_caching_works( assert result_tools == tools +@pytest.mark.asyncio +@patch("mcp.client.stdio.stdio_client", return_value=DummyStreamsContextManager()) +@patch("mcp.client.session.ClientSession.initialize", new_callable=AsyncMock, return_value=None) +@patch("mcp.client.session.ClientSession.call_tool", new_callable=AsyncMock) +@patch("mcp.client.session.ClientSession.list_tools") +async def test_cache_invalidation_during_refresh_is_preserved( + mock_list_tools: AsyncMock, + mock_call_tool: AsyncMock, + mock_initialize: AsyncMock, + mock_stdio_client, +): + refresh_started = asyncio.Event() + release_refresh = asyncio.Event() + request_count = 0 + responses = [ + ListToolsResult( + tools=[ + MCPTool( + name="tool1", + description="initial", + inputSchema={"required": ["q"]}, + ), + ], + ), + ListToolsResult( + tools=[ + MCPTool( + name="tool1", + description="before-second-invalidation", + inputSchema={}, + ), + ], + ), + ListToolsResult( + tools=[ + MCPTool( + name="tool1", + description="after-second-invalidation", + inputSchema={"required": ["latest"]}, + ), + ], + ), + ] + + async def list_tools(): + nonlocal request_count + request_count += 1 + if request_count == 2: + refresh_started.set() + await release_refresh.wait() + return responses[request_count - 1] + + mock_list_tools.side_effect = list_tools + mock_call_tool.return_value = CallToolResult( + content=[TextContent(type="text", text="ok")], + ) + server = MCPServerStdio( + params={"command": tee}, + cache_tools_list=True, + ) + + async with server: + initial = await server.list_tools() + assert initial[0].description == "initial" + + server.invalidate_tools_cache() + refresh_task = asyncio.create_task(server.list_tools()) + try: + await asyncio.wait_for(refresh_started.wait(), timeout=1) + + server.invalidate_tools_cache() + release_refresh.set() + refreshed = await asyncio.wait_for(refresh_task, timeout=1) + finally: + release_refresh.set() + if not refresh_task.done(): + refresh_task.cancel() + await asyncio.gather(refresh_task, return_exceptions=True) + + assert refreshed[0].description == "before-second-invalidation" + assert (server.cached_tools or [])[0].description == "initial" + + await server.call_tool("tool1", {}) + assert mock_call_tool.call_count == 1 + + latest = await server.list_tools() + assert latest[0].description == "after-second-invalidation" + assert (server.cached_tools or [])[0].description == "after-second-invalidation" + assert request_count == 3 + + with pytest.raises(UserError, match="missing required parameters: latest"): + await server.call_tool("tool1", {}) + assert mock_call_tool.call_count == 1 + + @pytest.mark.asyncio @patch("mcp.client.stdio.stdio_client", return_value=DummyStreamsContextManager()) @patch("mcp.client.session.ClientSession.initialize", new_callable=AsyncMock, return_value=None) diff --git a/tests/mcp/test_client_session_retries.py b/tests/mcp/test_client_session_retries.py index 5d8df49241..57fd284167 100644 --- a/tests/mcp/test_client_session_retries.py +++ b/tests/mcp/test_client_session_retries.py @@ -225,6 +225,7 @@ async def test_call_tool_validates_required_parameters_before_remote_call(): }, ) ] + server._cache_dirty = False # noqa: SLF001 with pytest.raises(UserError, match="missing required parameters: param_a"): await server.call_tool("tool", {}) @@ -246,6 +247,7 @@ async def test_call_tool_with_required_parameters_still_calls_remote_tool(): }, ) ] + server._cache_dirty = False # noqa: SLF001 result = await server.call_tool("tool", {"param_a": "value"}) assert isinstance(result, CallToolResult) @@ -257,6 +259,7 @@ async def test_call_tool_skips_validation_when_tool_is_missing_from_cache(): session = DummySession() server = DummyServer(session=session, retries=0) server._tools_list = [MCPTool(name="different_tool", inputSchema={"required": ["param_a"]})] # noqa: SLF001 + server._cache_dirty = False # noqa: SLF001 await server.call_tool("tool", {}) assert session.call_tool_attempts == 1 @@ -267,6 +270,7 @@ async def test_call_tool_skips_validation_when_required_list_is_absent(): session = DummySession() server = DummyServer(session=session, retries=0) server._tools_list = [MCPTool(name="tool", inputSchema={"type": "object"})] # noqa: SLF001 + server._cache_dirty = False # noqa: SLF001 await server.call_tool("tool", None) assert session.call_tool_attempts == 1 @@ -277,6 +281,7 @@ async def test_call_tool_validates_required_parameters_when_arguments_is_none(): session = DummySession() server = DummyServer(session=session, retries=0) server._tools_list = [MCPTool(name="tool", inputSchema={"required": ["param_a"]})] # noqa: SLF001 + server._cache_dirty = False # noqa: SLF001 with pytest.raises(UserError, match="missing required parameters: param_a"): await server.call_tool("tool", None) @@ -289,6 +294,7 @@ async def test_call_tool_rejects_non_object_arguments_before_remote_call(): session = DummySession() server = DummyServer(session=session, retries=0) server._tools_list = [MCPTool(name="tool", inputSchema={"required": ["param_a"]})] # noqa: SLF001 + server._cache_dirty = False # noqa: SLF001 with pytest.raises(UserError, match="arguments must be an object"): await server.call_tool("tool", cast(dict[str, object] | None, ["bad"]))