Skip to content

Commit eb1c136

Browse files
committed
fix(mcp): order concurrent cache refresh publication
1 parent 70ae165 commit eb1c136

2 files changed

Lines changed: 73 additions & 1 deletion

File tree

src/agents/mcp/server.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -966,6 +966,7 @@ def __init__(
966966
# The cache is always dirty at startup, so that we fetch tools at least once
967967
self._cache_dirty = True
968968
self._tools_cache_generation = 0
969+
self._tools_refresh_sequence = 0
969970
self._tools_list: list[MCPTool] | None = None
970971

971972
self.tool_filter = tool_filter
@@ -1450,6 +1451,8 @@ async def list_tools(
14501451
tools = self._tools_list
14511452
else:
14521453
refresh_generation = self._tools_cache_generation
1454+
self._tools_refresh_sequence += 1
1455+
refresh_sequence = self._tools_refresh_sequence
14531456
tools = []
14541457
cursor: str | None = None
14551458
seen_cursors: set[str | None] = set()
@@ -1498,7 +1501,10 @@ async def fetch_pages() -> bool:
14981501
cursor = None
14991502
seen_cursors.clear()
15001503
del fetch_pages
1501-
if refresh_generation == self._tools_cache_generation:
1504+
if (
1505+
refresh_generation == self._tools_cache_generation
1506+
and refresh_sequence == self._tools_refresh_sequence
1507+
):
15021508
self._tools_list = tools
15031509
self._cache_dirty = False
15041510

tests/mcp/test_caching.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,72 @@ async def list_tools():
161161
assert mock_call_tool.call_count == 1
162162

163163

164+
@pytest.mark.asyncio
165+
@patch("mcp.client.stdio.stdio_client", return_value=DummyStreamsContextManager())
166+
@patch("mcp.client.session.ClientSession.initialize", new_callable=AsyncMock, return_value=None)
167+
@patch("mcp.client.session.ClientSession.list_tools")
168+
async def test_older_concurrent_refresh_does_not_overwrite_newer_cache(
169+
mock_list_tools: AsyncMock,
170+
mock_initialize: AsyncMock,
171+
mock_stdio_client,
172+
):
173+
first_refresh_started = asyncio.Event()
174+
release_first_refresh = asyncio.Event()
175+
request_count = 0
176+
177+
async def list_tools():
178+
nonlocal request_count
179+
request_count += 1
180+
if request_count == 1:
181+
first_refresh_started.set()
182+
await release_first_refresh.wait()
183+
return ListToolsResult(
184+
tools=[
185+
MCPTool(
186+
name="tool1",
187+
description="first-started",
188+
inputSchema={"required": ["old"]},
189+
),
190+
],
191+
)
192+
return ListToolsResult(
193+
tools=[
194+
MCPTool(
195+
name="tool1",
196+
description="second-started",
197+
inputSchema={"required": ["latest"]},
198+
),
199+
],
200+
)
201+
202+
mock_list_tools.side_effect = list_tools
203+
server = MCPServerStdio(
204+
params={"command": tee},
205+
cache_tools_list=True,
206+
)
207+
208+
async with server:
209+
first_refresh = asyncio.create_task(server.list_tools())
210+
try:
211+
await asyncio.wait_for(first_refresh_started.wait(), timeout=1)
212+
second_result = await asyncio.wait_for(server.list_tools(), timeout=1)
213+
assert second_result[0].description == "second-started"
214+
assert (server.cached_tools or [])[0].description == "second-started"
215+
216+
release_first_refresh.set()
217+
first_result = await asyncio.wait_for(first_refresh, timeout=1)
218+
finally:
219+
release_first_refresh.set()
220+
if not first_refresh.done():
221+
first_refresh.cancel()
222+
await asyncio.gather(first_refresh, return_exceptions=True)
223+
224+
assert first_result[0].description == "first-started"
225+
assert (server.cached_tools or [])[0].description == "second-started"
226+
assert (server.cached_tools or [])[0].input_schema == {"required": ["latest"]}
227+
assert request_count == 2
228+
229+
164230
@pytest.mark.asyncio
165231
@patch("mcp.client.stdio.stdio_client", return_value=DummyStreamsContextManager())
166232
@patch("mcp.client.session.ClientSession.initialize", new_callable=AsyncMock, return_value=None)

0 commit comments

Comments
 (0)