fix(mcp): stop handing the tools cache to callers - #4424

Summary

_MCPServerWithClientSession hands its live tools cache to callers, so ordinary use of the returned list corrupts the server's cache.

Two exits return the cached list object itself:

• list_tools() sets filtered_tools = tools and returns it unchanged when there is no tool filter. tools is self._tools_list on a cache hit, and on a miss it is the same list that was just assigned to self._tools_list, so the alias exists whether or not the cache was warm — and whether or not cache_tools_list is set, since the fetch path always populates _tools_list.
• The cached_tools property returns self._tools_list directly.

A caller that mutates what it was given silently changes what every later call returns:

tools = await server.list_tools(run_context, agent)
tools.pop()                      # drop a tool this caller does not want to use
await server.list_tools(run_context, agent)   # the dropped tool is gone for everyone

The cached_tools direction is the worse of the two, because the mutation flows toward the model rather than away from it — appending an entry injects a tool definition that is then served as if the MCP server had advertised it:

snapshot = server.cached_tools
snapshot.append(MCPTool(name="injected", inputSchema={}))
await server.list_tools(run_context, agent)   # -> [tool1, tool2, injected]

Both are one-line consequences of the same missing boundary: the cache is internal state and must not leave the object.

What this changes

cached_tools returns a new list, preserving the None sentinel that distinguishes "nothing fetched yet" from "fetched, empty".

list_tools() copies only when the value it is about to return is the cache itself:

if filtered_tools is self._tools_list:
    return list(filtered_tools)
return filtered_tools

The identity check is deliberate rather than an unconditional list(...). Both filter paths already build new lists (_apply_static_tool_filter uses comprehensions, _apply_dynamic_tool_filter accumulates into a fresh list), so copying again would allocate on every filtered call for no benefit. The check also covers the one filtered case that does alias: a static filter dict carrying neither allowed_tool_names nor blocked_tool_names falls through _apply_static_tool_filter and returns its input unchanged. That case is covered by a test rather than left to the reader.

Scope

Only the two places that leak the cache. list_tools() returns a shallow copy — the MCPTool objects are still shared, which matches how the rest of the SDK passes tool definitions around, and the reported failure is list mutation, not tool mutation. Deep-copying every tool on every call would be a real cost for a case with no evidence behind it.

cached_tools on the MCPServer base class returns None and needed no change.

Test plan

Four tests in tests/mcp/test_caching.py:

• test_list_tools_does_not_expose_the_tools_cache — pop() on the returned list; the next call still returns both tools, and mock_list_tools.call_count stays at 1 so the assertion cannot pass by silently refetching.
• test_list_tools_does_not_expose_the_cache_with_a_no_op_static_filter — same, through tool_filter={}, pinning the aliasing filter path.
• test_cached_tools_returns_a_snapshot — append() on the snapshot; neither cached_tools nor list_tools() reports the injected tool.
• test_cached_tools_is_none_before_the_first_list — negative control that the copy preserves None instead of returning [].

Fail-first: the first three fail on main with the fix reverted and the fourth passes, confirming it is a control rather than a second assertion of the same bug. The call_count assertion matters — a caller calling clear() instead of pop() happens to self-heal, because the cache-hit branch tests self._tools_list for truthiness and an emptied cache falls through to a refetch. pop() and append() leave the cache truthy and corrupt.

tests/mcp/ is 456 passed / 24 skipped, unchanged from main. The two existing cached_tools is None assertions in test_server_errors.py and test_client_session_retries.py still hold, and test_paginated_tools_are_cached_before_filtering compares with == rather than is, so the snapshot does not disturb it.

Full stack via .agents/skills/code-change-verification/scripts/run.sh: make format, make lint, make typecheck, make tests all pass.

Issue number

None — found while reading the caching path.

Checks

• I've added new tests, if relevant
• I've run .agents/skills/code-change-verification/scripts/run.sh
• I've confirmed all verification steps pass
• If using Codex, I've run /review before submitting this PR

Disclosure: this change was prepared with AI assistance. The bug, the fix, and the tests were verified locally as described above.