From 36e85b1dff50101e178070131949182078b93744 Mon Sep 17 00:00:00 2001 From: fengting Date: Wed, 2 Sep 2026 01:33:51 +0800 Subject: [PATCH 1/2] fix(mcp): preserve cache invalidation during refresh --- src/agents/mcp/server.py | 8 +++- tests/mcp/test_caching.py | 80 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/src/agents/mcp/server.py b/src/agents/mcp/server.py index cdc8927b55..503c8480bb 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 diff --git a/tests/mcp/test_caching.py b/tests/mcp/test_caching.py index dc30f5d61f..7939f41329 100644 --- a/tests/mcp/test_caching.py +++ b/tests/mcp/test_caching.py @@ -1,3 +1,4 @@ +import asyncio from unittest.mock import AsyncMock, call, patch import pytest @@ -64,6 +65,85 @@ 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.list_tools") +async def test_cache_invalidation_during_refresh_is_preserved( + mock_list_tools: AsyncMock, mock_initialize: AsyncMock, mock_stdio_client +): + refresh_started = asyncio.Event() + release_refresh = asyncio.Event() + request_count = 0 + + async def list_tools(): + nonlocal request_count + request_count += 1 + if request_count == 1: + return ListToolsResult( + tools=[MCPTool(name="initial", inputSchema={})], + ) + if request_count == 2: + refresh_started.set() + await release_refresh.wait() + return ListToolsResult( + tools=[ + MCPTool( + name="before-second-invalidation", + inputSchema={}, + ), + ], + ) + return ListToolsResult( + tools=[ + MCPTool( + name="after-second-invalidation", + inputSchema={}, + ), + ], + ) + + mock_list_tools.side_effect = list_tools + server = MCPServerStdio( + params={"command": tee}, + cache_tools_list=True, + ) + + async with server: + initial = await server.list_tools() + assert [tool.name for tool in initial] == ["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 [tool.name for tool in refreshed] == [ + "before-second-invalidation", + ] + assert [tool.name for tool in (server.cached_tools or [])] == [ + "initial", + ] + + latest = await server.list_tools() + assert [tool.name for tool in latest] == [ + "after-second-invalidation", + ] + assert [tool.name for tool in (server.cached_tools or [])] == [ + "after-second-invalidation", + ] + assert request_count == 3 + + @pytest.mark.asyncio @patch("mcp.client.stdio.stdio_client", return_value=DummyStreamsContextManager()) @patch("mcp.client.session.ClientSession.initialize", new_callable=AsyncMock, return_value=None) From cbbcd49b532b784ba4cfb035872d7807a81850cf Mon Sep 17 00:00:00 2001 From: fengting Date: Wed, 2 Sep 2026 12:11:30 +0800 Subject: [PATCH 2/2] fix(mcp): avoid validation from dirty tool cache --- src/agents/mcp/server.py | 2 +- tests/mcp/test_caching.py | 87 ++++++++++++++---------- tests/mcp/test_client_session_retries.py | 6 ++ 3 files changed, 59 insertions(+), 36 deletions(-) diff --git a/src/agents/mcp/server.py b/src/agents/mcp/server.py index 503c8480bb..b1e281f41f 100644 --- a/src/agents/mcp/server.py +++ b/src/agents/mcp/server.py @@ -1600,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 7939f41329..86a80ea878 100644 --- a/tests/mcp/test_caching.py +++ b/tests/mcp/test_caching.py @@ -2,9 +2,10 @@ 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 @@ -68,42 +69,59 @@ async def test_server_caching_works( @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_initialize: AsyncMock, mock_stdio_client + 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 == 1: - return ListToolsResult( - tools=[MCPTool(name="initial", inputSchema={})], - ) if request_count == 2: refresh_started.set() await release_refresh.wait() - return ListToolsResult( - tools=[ - MCPTool( - name="before-second-invalidation", - inputSchema={}, - ), - ], - ) - return ListToolsResult( - tools=[ - MCPTool( - name="after-second-invalidation", - inputSchema={}, - ), - ], - ) + 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, @@ -111,7 +129,7 @@ async def list_tools(): async with server: initial = await server.list_tools() - assert [tool.name for tool in initial] == ["initial"] + assert initial[0].description == "initial" server.invalidate_tools_cache() refresh_task = asyncio.create_task(server.list_tools()) @@ -127,22 +145,21 @@ async def list_tools(): refresh_task.cancel() await asyncio.gather(refresh_task, return_exceptions=True) - assert [tool.name for tool in refreshed] == [ - "before-second-invalidation", - ] - assert [tool.name for tool in (server.cached_tools or [])] == [ - "initial", - ] + 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 [tool.name for tool in latest] == [ - "after-second-invalidation", - ] - assert [tool.name for tool in (server.cached_tools or [])] == [ - "after-second-invalidation", - ] + 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()) 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"]))