fix(tools): make _last_resolved_tool_names per-context (#34442)

[Bartok9]

Summary

• Make model_tools._last_resolved_tool_names per-context (backed by a ContextVar) instead of a process-global list.
• Fixes the race where two concurrent sessions in one gateway process clobber each other's resolved tool list, which can cause valid tool calls to be rejected as "tool not available".

Motivation

Closes #34442.

_last_resolved_tool_names (model_tools.py:213) was a module-level List[str]. When two sessions run concurrently in the same gateway process (e.g. two Telegram/Feishu users messaging from different chats), each session's get_tool_definitions() overwrites this global, so session A's handle_function_call() can check against session B's tool list. The existing delegate_tool.py save/restore only covers the delegation chain within a single session, not concurrent top-level sessions (as the issue notes).

This implements the issue's suggested Option A: a ContextVar[List[str]], so each asyncio task / session carries its own copy.

To keep the fix surgical, the module attribute model_tools._last_resolved_tool_names is preserved as a transparently-proxied read/write surface (PEP 562 __getattr__ + a ModuleType subclass __setattr__ that route through the ContextVar). That means the ~9 existing call sites in delegate_tool.py, code_execution, and the test suite that read or assign the attribute directly keep working unchanged — they simply gain per-context isolation. No public signature changes.

Verification

• python3 -m pytest tests/tools/test_delegate.py — 134 passed, 1 deselected (the deselected test_heartbeat_does_not_trip_idle_stale_while_inside_tool is a pre-existing wall-clock timing flake that also fails on clean origin/main without this change).
• New tests in tests/tools/test_delegate.py::TestLastResolvedToolNamesConcurrency:
  • test_attribute_read_write_roundtrip — attribute still behaves like a plain list (incl. the delegate save/restore pattern).
  • test_concurrent_sessions_do_not_clobber_each_other — two concurrent asyncio tasks each retain their own tool list across an await boundary (fails before this change).
• Existing TestToolNamePreservation (delegate save/restore) — still passes.
• python3 -m py_compile model_tools.py — clean.

Did NOT change

• The enabled_tools parameter on handle_function_call() remains the preferred path for execute_code; the per-context value is still only the fallback, exactly as before.
• delegate_tool.py save/restore logic is untouched (it now restores into the calling context, which is the correct scope).

[liuhao1024]

Verified: the ContextVar approach is correct.

Why default=[] is safe: all three assignment sites in the diff use .set([...new list...]) (list comprehension or [t[...] for t in cached]), never in-place mutation (.append(), .extend()). So two concurrent contexts can't clobber each other through the shared default object.

Module __class__ swap (PEP 562 proxy): confirmed working — sys.modules[name].__class__ = _ModelToolsModule causes all subsequent model_tools._last_resolved_tool_names attribute access to route through __getattr__/__setattr__, returning the calling task's ContextVar value. I verified this empirically: after the swap, module.my_attr reads/writes through the ContextVar transparently.

One caveat for future callers: from model_tools import _last_resolved_tool_names (direct name binding at import time) would NOT get the proxy — it snapshots the ContextVar's default at import time. Only model_tools._last_resolved_tool_names (attribute access on the live module object) benefits from per-context isolation. The PR correctly identifies that all existing callers use the attribute-access pattern, so this is not a current issue.

[teknium1]

Thanks for the careful diagnosis and the clean ContextVar implementation, @Bartok9 — your read of the global was correct.

Closing as already-mitigated, after tracing the actual impact:

_last_resolved_tool_names has exactly one production reader — the execute_code sandbox in model_tools.py (sandbox_enabled = enabled_tools if enabled_tools is not None else _last_resolved_tool_names). The agent loop already threads a per-instance list into every dispatch (enabled_tools=list(agent.valid_tool_names) in agent/tool_executor.py and agent/agent_runtime_helpers.py), so the global is only consulted when enabled_tools is None — i.e. an agent with no tools, which can't call execute_code in the first place. The issue's headline scenario (handle_function_call rejecting a tool by checking another session's list) doesn't exist in the code; there is no such gating path.

So the per-instance threading that shipped earlier already closed the real hole. The remaining global is vestigial — it still gets clobbered across sessions, but no production decision reads the clobbered value.

If we ever want the vestigial global gone for cleanliness, it's a ~5-line change (ContextVar + the few internal get/set sites + delegate save/restore), not a 113-line module-type proxy. The PEP-562 __getattr__ + sys.modules[__name__].__class__ swap exists only to avoid editing ~9 call sites, which is the wrong tradeoff for a non-behavioral cleanup.

Appreciate the contribution — the analysis was right, the fix just isn't load-bearing.