Handle collisions between tools and subagents names better (#44)

This change:

- Introduces a reserved "__" namespace for internally generated names
- Update subagent prefix to "__agent-"
- If a tool name begins with "__", prepend "__tool-" to ensure unprefixed
  tools never conflict with reserved/internal identifiers.

Author: mateusz834

splunklib/ai/core/backend.py

@@ -23,10 +23,6 @@ class InvalidModelError(Exception):
     """Raised when an invalid model is specified for a backend."""
 
 
-class InvalidToolNameError(Exception):
-    """Raised when a tool name contains invalid prefix."""
-
-
 class InvalidMessageTypeError(Exception):
     """Raised when a message type is not supported by the backend."""
 

splunklib/ai/engines/langchain.py

@@ -47,7 +47,6 @@
     Backend,
     InvalidMessageTypeError,
     InvalidModelError,
-    InvalidToolNameError,
 )
 from splunklib.ai.messages import (
     AgentCall,
@@ -70,7 +69,21 @@
 )
 from splunklib.ai.tools import Tool, ToolException
 
-AGENT_PREFIX = "agent-"
+# RESERVED_LC_TOOL_PREFIX represents a prefix that is reserved for internal use
+# and no user-visible tool or subagent name can contain it (as a prefix).
+RESERVED_LC_TOOL_PREFIX = "__"
+
+# AGENT_PREFIX is a prefix prepended to a name of an agent,
+# during the conversion of a subagent to a tool.
+# All subagents as tools have this prefix.
+AGENT_PREFIX = f"{RESERVED_LC_TOOL_PREFIX}agent-"
+
+# CONFLICTING_TOOL_PREFIX is a prefix that is prepended to a tool name
+# in case the tool name already starts with RESERVED_LC_TOOL_PREFIX.
+# This prevents the user-provided tools to start with AGENT_PREFIX and also
+# serves as a backward compatibility mechanism for us i.e. we are free to use
+# any tool name that starts with RESERVED_LC_TOOL_PREFIX for other uses.
+CONFLICTING_TOOL_PREFIX = f"{RESERVED_LC_TOOL_PREFIX}tool-"
 
 AGENT_AS_TOOLS_PROMPT = f"""
 You are provided with Agents.
@@ -202,7 +215,7 @@ async def _tool_call(
         return result.content, result.structured_content
 
     return StructuredTool(
-        name=tool.name,
+        name=_normalize_tool_name(tool.name),
         description=tool.description,
         args_schema=tool.input_schema,
         coroutine=_tool_call,
@@ -217,17 +230,23 @@ def langchain_backend_factory() -> LangChainBackend:
 
 
 def _normalize_agent_name(name: str) -> str:
-    # TODO: should we check for collisions here?
-    # TODO: we shouldn't change the name here - only add a prefix.
-    # We should validate the name when the Agent is created
-    name = "-".join(name.strip().split())
     return f"{AGENT_PREFIX}{name}"
 
 
 def _denormalize_agent_name(name: str) -> str:
     return name.removeprefix(AGENT_PREFIX)
 
 
+def _normalize_tool_name(name: str) -> str:
+    if name.startswith(RESERVED_LC_TOOL_PREFIX):
+        return f"{CONFLICTING_TOOL_PREFIX}{name}"
+    return name
+
+
+def _denormalize_tool_name(name: str) -> str:
+    return name.removeprefix(CONFLICTING_TOOL_PREFIX)
+
+
 def _agent_as_tool(agent: BaseAgent[OutputT]):
     if not agent.name:
         raise AssertionError("Agent must have a name to be used by other Agents")
@@ -274,21 +293,18 @@ def _map_tool_call_from_langchain(tool_call: LC_ToolCall) -> ToolCall | AgentCal
         )
 
     return ToolCall(
-        name=tool_call["name"],
+        name=_denormalize_tool_name(tool_call["name"]),
         args=tool_call["args"],
         id=tool_call["id"],
     )
 
 
 def _map_tool_call_to_langchain(call: ToolCall | AgentCall) -> LC_ToolCall:
-    if AGENT_PREFIX in call.name:
-        raise InvalidToolNameError(
-            f"ToolCall name cannot contain agent prefix: {call.name}"
-        )
-
-    name = call.name
-    if isinstance(call, AgentCall):
-        name = _normalize_agent_name(call.name)
+    match call:
+        case AgentCall():
+            name = _normalize_agent_name(call.name)
+        case ToolCall():
+            name = _normalize_tool_name(call.name)
 
     return LC_ToolCall(
         name=name,
@@ -320,7 +336,7 @@ def _map_message_from_langchain(message: LC_BaseMessage) -> BaseMessage:
                 "langchain responded with a tool call that does not have a name"
             )
             return ToolMessage(
-                name=message.name,
+                name=_denormalize_tool_name(message.name),
                 content=str(message.content),
                 call_id=message.tool_call_id,
                 status=message.status,
@@ -351,9 +367,9 @@ def _map_message_to_langchain(message: BaseMessage) -> LC_BaseMessage:
             )
         case ToolMessage():
             return LC_ToolMessage(
+                name=_normalize_tool_name(message.name),
                 content=message.content,
                 tool_call_id=message.call_id,
-                name=message.name,
                 status=message.status,
             )
         case SystemMessage():

tests/unit/ai/engine/test_langchain_backend.py

@@ -28,7 +28,6 @@
 from splunklib.ai.core.backend import (
     InvalidMessageTypeError,
     InvalidModelError,
-    InvalidToolNameError,
 )
 from splunklib.ai.engines import langchain as lc
 from splunklib.ai.messages import (
@@ -159,38 +158,95 @@ def test_map_message_to_langchain_ai_with_agent_call(self) -> None:
             )
         ]
 
-    def test_map_message_to_langchain_tool_call_with_agent_prefix_raises(
+    def test_map_message_to_langchain_human(self) -> None:
+        message = HumanMessage(content="hello")
+        mapped = lc._map_message_to_langchain(message)
+
+        assert isinstance(mapped, LC_HumanMessage)
+        assert mapped.content == "hello"
+
+    def test_map_message_to_langchain_tool_call_with_reserved_prefix(
         self,
     ) -> None:
-        message = AIMessage(
-            content="hi",
-            calls=[ToolCall(name=f"{lc.AGENT_PREFIX}bad-tool", args={}, id="tc-3")],
+        message = lc._map_message_to_langchain(
+            AIMessage(
+                content="hi",
+                calls=[ToolCall(name=f"{lc.AGENT_PREFIX}bad-tool", args={}, id="tc-1")],
+            )
+        )
+        assert isinstance(message, LC_AIMessage)
+        assert message.tool_calls == [
+            LC_ToolCall(name="__tool-__agent-bad-tool", args={}, id="tc-1")
+        ]
+
+        message = lc._map_message_to_langchain(
+            AIMessage(
+                content="hi",
+                calls=[ToolCall(name="__bad-tool", args={}, id="tc-2")],
+            )
         )
+        assert isinstance(message, LC_AIMessage)
+        assert message.tool_calls == [
+            LC_ToolCall(name="__tool-__bad-tool", args={}, id="tc-2")
+        ]
 
-        with pytest.raises(InvalidToolNameError):
-            lc._map_message_to_langchain(message)
+        message = lc._map_message_to_langchain(
+            ToolMessage(content="hi", name="__bad-tool")
+        )
+        assert isinstance(message, LC_ToolMessage)
+        assert message.name == "__tool-__bad-tool"
 
-    def test_map_message_to_langchain_agent_call_with_agent_prefix_raises(
+    def test_map_message_from_langchain_tool_call_with_reserved_prefix(
         self,
     ) -> None:
-        message = AIMessage(
-            content="hi",
-            calls=[
-                AgentCall(
-                    name=f"{lc.AGENT_PREFIX}bad-agent", args={"q": "test"}, id="tc-4"
-                )
-            ],
+        message = lc._map_message_from_langchain(
+            LC_AIMessage(
+                content="hi",
+                tool_calls=[
+                    LC_ToolCall(
+                        name="__tool-__bad-tool",
+                        args={},
+                        id="tc-1",
+                    )
+                ],
+            )
         )
+        assert isinstance(message, AIMessage)
+        assert len(message.calls) > 0
+        assert message.calls[0].name == "__bad-tool"
+
+        message = lc._map_message_from_langchain(
+            message=LC_ToolMessage(
+                name="__tool-__bad-tool",
+                content="result",
+                tool_call_id="call-1",
+                status="success",
+            )
+        )
+        assert isinstance(message, ToolMessage)
+        assert message.name == "__bad-tool"
 
-        with pytest.raises(InvalidToolNameError):
-            lc._map_message_to_langchain(message)
-
-    def test_map_message_to_langchain_human(self) -> None:
-        message = HumanMessage(content="hello")
-        mapped = lc._map_message_to_langchain(message)
+    def test_map_message_to_langchain_agent_call_with_agent_prefix_raises(
+        self,
+    ) -> None:
+        message = lc._map_message_to_langchain(
+            AIMessage(
+                content="hi",
+                calls=[
+                    AgentCall(
+                        name=f"{lc.AGENT_PREFIX}bad-agent",
+                        args={},
+                        id="tc-1",
+                    )
+                ],
+            )
+        )
 
-        assert isinstance(mapped, LC_HumanMessage)
-        assert mapped.content == "hello"
+        # Fine, but in practice a unnecessary prefix.
+        assert isinstance(message, LC_AIMessage)
+        assert message.tool_calls == [
+            LC_ToolCall(name="__agent-__agent-bad-agent", args={}, id="tc-1")
+        ]
 
     def test_map_message_to_langchain_system(self) -> None:
         message = SystemMessage(content="be helpful")
@@ -219,7 +275,7 @@ def test_map_message_to_langchain_subagent(self) -> None:
 
         assert isinstance(mapped, LC_ToolMessage)
         assert mapped.content == "ping"
-        assert mapped.name == f"{lc.AGENT_PREFIX}My-Agent"
+        assert mapped.name == f"{lc.AGENT_PREFIX}My Agent"
         assert mapped.tool_call_id == "call-2"
         assert mapped.status == "error"