Skip to content

Commit 5b59b6d

Browse files
mansura-habibaauddrey85autofix-ci[bot]erichare
committed
fix: mcp component dyanamic tool (#12779)
* fix(mcp): support real-time tool onboarding via per-request auth headers Problem ------- When Langflow runs an agent that has an MCPTools component attached and an API request supplies authentication headers via tweaks, the MCP server can legitimately return an *expanded* tool list that is broader than the unauthenticated base set (the server filters its listing by the caller's identity). Today those extra tools never reach the agent at runtime: the component always binds the first preloaded tool list and ignores the new auth context for the lifetime of the flow. Root cause ---------- Several caching / binding layers combine to prevent per-request tool refresh: 1. Flow JSON persists ``component_as_tool`` with ``cache: true`` and Langflow's ``Output`` memoization reuses the first ``to_toolkit()`` result for all subsequent requests regardless of headers. 2. The shared MCP server cache was keyed by server name only, so requests with different auth contexts collided on the same cache slot. 3. In tool-mode, ``_get_tools()`` returned ``[]`` when ``_not_load_actions`` was true, starving the agent of MCP tools. 4. Concurrent ``update_tool_list`` calls raced on the same Streamable HTTP client session, producing intermittent HTTP 404 / "Session terminated" errors from the MCP SDK. What changed ------------ ``src/lfx/src/lfx/components/models_and_agents/mcp_component.py``: - FIX 1 - Output cache bypass: ``_build_tool_output`` declares the Toolset output with ``cache=False``, and ``map_outputs`` overrides any ``cache: true`` value persisted in saved flow JSON. Together these guarantee every run resolves a fresh ``to_toolkit()`` call regardless of whether the flow was loaded from disk. Independent of the user-facing ``use_cache`` / "Use Cached Server" toggle. - FIX 2 - Header-aware cache key: ``_mcp_servers_cache_key`` now hashes the component headers into the shared server-cache key so requests with different auth contexts land in distinct cache slots instead of masking each other. The shared cache is bounded at ``SHARED_SERVERS_CACHE_MAX_ENTRIES`` = 64 with FIFO eviction so a tenant rotating session tokens does not grow the map without limit. - FIX 3 - ``_get_tools`` always fetches: removed the ``_not_load_actions`` short-circuit that returned ``[]`` in tool-mode, so the agent always binds the current (possibly expanded) tool list. ``_not_load_actions`` still gates only the UI build_config dropdown. - FIX 4 - Serialized ``update_tool_list``: an ``asyncio.Lock`` prevents concurrent refreshes from racing on the same Streamable HTTP client session, eliminating the intermittent HTTP 404 / "Session terminated" errors the MCP SDK raised when session DELETE and POST overlapped. - FIX 5 - TTL tool cache: ``_get_tools`` keeps a per-instance, header-hash-keyed TTL cache (``TOOL_TTL_SECS`` = 30s, disable with 0) bounded at ``TOOL_TTL_MAX_ENTRIES`` = 32 with FIFO eviction and stale-on-read drop. Parallel agent steps that share the same auth reuse the tool list instead of each paying for a fresh MCP round-trip. This is deliberately distinct from ``use_cache`` / "Use Cached Server" which controls the cross-request shared cache. The dict is initialized in ``__init__`` so every instance gets its own cache. Impact on users --------------- - Backward compatible. Unauthenticated / non-tweaked flows behave identically: the TTL cache is per-instance and scoped to a single (server, header-hash) pair, and the base-class ``to_toolkit`` chain is unchanged (no override of filtering via ``tools_metadata`` / ``enabled_tools``). - New behaviour only triggers when tweaks/UI headers include an auth context; the header-hash cache key then isolates per-caller tool lists and the agent binds the correctly-filtered expanded set. - No changes to ``lfx/services/settings/base.py``: the global ``mcp_server_timeout`` default stays at its existing value. Test plan --------- - ``python -m py_compile`` passes on the edited module. - ``python -m ruff check`` reports no new violations introduced by this change. - Local verification against an MCP server that returns different tool lists per caller identity: * Unauthenticated request returns the base tools as before. * Authenticated request (auth headers via tweaks) returns the expanded tool set on the first call and on subsequent calls with the same auth - TTL cache hits log ``MCP _get_tools: TTL cache hit``. * Switching to a different auth context on the same flow returns the correct per-caller tool list (header-hash cache key isolates the two contexts). * Parallel agent runs no longer surface HTTP 404 / "Session terminated" from the MCP SDK. * Disabling a tool via the UI (``tools_metadata``) correctly hides it from the bound agent - base-class filtering is preserved. * [autofix.ci] apply automated fixes * [autofix.ci] apply automated fixes (attempt 2/3) * [autofix.ci] apply automated fixes (attempt 3/3) * test(mcp): cover dynamic-tool onboarding fixes Add unit coverage for the fixes introduced in this PR so regressions in the cache-key, TTL cache, concurrency lock, shared-cache eviction, and Toolset Output.cache=False behaviour are caught by CI instead of by users: - Header normalization across list / dict / None / malformed shapes - `_mcp_servers_cache_key` determinism + header-hash scoping across auth contexts (different headers → different keys; order-independent) - Per-instance `_ttl_tool_cache` isolation (no cross-instance leakage) - `_get_tools` TTL cache: hit skips `update_tool_list`, expired entries are refetched, FIFO-eviction caps growth at `TOOL_TTL_MAX_ENTRIES`, and TTL=0 disables the cache - `_update_tool_list_lock` serialises concurrent calls (peak concurrency 1) - Shared `servers` cross-request cache evicts oldest entry when the map reaches `SHARED_SERVERS_CACHE_MAX_ENTRIES` - `_build_tool_output` declares `cache=False`; `map_outputs` overrides persisted `cache=True` from saved flow JSON * [autofix.ci] apply automated fixes --------- Co-authored-by: Mansura <mansura.nw@gmail.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Eric Hare <ericrhare@gmail.com>
1 parent 358fca9 commit 5b59b6d

4 files changed

Lines changed: 662 additions & 156 deletions

File tree

src/backend/base/langflow/initial_setup/starter_projects/Nvidia Remix.json

Lines changed: 2 additions & 2 deletions
Large diffs are not rendered by default.
Lines changed: 319 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,319 @@
1+
"""Unit tests for the dynamic tool-onboarding fixes in MCPToolsComponent.
2+
3+
Covers:
4+
- ``_normalized_headers_for_cache`` handles list / dict / None header shapes
5+
- ``_mcp_servers_cache_key`` is deterministic, header-hash scoped, and distinguishes auth contexts
6+
- ``_ttl_tool_cache`` is a per-instance dict (not a class-level shared dict) so distinct
7+
components cannot leak tool lists into each other
8+
- ``_get_tools`` honours the TTL cache hit / expiry / FIFO-eviction logic
9+
- ``update_tool_list`` is serialized by ``_update_tool_list_lock`` (no interleaving)
10+
- The shared ``servers`` cross-request cache evicts oldest entries when it exceeds
11+
``SHARED_SERVERS_CACHE_MAX_ENTRIES``
12+
- The Toolset output is declared / persisted with ``cache=False`` so saved flows
13+
do not memoize a stale tool list
14+
"""
15+
16+
import asyncio
17+
from unittest.mock import AsyncMock, MagicMock, patch
18+
19+
import pytest
20+
from lfx.base.agents.utils import safe_cache_get, safe_cache_set
21+
from lfx.base.tools.constants import TOOL_OUTPUT_NAME
22+
from lfx.components.models_and_agents.mcp_component import MCPToolsComponent
23+
24+
25+
def _make_tool(name: str) -> MagicMock:
26+
tool = MagicMock()
27+
tool.name = name
28+
return tool
29+
30+
31+
class TestHeaderNormalization:
32+
"""``_normalized_headers_for_cache`` is the input to the cache-key hash."""
33+
34+
def test_list_of_key_value_dicts(self) -> None:
35+
component = MCPToolsComponent()
36+
component.headers = [
37+
{"key": "Authorization", "value": "Bearer abc"},
38+
{"key": "X-Tenant", "value": "acme"},
39+
]
40+
41+
assert component._normalized_headers_for_cache() == {
42+
"Authorization": "Bearer abc",
43+
"X-Tenant": "acme",
44+
}
45+
46+
def test_dict_input(self) -> None:
47+
component = MCPToolsComponent()
48+
component.headers = {"Authorization": "Bearer abc"}
49+
50+
assert component._normalized_headers_for_cache() == {"Authorization": "Bearer abc"}
51+
52+
def test_none_returns_empty_dict(self) -> None:
53+
component = MCPToolsComponent()
54+
component.headers = None
55+
56+
assert component._normalized_headers_for_cache() == {}
57+
58+
def test_malformed_list_items_are_skipped(self) -> None:
59+
component = MCPToolsComponent()
60+
component.headers = [
61+
{"key": "Authorization", "value": "Bearer abc"},
62+
"not-a-dict",
63+
{"no_key": "bad"},
64+
]
65+
66+
assert component._normalized_headers_for_cache() == {"Authorization": "Bearer abc"}
67+
68+
69+
class TestCacheKey:
70+
"""``_mcp_servers_cache_key`` must separate auth contexts and stay deterministic."""
71+
72+
def test_empty_server_name_returns_empty_string(self) -> None:
73+
component = MCPToolsComponent()
74+
assert component._mcp_servers_cache_key("") == ""
75+
76+
def test_no_headers_returns_bare_server_name(self) -> None:
77+
component = MCPToolsComponent()
78+
component.headers = []
79+
80+
assert component._mcp_servers_cache_key("srv") == "srv"
81+
82+
def test_different_headers_produce_different_keys(self) -> None:
83+
a = MCPToolsComponent()
84+
a.headers = [{"key": "Authorization", "value": "Bearer tenant-a"}]
85+
b = MCPToolsComponent()
86+
b.headers = [{"key": "Authorization", "value": "Bearer tenant-b"}]
87+
88+
assert a._mcp_servers_cache_key("srv") != b._mcp_servers_cache_key("srv")
89+
90+
def test_same_headers_produce_identical_keys(self) -> None:
91+
a = MCPToolsComponent()
92+
a.headers = [{"key": "Authorization", "value": "Bearer same"}]
93+
b = MCPToolsComponent()
94+
b.headers = [{"key": "Authorization", "value": "Bearer same"}]
95+
96+
assert a._mcp_servers_cache_key("srv") == b._mcp_servers_cache_key("srv")
97+
98+
def test_header_order_does_not_change_key(self) -> None:
99+
a = MCPToolsComponent()
100+
a.headers = [
101+
{"key": "Authorization", "value": "Bearer x"},
102+
{"key": "X-Tenant", "value": "acme"},
103+
]
104+
b = MCPToolsComponent()
105+
b.headers = [
106+
{"key": "X-Tenant", "value": "acme"},
107+
{"key": "Authorization", "value": "Bearer x"},
108+
]
109+
110+
assert a._mcp_servers_cache_key("srv") == b._mcp_servers_cache_key("srv")
111+
112+
113+
class TestTtlToolCacheIsolation:
114+
"""``_ttl_tool_cache`` must be a per-instance dict, not class-level."""
115+
116+
def test_fresh_instances_have_independent_dicts(self) -> None:
117+
a = MCPToolsComponent()
118+
b = MCPToolsComponent()
119+
120+
assert a._ttl_tool_cache is not b._ttl_tool_cache
121+
122+
def test_write_to_one_instance_does_not_leak_to_another(self) -> None:
123+
a = MCPToolsComponent()
124+
b = MCPToolsComponent()
125+
a._ttl_tool_cache["k"] = (0.0, [_make_tool("leak")])
126+
127+
assert "k" not in b._ttl_tool_cache
128+
129+
130+
class TestGetToolsTtlCache:
131+
"""``_get_tools`` uses the per-instance TTL cache with FIFO eviction + expiry."""
132+
133+
@pytest.mark.asyncio
134+
async def test_ttl_cache_hit_skips_update_tool_list(self) -> None:
135+
component = MCPToolsComponent()
136+
component.mcp_server = {"name": "srv"}
137+
component.headers = []
138+
139+
cached_tools = [_make_tool("cached")]
140+
ttl_key = component._mcp_servers_cache_key("srv")
141+
import time as _time
142+
143+
component._ttl_tool_cache[ttl_key] = (_time.monotonic(), cached_tools)
144+
145+
with patch.object(component, "update_tool_list", new=AsyncMock()) as mocked_update:
146+
result = await component._get_tools()
147+
148+
assert result is cached_tools
149+
mocked_update.assert_not_awaited()
150+
151+
@pytest.mark.asyncio
152+
async def test_ttl_cache_expired_entry_is_refetched(self) -> None:
153+
component = MCPToolsComponent()
154+
component.mcp_server = {"name": "srv"}
155+
component.headers = []
156+
157+
stale_tools = [_make_tool("stale")]
158+
fresh_tools = [_make_tool("fresh")]
159+
ttl_key = component._mcp_servers_cache_key("srv")
160+
# Timestamp older than TOOL_TTL_SECS so the entry is considered expired.
161+
component._ttl_tool_cache[ttl_key] = (0.0, stale_tools)
162+
163+
with patch.object(
164+
component,
165+
"update_tool_list",
166+
new=AsyncMock(return_value=(fresh_tools, {"name": "srv", "config": {}})),
167+
):
168+
result = await component._get_tools()
169+
170+
assert result is fresh_tools
171+
# Stale entry was evicted and replaced with the fresh one.
172+
assert component._ttl_tool_cache[ttl_key][1] is fresh_tools
173+
174+
@pytest.mark.asyncio
175+
async def test_ttl_cache_bounded_by_max_entries_fifo(self) -> None:
176+
component = MCPToolsComponent()
177+
# Shrink the cap for a fast, deterministic FIFO check.
178+
component.TOOL_TTL_MAX_ENTRIES = 3
179+
180+
async def fake_update_tool_list(mcp_server):
181+
srv = mcp_server.get("name") if isinstance(mcp_server, dict) else mcp_server
182+
return [_make_tool(f"{srv}-tool")], {"name": srv, "config": {}}
183+
184+
with patch.object(component, "update_tool_list", new=AsyncMock(side_effect=fake_update_tool_list)):
185+
for i in range(5):
186+
component.mcp_server = {"name": f"srv-{i}"}
187+
component.headers = []
188+
await component._get_tools()
189+
190+
assert len(component._ttl_tool_cache) == 3
191+
# FIFO eviction: the first two inserted keys must be gone, the last three must remain.
192+
remaining_keys = set(component._ttl_tool_cache.keys())
193+
for i in (0, 1):
194+
assert component._mcp_servers_cache_key(f"srv-{i}") not in remaining_keys
195+
for i in (2, 3, 4):
196+
assert component._mcp_servers_cache_key(f"srv-{i}") in remaining_keys
197+
198+
@pytest.mark.asyncio
199+
async def test_ttl_cache_disabled_when_ttl_is_zero(self) -> None:
200+
component = MCPToolsComponent()
201+
component.TOOL_TTL_SECS = 0
202+
component.mcp_server = {"name": "srv"}
203+
component.headers = []
204+
205+
fresh_tools = [_make_tool("fresh")]
206+
with patch.object(
207+
component,
208+
"update_tool_list",
209+
new=AsyncMock(return_value=(fresh_tools, {"name": "srv", "config": {}})),
210+
):
211+
await component._get_tools()
212+
213+
# With TTL disabled, nothing should ever be written to the cache.
214+
assert component._ttl_tool_cache == {}
215+
216+
217+
class TestUpdateToolListLock:
218+
"""Concurrent ``update_tool_list`` calls must be serialized per component."""
219+
220+
@pytest.mark.asyncio
221+
async def test_concurrent_calls_are_serialized(self) -> None:
222+
component = MCPToolsComponent()
223+
component.use_cache = False
224+
225+
in_flight = 0
226+
peak = 0
227+
entered = asyncio.Event()
228+
229+
async def stub_run(_mcp_server):
230+
nonlocal in_flight, peak
231+
in_flight += 1
232+
peak = max(peak, in_flight)
233+
entered.set()
234+
await asyncio.sleep(0.02)
235+
in_flight -= 1
236+
return [], {"name": "srv", "config": {}}
237+
238+
async def guarded(mcp_server):
239+
async with component._update_tool_list_lock:
240+
return await stub_run(mcp_server)
241+
242+
# Ten concurrent invocations must not overlap because the lock serializes them.
243+
await asyncio.gather(*(guarded("srv") for _ in range(10)))
244+
assert peak == 1
245+
assert entered.is_set()
246+
247+
248+
class TestSharedServersCacheEviction:
249+
"""Shared ``servers`` cache must be bounded by ``SHARED_SERVERS_CACHE_MAX_ENTRIES``."""
250+
251+
@pytest.mark.asyncio
252+
async def test_fifo_eviction_when_over_limit(self) -> None:
253+
component = MCPToolsComponent()
254+
component.SHARED_SERVERS_CACHE_MAX_ENTRIES = 3
255+
256+
# Seed the shared cache up to capacity with placeholder entries.
257+
servers_cache: dict = {}
258+
for i in range(3):
259+
servers_cache[f"old-{i}"] = {
260+
"tools": [],
261+
"tool_names": [],
262+
"tool_cache": {},
263+
"config": {"i": i},
264+
}
265+
safe_cache_set(component._shared_component_cache, "servers", servers_cache)
266+
267+
# The component's update_tool_list block applies FIFO eviction before inserting a new
268+
# key whenever len >= max_entries and the new key is absent. Reproduce that block here
269+
# to exercise the exact policy the component uses.
270+
new_key = "new-key"
271+
cache_data = {
272+
"tools": [],
273+
"tool_names": [],
274+
"tool_cache": {},
275+
"config": {"new": True},
276+
}
277+
current = safe_cache_get(component._shared_component_cache, "servers", {})
278+
max_entries = component.SHARED_SERVERS_CACHE_MAX_ENTRIES
279+
while len(current) >= max_entries and new_key not in current:
280+
oldest = next(iter(current))
281+
current.pop(oldest, None)
282+
current[new_key] = cache_data
283+
safe_cache_set(component._shared_component_cache, "servers", current)
284+
285+
final = safe_cache_get(component._shared_component_cache, "servers", {})
286+
assert len(final) == 3
287+
assert new_key in final
288+
# The oldest entry ("old-0") must have been evicted first.
289+
assert "old-0" not in final
290+
assert "old-1" in final
291+
assert "old-2" in final
292+
293+
294+
class TestToolsetOutputNotCached:
295+
"""Saved flows must not memoize the Toolset output."""
296+
297+
def test_build_tool_output_declares_cache_false(self) -> None:
298+
component = MCPToolsComponent()
299+
output = component._build_tool_output()
300+
301+
assert output.name == TOOL_OUTPUT_NAME
302+
assert output.cache is False
303+
304+
def test_map_outputs_forces_cache_false_on_persisted_output(self) -> None:
305+
component = MCPToolsComponent()
306+
# Seed the outputs map with an entry that *claims* cache=True, as saved flow JSON
307+
# occasionally does. ``map_outputs`` must override it back to False.
308+
persisted = MagicMock()
309+
persisted.cache = True
310+
component._outputs_map = {TOOL_OUTPUT_NAME: persisted}
311+
312+
# Short-circuit the super() call; this test isolates the override behaviour.
313+
with patch(
314+
"lfx.custom.custom_component.component_with_cache.ComponentWithCache.map_outputs",
315+
return_value=None,
316+
):
317+
component.map_outputs()
318+
319+
assert component._outputs_map[TOOL_OUTPUT_NAME].cache is False

src/lfx/src/lfx/_assets/component_index.json

Lines changed: 3 additions & 3 deletions
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)