From d176017232acf78640ca8e396af087c3c8771c4c Mon Sep 17 00:00:00 2001 From: fengting Date: Wed, 2 Sep 2026 01:33:51 +0800 Subject: [PATCH 1/4] 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 2fa48d6270ab83b9425df4e1450eebf5f5fe4176 Mon Sep 17 00:00:00 2001 From: fengting Date: Wed, 2 Sep 2026 12:11:30 +0800 Subject: [PATCH 2/4] 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"])) From 453df5aa26e2b55044963e815e347b76de14a857 Mon Sep 17 00:00:00 2001 From: fengting Date: Sat, 5 Sep 2026 19:14:52 +0800 Subject: [PATCH 3/4] fix(mcp): order concurrent cache refresh publication --- src/agents/mcp/server.py | 8 ++++- tests/mcp/test_caching.py | 66 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 1 deletion(-) diff --git a/src/agents/mcp/server.py b/src/agents/mcp/server.py index b1e281f41f..6fcaade1e1 100644 --- a/src/agents/mcp/server.py +++ b/src/agents/mcp/server.py @@ -966,6 +966,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_refresh_sequence = 0 self._tools_list: list[MCPTool] | None = None self.tool_filter = tool_filter @@ -1450,6 +1451,8 @@ async def list_tools( tools = self._tools_list else: refresh_generation = self._tools_cache_generation + self._tools_refresh_sequence += 1 + refresh_sequence = self._tools_refresh_sequence tools = [] cursor: str | None = None seen_cursors: set[str | None] = set() @@ -1498,7 +1501,10 @@ async def fetch_pages() -> bool: cursor = None seen_cursors.clear() del fetch_pages - if refresh_generation == self._tools_cache_generation: + if ( + refresh_generation == self._tools_cache_generation + and refresh_sequence == self._tools_refresh_sequence + ): self._tools_list = tools self._cache_dirty = False diff --git a/tests/mcp/test_caching.py b/tests/mcp/test_caching.py index 86a80ea878..5790c970a4 100644 --- a/tests/mcp/test_caching.py +++ b/tests/mcp/test_caching.py @@ -161,6 +161,72 @@ async def list_tools(): 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) +@patch("mcp.client.session.ClientSession.list_tools") +async def test_older_concurrent_refresh_does_not_overwrite_newer_cache( + mock_list_tools: AsyncMock, + mock_initialize: AsyncMock, + mock_stdio_client, +): + first_refresh_started = asyncio.Event() + release_first_refresh = asyncio.Event() + request_count = 0 + + async def list_tools(): + nonlocal request_count + request_count += 1 + if request_count == 1: + first_refresh_started.set() + await release_first_refresh.wait() + return ListToolsResult( + tools=[ + MCPTool( + name="tool1", + description="first-started", + inputSchema={"required": ["old"]}, + ), + ], + ) + return ListToolsResult( + tools=[ + MCPTool( + name="tool1", + description="second-started", + inputSchema={"required": ["latest"]}, + ), + ], + ) + + mock_list_tools.side_effect = list_tools + server = MCPServerStdio( + params={"command": tee}, + cache_tools_list=True, + ) + + async with server: + first_refresh = asyncio.create_task(server.list_tools()) + try: + await asyncio.wait_for(first_refresh_started.wait(), timeout=1) + second_result = await asyncio.wait_for(server.list_tools(), timeout=1) + assert second_result[0].description == "second-started" + assert (server.cached_tools or [])[0].description == "second-started" + + release_first_refresh.set() + first_result = await asyncio.wait_for(first_refresh, timeout=1) + finally: + release_first_refresh.set() + if not first_refresh.done(): + first_refresh.cancel() + await asyncio.gather(first_refresh, return_exceptions=True) + + assert first_result[0].description == "first-started" + assert (server.cached_tools or [])[0].description == "second-started" + assert (server.cached_tools or [])[0].input_schema == {"required": ["latest"]} + assert request_count == 2 + + @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 14014be08f5f01e9dcbed54c7e46e0d210c3cba7 Mon Sep 17 00:00:00 2001 From: fengting Date: Tue, 8 Sep 2026 20:00:10 +0800 Subject: [PATCH 4/4] fix(mcp): preserve successful refresh publication --- src/agents/mcp/server.py | 4 ++- tests/mcp/test_caching.py | 61 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/agents/mcp/server.py b/src/agents/mcp/server.py index 6fcaade1e1..fad6bed87b 100644 --- a/src/agents/mcp/server.py +++ b/src/agents/mcp/server.py @@ -967,6 +967,7 @@ def __init__( self._cache_dirty = True self._tools_cache_generation = 0 self._tools_refresh_sequence = 0 + self._tools_last_published_refresh_sequence = 0 self._tools_list: list[MCPTool] | None = None self.tool_filter = tool_filter @@ -1503,10 +1504,11 @@ async def fetch_pages() -> bool: del fetch_pages if ( refresh_generation == self._tools_cache_generation - and refresh_sequence == self._tools_refresh_sequence + and refresh_sequence > self._tools_last_published_refresh_sequence ): self._tools_list = tools self._cache_dirty = False + self._tools_last_published_refresh_sequence = refresh_sequence # Filter tools based on tool_filter filtered_tools = tools diff --git a/tests/mcp/test_caching.py b/tests/mcp/test_caching.py index 5790c970a4..cfc58378ac 100644 --- a/tests/mcp/test_caching.py +++ b/tests/mcp/test_caching.py @@ -227,6 +227,67 @@ async def list_tools(): assert request_count == 2 +@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_older_refresh_publishes_when_newer_refresh_fails( + mock_list_tools: AsyncMock, + mock_initialize: AsyncMock, + mock_stdio_client, +): + first_refresh_started = asyncio.Event() + release_first_refresh = asyncio.Event() + request_count = 0 + + async def list_tools(): + nonlocal request_count + request_count += 1 + if request_count == 1: + first_refresh_started.set() + await release_first_refresh.wait() + return ListToolsResult( + tools=[ + MCPTool( + name="tool1", + description="first-started", + inputSchema={"required": ["q"]}, + ), + ], + ) + raise RuntimeError("second refresh failed") + + mock_list_tools.side_effect = list_tools + server = MCPServerStdio( + params={"command": tee}, + cache_tools_list=True, + ) + + async with server: + first_refresh = asyncio.create_task(server.list_tools()) + try: + await asyncio.wait_for(first_refresh_started.wait(), timeout=1) + with pytest.raises(RuntimeError, match="second refresh failed"): + await asyncio.wait_for(server.list_tools(), timeout=1) + + release_first_refresh.set() + first_result = await asyncio.wait_for(first_refresh, timeout=1) + finally: + release_first_refresh.set() + if not first_refresh.done(): + first_refresh.cancel() + await asyncio.gather(first_refresh, return_exceptions=True) + + assert first_result[0].description == "first-started" + cached_tools = server.cached_tools + assert cached_tools is not None + assert cached_tools[0].description == "first-started" + + cached_result = await server.list_tools() + assert cached_result[0].description == "first-started" + assert request_count == 2 + + @pytest.mark.asyncio @patch("mcp.client.stdio.stdio_client", return_value=DummyStreamsContextManager()) @patch("mcp.client.session.ClientSession.initialize", new_callable=AsyncMock, return_value=None)