From ac5ea37d1447be0bff99dfa5d80b3cfaf817972d Mon Sep 17 00:00:00 2001 From: dinos Date: Thu, 19 Mar 2026 13:47:47 +0100 Subject: [PATCH] fix(mcp): eliminate duplicate config loading during startup (#69) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(mcp): eliminate duplicate config loading during startup load_mcp_config() was called twice on every startup — once in _mcp_config_signature() to compute the cache key, and again inside load_mcp_tools(). This caused warnings to appear twice. Merge the two calls into _load_mcp_config_once() which returns both the signature and the parsed config, then pass the config through to load_mcp_tools() via a new optional parameter. * test(mcp): fix existing cache tests and add coverage for single-load guarantee - Update fake_load_mcp_tools to accept optional config kwarg - Add test_load_mcp_config_called_once_per_cache_miss: verifies load_mcp_config is called exactly once per cache miss (the bug) - Add test_cached_config_passed_to_load_mcp_tools: verifies the pre-loaded config dict is forwarded to load_mcp_tools --------- Co-authored-by: Xi Zhang <106144707+X-iZhang@users.noreply.github.com> --- EvoScientist/EvoScientist.py | 16 +++++++------- EvoScientist/mcp/client.py | 20 ++++++++++++++---- tests/test_agent_mcp_cache.py | 40 +++++++++++++++++++++++++++++++++-- 3 files changed, 62 insertions(+), 14 deletions(-) diff --git a/EvoScientist/EvoScientist.py b/EvoScientist/EvoScientist.py index 4e5d987..576b149 100644 --- a/EvoScientist/EvoScientist.py +++ b/EvoScientist/EvoScientist.py @@ -89,18 +89,18 @@ def _ensure_chat_model(): # ============================================================================= -def _mcp_config_signature() -> str: - """Return a stable signature for the effective MCP config.""" +def _load_mcp_config_once() -> tuple[str, dict]: + """Load MCP config and return ``(signature, config)``.""" from .mcp.client import load_mcp_config cfg = load_mcp_config() if not cfg: - return "" + return "", {} try: - return json.dumps(cfg, sort_keys=True, ensure_ascii=True) + sig = json.dumps(cfg, sort_keys=True, ensure_ascii=True) except TypeError: - # Fallback for non-JSON-serializable values (should be rare) - return repr(cfg) + sig = repr(cfg) + return sig, cfg def _load_mcp_tools_cached() -> dict[str, list]: @@ -109,7 +109,7 @@ def _load_mcp_tools_cached() -> dict[str, list]: from .mcp import load_mcp_tools - cfg_key = _mcp_config_signature() + cfg_key, cfg = _load_mcp_config_once() if not cfg_key: _MCP_TOOLS_CACHE_KEY = "" _MCP_TOOLS_CACHE_VALUE = {} @@ -118,7 +118,7 @@ def _load_mcp_tools_cached() -> dict[str, list]: if _MCP_TOOLS_CACHE_KEY == cfg_key and _MCP_TOOLS_CACHE_VALUE is not None: return {k: list(v) for k, v in _MCP_TOOLS_CACHE_VALUE.items()} - loaded = load_mcp_tools() + loaded = load_mcp_tools(config=cfg) _MCP_TOOLS_CACHE_KEY = cfg_key _MCP_TOOLS_CACHE_VALUE = {k: list(v) for k, v in loaded.items()} return {k: list(v) for k, v in loaded.items()} diff --git a/EvoScientist/mcp/client.py b/EvoScientist/mcp/client.py index 760d639..9104d0a 100644 --- a/EvoScientist/mcp/client.py +++ b/EvoScientist/mcp/client.py @@ -660,12 +660,17 @@ async def _load_tools(config: dict[str, Any]) -> dict[str, list]: return server_tools -async def aload_mcp_tools() -> dict[str, list]: +async def aload_mcp_tools(config: dict[str, Any] | None = None) -> dict[str, list]: """Async version of :func:`load_mcp_tools`. Prefer this when already inside an async context (e.g. Jupyter, async CLI). + + Args: + config: Optional pre-loaded MCP config dict. When ``None``, + loads from ``~/.config/evoscientist/mcp.yaml``. """ - config = load_mcp_config() + if config is None: + config = load_mcp_config() if not config: return {} try: @@ -676,7 +681,7 @@ async def aload_mcp_tools() -> dict[str, list]: return _route_tools(config, server_tools) -def load_mcp_tools() -> dict[str, list]: +def load_mcp_tools(config: dict[str, Any] | None = None) -> dict[str, list]: """Load MCP tools and return them grouped by target agent. This is the main synchronous entry point. It: @@ -685,12 +690,19 @@ def load_mcp_tools() -> dict[str, list]: 3. Filters tools per server allowlist 4. Routes tools to target agents + Args: + config: Optional pre-loaded MCP config dict. When ``None``, + loads from ``~/.config/evoscientist/mcp.yaml``. Passing a + pre-loaded config avoids duplicate env-var interpolation + warnings when the caller has already loaded the config. + Returns: Dict mapping agent name -> list of LangChain ``BaseTool`` objects. Key ``"main"`` = main agent. Other keys = subagent names. Returns empty dict if no MCP servers are configured. """ - config = load_mcp_config() + if config is None: + config = load_mcp_config() if not config: return {} diff --git a/tests/test_agent_mcp_cache.py b/tests/test_agent_mcp_cache.py index 0c7ead8..43c1024 100644 --- a/tests/test_agent_mcp_cache.py +++ b/tests/test_agent_mcp_cache.py @@ -23,7 +23,7 @@ class TestMcpToolCaching: lambda: {"srv": {"transport": "stdio", "command": "demo"}}, ) - def fake_load_mcp_tools(): + def fake_load_mcp_tools(config=None): calls["load"] += 1 return {"main": [tool]} @@ -44,7 +44,7 @@ class TestMcpToolCaching: def fake_load_config(): return state["cfg"] - def fake_load_mcp_tools(): + def fake_load_mcp_tools(config=None): calls["load"] += 1 return {"main": [f"tool-v{calls['load']}"]} @@ -57,3 +57,39 @@ class TestMcpToolCaching: assert calls["load"] == 2 assert first != second + + def test_load_mcp_config_called_once_per_cache_miss(self, monkeypatch): + """load_mcp_config should be called exactly once per cache miss, + not twice (once for the signature and once inside load_mcp_tools).""" + calls = {"config": 0} + + def counting_load_config(): + calls["config"] += 1 + return {"srv": {"transport": "stdio", "command": "demo"}} + + monkeypatch.setattr( + "EvoScientist.mcp.client.load_mcp_config", counting_load_config + ) + monkeypatch.setattr( + "EvoScientist.mcp.load_mcp_tools", lambda config=None: {"main": []} + ) + + agent_module._load_mcp_tools_cached() + assert calls["config"] == 1 + + def test_cached_config_passed_to_load_mcp_tools(self, monkeypatch): + """load_mcp_tools should receive the pre-loaded config dict.""" + received = {} + + def fake_load_config(): + return {"srv": {"transport": "stdio", "command": "demo"}} + + def fake_load_mcp_tools(config=None): + received["config"] = config + return {"main": []} + + monkeypatch.setattr("EvoScientist.mcp.client.load_mcp_config", fake_load_config) + monkeypatch.setattr("EvoScientist.mcp.load_mcp_tools", fake_load_mcp_tools) + + agent_module._load_mcp_tools_cached() + assert received["config"] == {"srv": {"transport": "stdio", "command": "demo"}}