From 41b4c822c7e946f3245b493fb401b0f30dfb716a Mon Sep 17 00:00:00 2001 From: Radu Mihai Gheorghe Date: Mon, 21 Sep 2026 17:50:54 +0300 Subject: [PATCH] fix(mcp): qualify MCP tool names by the resource that owns them MCP scopes tool names per server. The Unified Runtime put the raw tool name into the one flat tool list it hands the model, so two connected MCP servers exposing the same tool name produced two identically named tools: the provider rejected the request as a duplicate definition before any LLM call, and the graph's tool-node map -- keyed by that name -- would have kept only the last one and routed both servers' calls to it. The name is now mcp-{resource}-tool-{tool}, matching the format the Temporal runtime has always used, so traces and eval assertions read the same on either runtime. Resource names are unique within an agent definition, so the pair is unique by construction; a pair past the provider's 64-character cap is truncated and carries a digest of the full pair. create_tool_node now refuses a duplicate name instead of silently overwriting, naming both tools and the resources they came from. Since #1072 the provider's own 400 no longer relays its message, so a collision from any other source (A2A cards, sanitization) would otherwise surface only as a generic rejection. Alternatives considered: qualifying only on collision, which would make a tool's identity depend on which other servers happen to be attached, so adding a second server would silently rename the first server's tools. PRODEV-1599 --- pyproject.toml | 2 +- .../react/guardrails/guardrails_subgraph.py | 34 +++-- .../agent/tools/mcp/claude.md | 9 ++ .../agent/tools/mcp/mcp_tool.py | 50 +++++++- src/uipath_langchain/agent/tools/tool_node.py | 24 ++++ .../src/simple-http-mcp/agents_api.py | 10 +- .../test_tool_guardrails_subgraph.py | 47 +++++++ tests/agent/tools/test_mcp/claude.md | 2 +- .../test_mcp/test_mcp_client_real_http.py | 6 +- tests/agent/tools/test_mcp/test_mcp_tool.py | 119 ++++++++++++++++-- tests/agent/tools/test_tool_node.py | 24 ++++ uv.lock | 2 +- 12 files changed, 301 insertions(+), 28 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index e74b80dfd..572ab6b09 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "uipath-langchain" -version = "0.18.13" +version = "0.18.14" description = "Python SDK that enables developers to build and deploy LangGraph agents to the UiPath Cloud Platform" readme = { file = "README.md", content-type = "text/markdown" } requires-python = ">=3.11" diff --git a/src/uipath_langchain/agent/react/guardrails/guardrails_subgraph.py b/src/uipath_langchain/agent/react/guardrails/guardrails_subgraph.py index 4f2d2e800..18c5753ed 100644 --- a/src/uipath_langchain/agent/react/guardrails/guardrails_subgraph.py +++ b/src/uipath_langchain/agent/react/guardrails/guardrails_subgraph.py @@ -29,6 +29,7 @@ AgentGuardrailsGraphState, ) from uipath_langchain.agent.react.utils import create_guardrails_state_with_input +from uipath_langchain.agent.tools.utils import sanitize_tool_name _VALIDATOR_ALLOWED_STAGES = { "prompt_injection": {ExecutionStage.PRE_EXECUTION}, @@ -275,6 +276,13 @@ def create_llm_guardrails_subgraph( ) +def _tool_metadata(tool_node: RunnableCallable) -> dict[str, Any] | None: + """A UiPathToolNode's underlying tool metadata, when it carries any.""" + tool = getattr(tool_node, "tool", None) + metadata = getattr(tool, "metadata", None) if tool is not None else None + return metadata if isinstance(metadata, dict) else None + + def _extract_tool_type(tool_node: RunnableCallable) -> str | None: """Extract tool_type from a UiPathToolNode's underlying tool metadata. @@ -284,12 +292,8 @@ def _extract_tool_type(tool_node: RunnableCallable) -> str | None: Returns: The tool_type string if available, otherwise None. """ - tool = getattr(tool_node, "tool", None) - if tool is not None: - metadata = getattr(tool, "metadata", None) - if isinstance(metadata, dict): - return metadata.get("tool_type") - return None + metadata = _tool_metadata(tool_node) + return metadata.get("tool_type") if metadata else None def create_tools_guardrails_subgraph( @@ -309,12 +313,13 @@ def create_tools_guardrails_subgraph( """ result: dict[str, RunnableCallable] = {} for tool_name, tool_node in tool_nodes.items(): - tool_type = _extract_tool_type(tool_node) + metadata = _tool_metadata(tool_node) subgraph = create_tool_guardrails_subgraph( (tool_name, tool_node), guardrails, input_schema=input_schema, - tool_type=tool_type, + tool_type=metadata.get("tool_type") if metadata else None, + display_name=metadata.get("display_name") if metadata else None, ) result[tool_name] = subgraph @@ -420,6 +425,7 @@ def create_tool_guardrails_subgraph( guardrails: Sequence[tuple[BaseGuardrail, GuardrailAction]] | None, input_schema: type[BaseModel] | None = None, tool_type: str | None = None, + display_name: str | None = None, ): """Create a guarded tool node. @@ -428,19 +434,29 @@ def create_tool_guardrails_subgraph( guardrails: Optional sequence of (guardrail, action) tuples. input_schema: Optional input schema to include in state. tool_type: Optional type of the tool (e.g., "process", "escalation", "mcp"). + display_name: The tool's own name, where it differs from the node key. A + selector naming an MCP tool holds that name sanitized, not the qualified + key, and ``guardrails_factory`` accepts it when validating the same + selector. Returns: Either the original tool node callable (if no matching guardrails) or a compiled LangGraph subgraph that enforces the matching tool guardrails. """ tool_name, _ = tool_node + # Tool-scope selectors are sanitized before they reach here + # (_sanitize_selector_tool_names), so the display name is normalized the same + # way before being matched against them. + selector_names = {tool_name} + if display_name: + selector_names.add(sanitize_tool_name(display_name)) applicable_guardrails = [ (guardrail, action) for (guardrail, action) in (guardrails or []) if guardrail.selector is not None and GuardrailScope.TOOL in guardrail.selector.scopes and guardrail.selector.match_names is not None - and tool_name in guardrail.selector.match_names + and not selector_names.isdisjoint(guardrail.selector.match_names) ] if applicable_guardrails is None or len(applicable_guardrails) == 0: return tool_node[1] diff --git a/src/uipath_langchain/agent/tools/mcp/claude.md b/src/uipath_langchain/agent/tools/mcp/claude.md index b0828979f..a0f6c2ac6 100644 --- a/src/uipath_langchain/agent/tools/mcp/claude.md +++ b/src/uipath_langchain/agent/tools/mcp/claude.md @@ -457,6 +457,15 @@ finally: Creates tools for a single MCP resource config using an existing McpClient. +**Tool naming:** the LLM-facing name is `mcp-{resource}-tool-{tool}`, built by +`mcp_tool_identity()`. MCP scopes tool names per server, so the resource is what +makes the name unique across the flat tool list the model is given; resource names +are unique within an agent definition, so the pair is unique by construction. A pair +past the provider's 64-character cap spends the cap on the tool name first, since that +is the part the model reads when choosing between tools, and shortens the resource to +a stub; a digest of the full pair keeps two shortened names apart. The tool's own name +stays on `metadata["display_name"]`, which is what span titles show. + The discovery mode comes from `config.tools_configuration.discovery_mode`, defaulting to cached when `tools_configuration` is unset. diff --git a/src/uipath_langchain/agent/tools/mcp/mcp_tool.py b/src/uipath_langchain/agent/tools/mcp/mcp_tool.py index a06414389..407d5e988 100644 --- a/src/uipath_langchain/agent/tools/mcp/mcp_tool.py +++ b/src/uipath_langchain/agent/tools/mcp/mcp_tool.py @@ -1,3 +1,4 @@ +import hashlib import logging from contextlib import AsyncExitStack, asynccontextmanager from typing import Any, AsyncGenerator @@ -26,6 +27,53 @@ logger: logging.Logger = logging.getLogger(__name__) +# Providers cap a tool name at 64 characters; the digest that keeps a shortened +# name unique costs 8 hex characters plus its separator. +_MAX_TOOL_NAME_LENGTH = 64 +_DIGEST_LENGTH = 8 +# A separator no name can contain, so distinct pairs cannot hash alike. +_IDENTITY_SEPARATOR = "\x00" +# "mcp-" + "-tool-", the scaffolding the resource and tool names sit in. +_SCAFFOLDING_LENGTH = 10 +# What a shortened resource keeps, enough to tell two servers apart by eye. +_MIN_RESOURCE_LENGTH = 8 + + +def mcp_tool_identity(resource_name: str, tool_name: str) -> str: + """The LLM-facing name for an MCP tool, qualified by the resource that owns it. + + MCP scopes tool names per server, so the resource is what makes the name unique + across the one flat tool list the model is given. Resource names are unique + within an agent definition, so ```` identifies one tool. Two + resource names that normalize alike, such as ``Case Management`` and + ``Case_Management``, share an identity; ``create_tool_node`` reports that pair + at startup. + + The format matches the Temporal runtime's, so a trace or an eval assertion reads + the same on either runtime. + + A pair past the 64-character cap spends the cap on the tool name first, since + that is the part the model reads when choosing between tools, and shortens the + resource down to a stub. A digest of the full pair keeps two shortened names + apart. + """ + resource = sanitize_tool_name(resource_name).lower() + tool = sanitize_tool_name(tool_name).lower() + qualified = f"mcp-{resource}-tool-{tool}" + if len(qualified) <= _MAX_TOOL_NAME_LENGTH: + return qualified + + budget = ( + _MAX_TOOL_NAME_LENGTH - _SCAFFOLDING_LENGTH - _DIGEST_LENGTH - 1 + ) # shared by the two names + tool = tool[: budget - min(len(resource), _MIN_RESOURCE_LENGTH)] + resource = resource[: budget - len(tool)] + digest = hashlib.blake2s( + _IDENTITY_SEPARATOR.join((resource_name, tool_name)).encode(), + digest_size=_DIGEST_LENGTH // 2, + ).hexdigest() + return f"mcp-{resource}-tool-{tool}-{digest}" + def _breaking_schema_change(cached: dict[str, Any], live: dict[str, Any]) -> bool: """Whether the live input schema differs from the cached one in a way that would @@ -278,7 +326,7 @@ async def create_mcp_tools( # it can refresh its own args_schema on schema drift (see _refresh_tool_schema). tool_holder: dict[str, BaseTool] = {} structured_tool = StructuredToolWithArgumentProperties( - name=sanitize_tool_name(mcp_tool.name), + name=mcp_tool_identity(config.name, mcp_tool.name), description=mcp_tool.description, args_schema=mcp_tool.input_schema, coroutine=build_mcp_tool( diff --git a/src/uipath_langchain/agent/tools/tool_node.py b/src/uipath_langchain/agent/tools/tool_node.py index f28a3b28f..f7a18dfb1 100644 --- a/src/uipath_langchain/agent/tools/tool_node.py +++ b/src/uipath_langchain/agent/tools/tool_node.py @@ -16,6 +16,8 @@ from uipath_langchain.agent.exceptions import ( AgentRuntimeError, AgentRuntimeErrorCode, + AgentStartupError, + AgentStartupErrorCode, ) from uipath_langchain.agent.react.types import AgentGraphState from uipath_langchain.agent.react.utils import ( @@ -323,6 +325,12 @@ def set_tool_wrappers( self.awrapper = awrapper +def _describe_tool_origin(tool: BaseTool) -> str: + """Name the resource a tool came from, for a name-collision message.""" + resource = (tool.metadata or {}).get("resource_name") + return f"'{tool.name}' from resource '{resource}'" if resource else f"'{tool.name}'" + + def create_tool_node(tools: Sequence[BaseTool]) -> dict[str, UiPathToolNode]: """Create individual ToolNode for each tool. @@ -332,9 +340,25 @@ def create_tool_node(tools: Sequence[BaseTool]) -> dict[str, UiPathToolNode]: Returns: Dict mapping tool.name -> UiPathToolNode. Each tool gets its own dedicated node for middleware composition. + + Raises: + AgentStartupError: Two tools share a name. The name keys both this mapping + and the flat tool list the model is given, so it has to be unique. """ dict_mapping: dict[str, UiPathToolNode] = {} for tool in tools: + if (clash := dict_mapping.get(tool.name)) is not None: + raise AgentStartupError( + code=AgentStartupErrorCode.INVALID_TOOL_CONFIG, + title="Two tools share a name", + detail=( + f"The agent exposes two tools named '{tool.name}': " + f"{_describe_tool_origin(clash.tool)} and " + f"{_describe_tool_origin(tool)}. Rename one of them so the " + f"model can tell the two tools apart." + ), + category=UiPathErrorCategory.USER, + ) if isinstance(tool, ToolWrapperMixin): dict_mapping[tool.name] = UiPathToolNode( tool, diff --git a/testcases/simple-http-mcp/src/simple-http-mcp/agents_api.py b/testcases/simple-http-mcp/src/simple-http-mcp/agents_api.py index a87b93952..2f530ea6a 100644 --- a/testcases/simple-http-mcp/src/simple-http-mcp/agents_api.py +++ b/testcases/simple-http-mcp/src/simple-http-mcp/agents_api.py @@ -40,6 +40,7 @@ from urllib.parse import quote import httpx +from langchain_core.tools import BaseTool from pydantic import BaseModel, Field from uipath._utils._ssl_context import get_httpx_client_kwargs from uipath.agent.models.agent import ( @@ -349,6 +350,11 @@ def _negotiated_version(client: McpClient) -> str | None: return None if version is None else str(version) +def _mcp_name(tool: BaseTool) -> str: + """The tool's name on the MCP server, which its LLM-facing name qualifies.""" + return (tool.metadata or {}).get("display_name", tool.name) + + async def _run_leg( label: str, resource: AgentMcpResourceConfig, @@ -393,12 +399,12 @@ async def _run_leg( ) clients = [client] tools = await create_mcp_tools(resource, client) - add_tool = next(tool for tool in tools if tool.name == "add") + add_tool = next(tool for tool in tools if _mcp_name(tool) == "add") blocks = await add_tool.ainvoke({"a": a, "b": b}) summary = LegSummary( label=label, protocol_mode=protocol_mode, - tools=sorted(tool.name for tool in tools), + tools=sorted(_mcp_name(tool) for tool in tools), tool_result=_first_text(blocks), session_id=await clients[0].get_session_id(), negotiated_version=_negotiated_version(clients[0]), diff --git a/tests/agent/guardrails/test_tool_guardrails_subgraph.py b/tests/agent/guardrails/test_tool_guardrails_subgraph.py index e0d905fc8..690f1c49c 100644 --- a/tests/agent/guardrails/test_tool_guardrails_subgraph.py +++ b/tests/agent/guardrails/test_tool_guardrails_subgraph.py @@ -58,6 +58,53 @@ def test_no_applicable_guardrails_returns_original_node(self): ) assert result_wrong_scope == inner[1] + def test_selector_matches_the_tools_display_name(self): + """An MCP selector holds the tool's own name, not the qualified node key.""" + inner = ("mcp-sales_mcp-tool-add", lambda s: s) + guardrail = MagicMock() + guardrail.selector = types.SimpleNamespace( + scopes=[GuardrailScope.TOOL], match_names=["add"] + ) + + result = mod.create_tool_guardrails_subgraph( + tool_node=inner, + guardrails=[(guardrail, MagicMock())], + display_name="add", + ) + + assert result != inner[1] + + def test_selector_matches_a_sanitized_display_name(self): + """Selectors arrive sanitized, so a display name with spaces still matches.""" + inner = ("mcp-sales_mcp-tool-search_tool", lambda s: s) + guardrail = MagicMock() + guardrail.selector = types.SimpleNamespace( + scopes=[GuardrailScope.TOOL], match_names=["Search_Tool"] + ) + + result = mod.create_tool_guardrails_subgraph( + tool_node=inner, + guardrails=[(guardrail, MagicMock())], + display_name="Search Tool!", + ) + + assert result != inner[1] + + def test_selector_ignores_a_display_name_it_does_not_name(self): + inner = ("mcp-sales_mcp-tool-add", lambda s: s) + guardrail = MagicMock() + guardrail.selector = types.SimpleNamespace( + scopes=[GuardrailScope.TOOL], match_names=["subtract"] + ) + + result = mod.create_tool_guardrails_subgraph( + tool_node=inner, + guardrails=[(guardrail, MagicMock())], + display_name="add", + ) + + assert result == inner[1] + def test_two_guardrails_build_chains_pre_and_post(self, monkeypatch): """Two guardrails should create reverse-ordered pre/post chains with failure edges.""" monkeypatch.setattr(mod, "StateGraph", FakeStateGraph) diff --git a/tests/agent/tools/test_mcp/claude.md b/tests/agent/tools/test_mcp/claude.md index acc41001f..3169c16ec 100644 --- a/tests/agent/tools/test_mcp/claude.md +++ b/tests/agent/tools/test_mcp/claude.md @@ -561,7 +561,7 @@ assert len(clients) == 2 # One per MCP server with patch(..., return_value=mock_uipath_class): tools, clients = await create_mcp_tools_and_clients(agent) assert len(tools) == 1 # Only enabled server's tool -assert tools[0].name == "enabled_tool" +assert tools[0].name == "mcp-enabled_server-tool-enabled_tool" ``` #### test_returns_empty_for_empty_resources diff --git a/tests/agent/tools/test_mcp/test_mcp_client_real_http.py b/tests/agent/tools/test_mcp/test_mcp_client_real_http.py index 66fafbfbd..dd89dfa95 100644 --- a/tests/agent/tools/test_mcp/test_mcp_client_real_http.py +++ b/tests/agent/tools/test_mcp/test_mcp_client_real_http.py @@ -544,7 +544,11 @@ async def test_tool_built_by_the_factory_invokes_over_real_http() -> None: async with serve(gateway) as url: async with connected_client(url) as client: tools = await create_mcp_tools(make_resource_config(), client) - add_tool = next(tool for tool in tools if tool.name == "add") + add_tool = next( + tool + for tool in tools + if (tool.metadata or {}).get("display_name") == "add" + ) result = await add_tool.ainvoke({"a": 2, "b": 3}) blocks = result if isinstance(result, list) else [result] diff --git a/tests/agent/tools/test_mcp/test_mcp_tool.py b/tests/agent/tools/test_mcp/test_mcp_tool.py index 4eed06128..b0b7c3959 100644 --- a/tests/agent/tools/test_mcp/test_mcp_tool.py +++ b/tests/agent/tools/test_mcp/test_mcp_tool.py @@ -24,10 +24,12 @@ ) from uipath_langchain.agent.tools.mcp import McpClient from uipath_langchain.agent.tools.mcp.mcp_tool import ( + _MAX_TOOL_NAME_LENGTH, _schema_change_message, build_mcp_tool, create_mcp_tools, create_mcp_tools_and_clients, + mcp_tool_identity, open_mcp_tools, ) from uipath_langchain.agent.tools.structured_tool_with_argument_properties import ( @@ -35,6 +37,12 @@ ) +def display_name(tool: BaseTool) -> str: + """The MCP tool's own name, which the qualified tool name shortens.""" + assert tool.metadata is not None + return tool.metadata["display_name"] + + class TestMcpToolMetadata: """Test that MCP tool has correct metadata for observability.""" @@ -165,8 +173,8 @@ async def test_creates_multiple_tools( tools = await create_mcp_tools(mcp_resource_multiple_tools, mock_mcp_client) assert len(tools) == 2 - assert tools[0].name == "tool_one" - assert tools[1].name == "tool_two" + assert tools[0].name == "mcp-multi_tool_server-tool-tool_one" + assert tools[1].name == "mcp-multi_tool_server-tool-tool_two" @pytest.mark.asyncio async def test_tool_has_correct_description( @@ -292,7 +300,7 @@ async def test_creates_tools_from_multiple_mcp_servers(self, mcp_resources): # Should have 3 tools total (2 from server 1, 1 from server 2) assert len(tools) == 3 - tool_names = [t.name for t in tools] + tool_names = [display_name(t) for t in tools] assert "tool_a" in tool_names assert "tool_b" in tool_names assert "tool_c" in tool_names @@ -312,7 +320,7 @@ async def test_skips_disabled_mcp_resources(self, mcp_resources_with_disabled): # Only enabled server's tool should be created assert len(tools) == 1 - assert tools[0].name == "enabled_tool" + assert tools[0].name == "mcp-enabled_server-tool-enabled_tool" # Only one client for enabled server assert len(clients) == 1 @@ -655,7 +663,7 @@ async def test_curated_dynamic_filters_to_available_tools( """Dynamic with allow_all=False only includes tools listed in available_tools.""" tools = await create_mcp_tools(mcp_resource_curated_dynamic, mock_mcp_client) - tool_names = [t.name for t in tools] + tool_names = [display_name(t) for t in tools] assert "tool_a" in tool_names assert "tool_b" in tool_names assert "tool_c" not in tool_names @@ -668,7 +676,7 @@ async def test_curated_dynamic_uses_server_schemas_and_descriptions( """Dynamic with allow_all=False uses input/output schemas and descriptions from the server.""" tools = await create_mcp_tools(mcp_resource_curated_dynamic, mock_mcp_client) - tool_a = next(t for t in tools if t.name == "tool_a") + tool_a = next(t for t in tools if display_name(t) == "tool_a") assert tool_a.description == "Tool A from server" assert isinstance(tool_a.args_schema, dict) assert "x" in tool_a.args_schema["properties"] @@ -703,7 +711,7 @@ async def test_curated_dynamic_warns_about_missing_allowed_tool( with caplog.at_level(logging.WARNING): tools = await create_mcp_tools(resource, mock_mcp_client) - tool_names = [t.name for t in tools] + tool_names = [display_name(t) for t in tools] assert "tool_a" in tool_names assert "phantom" not in tool_names assert any( @@ -728,7 +736,7 @@ async def test_dynamic_returns_all_server_tools( """Test that Dynamic mode returns every tool from the server.""" tools = await create_mcp_tools(mcp_resource_dynamic, mock_mcp_client) - tool_names = [t.name for t in tools] + tool_names = [display_name(t) for t in tools] assert "tool_a" in tool_names assert "tool_b" in tool_names assert "tool_c" in tool_names @@ -751,7 +759,7 @@ async def test_dynamic_uses_server_schemas_and_descriptions( """Test that Dynamic mode uses schemas and descriptions from the server.""" tools = await create_mcp_tools(mcp_resource_dynamic, mock_mcp_client) - tool_a = next(t for t in tools if t.name == "tool_a") + tool_a = next(t for t in tools if display_name(t) == "tool_a") assert tool_a.description == "Tool A from server" assert isinstance(tool_a.args_schema, dict) assert "x" in tool_a.args_schema["properties"] @@ -780,7 +788,7 @@ async def test_cached_default_does_not_call_list_tools(self): client.list_tools.assert_not_awaited() assert len(tools) == 1 - assert tools[0].name == "local_tool" + assert tools[0].name == "mcp-default_server-tool-local_tool" @pytest.mark.asyncio async def test_cached_uses_resource_schemas(self): @@ -1267,13 +1275,100 @@ async def test_all_mode_carries_over_argument_properties_for_matching_tools( divide_tool = next( cast(StructuredToolWithArgumentProperties, t) for t in tools - if t.name == "divide" + if display_name(t) == "divide" ) new_tool = next( cast(StructuredToolWithArgumentProperties, t) for t in tools - if t.name == "new_tool" + if display_name(t) == "new_tool" ) assert "$['a']" in divide_tool.argument_properties assert not new_tool.argument_properties + + +class TestMcpToolIdentity: + """The LLM-facing name carries the resource, so two servers cannot collide.""" + + def test_name_is_qualified_by_the_resource(self): + assert ( + mcp_tool_identity("Case Management", "get_case") + == "mcp-case_management-tool-get_case" + ) + + def test_same_tool_name_on_two_servers_stays_distinct(self): + """Both servers expose ``aggregate_table_data``.""" + first = mcp_tool_identity("Sales MCP", "aggregate_table_data") + second = mcp_tool_identity("Finance MCP", "aggregate_table_data") + + assert first != second + + def test_long_pair_is_shortened_within_the_provider_cap(self): + name = mcp_tool_identity("A" * 60, "B" * 60) + + assert len(name) == _MAX_TOOL_NAME_LENGTH + + def test_shortening_spends_the_cap_on_the_tool_name(self): + """The tool name is what the model reads, so the resource gives way.""" + name = mcp_tool_identity( + "Reporting Connector For Finance", "aggregate_table_data_monthly" + ) + + assert "aggregate_table_data_monthly" in name + assert name.startswith("mcp-reporting_connect") + + def test_shortened_tools_stay_distinguishable_by_name(self): + """Two tools on one server whose names differ only near the end.""" + resource = "Reporting Connector For Finance" + monthly = mcp_tool_identity(resource, "aggregate_table_data_monthly") + yearly = mcp_tool_identity(resource, "aggregate_table_data_yearly") + + assert len(monthly) == len(yearly) == _MAX_TOOL_NAME_LENGTH + assert "aggregate_table_data_monthly" in monthly + assert "aggregate_table_data_yearly" in yearly + + def test_pairs_sharing_a_truncated_prefix_stay_distinct(self): + prefix = "identical_resource_name_long_enough_to_be_truncated" + first = mcp_tool_identity(prefix, "tool_name_that_also_runs_past_the_cap_one") + second = mcp_tool_identity(prefix, "tool_name_that_also_runs_past_the_cap_two") + + assert first != second + + def test_identity_is_stable_across_processes(self): + """A salted digest would rebind a resumed run to a different name.""" + assert ( + mcp_tool_identity("A" * 60, "B" * 60) + == "mcp-aaaaaaaa-tool-bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb-af65f768" + ) + + def test_resource_names_normalizing_alike_share_an_identity(self): + """``create_tool_node`` reports the pair; the name alone cannot separate them.""" + assert mcp_tool_identity("Case Management", "add") == mcp_tool_identity( + "Case_Management", "add" + ) + + @pytest.mark.asyncio + async def test_two_resources_sharing_a_tool_name_produce_distinct_tools(self): + """End to end over create_mcp_tools, which is what builds the flat list.""" + + def resource(name: str) -> AgentMcpResourceConfig: + return AgentMcpResourceConfig( + name=name, + description="", + folder_path="/Shared", + slug=name.lower(), + available_tools=[ + AgentMcpTool( + name="aggregate_table_data", + description="Aggregate", + input_schema={"type": "object", "properties": {}}, + ) + ], + ) + + client = MagicMock(spec=McpClient) + sales = await create_mcp_tools(resource("Sales"), client) + finance = await create_mcp_tools(resource("Finance"), client) + + assert sales[0].name != finance[0].name + assert display_name(sales[0]) == display_name(finance[0]) diff --git a/tests/agent/tools/test_tool_node.py b/tests/agent/tools/test_tool_node.py index bc46a5e13..e54180acb 100644 --- a/tests/agent/tools/test_tool_node.py +++ b/tests/agent/tools/test_tool_node.py @@ -14,6 +14,8 @@ from uipath_langchain.agent.exceptions import ( AgentRuntimeError, AgentRuntimeErrorCode, + AgentStartupError, + AgentStartupErrorCode, ) from uipath_langchain.agent.react.types import AgentGraphState from uipath_langchain.agent.tools.tool_node import ( @@ -432,6 +434,28 @@ def test_create_tool_node_empty_tools(self): assert result == {} + def test_duplicate_tool_names_are_refused(self): + """Two tools under one name are refused.""" + tools = [MockTool(name="shared"), MockTool(name="shared")] + + with pytest.raises(AgentStartupError) as exc_info: + create_tool_node(tools) + + assert exc_info.value.error_info.code == AgentStartupError.full_code( + AgentStartupErrorCode.INVALID_TOOL_CONFIG + ) + assert "shared" in exc_info.value.error_info.detail + + def test_duplicate_tool_names_report_the_resources_they_came_from(self): + first = MockTool(name="shared", metadata={"resource_name": "Sales MCP"}) + second = MockTool(name="shared", metadata={"resource_name": "Finance MCP"}) + + with pytest.raises(AgentStartupError) as exc_info: + create_tool_node([first, second]) + + assert "Sales MCP" in exc_info.value.error_info.detail + assert "Finance MCP" in exc_info.value.error_info.detail + async def test_wrap_tools_with_error_handling_captures_error(self): """Test that wrap_tools_with_error_handling captures tool errors as error ToolMessages.""" failing_tool = MockFailingTool() diff --git a/uv.lock b/uv.lock index a1a2cb0fa..b41ef45ca 100644 --- a/uv.lock +++ b/uv.lock @@ -4828,7 +4828,7 @@ wheels = [ [[package]] name = "uipath-langchain" -version = "0.18.13" +version = "0.18.14" source = { editable = "." } dependencies = [ { name = "a2a-sdk" },