diff --git a/python/packages/orchestrations/agent_framework_orchestrations/_handoff.py b/python/packages/orchestrations/agent_framework_orchestrations/_handoff.py index aadcb8ad6df..8b65da4e202 100644 --- a/python/packages/orchestrations/agent_framework_orchestrations/_handoff.py +++ b/python/packages/orchestrations/agent_framework_orchestrations/_handoff.py @@ -32,6 +32,7 @@ import inspect import json import logging +import re import sys from collections.abc import Awaitable, Callable, Mapping, Sequence from copy import deepcopy @@ -122,9 +123,17 @@ def __hash__(self) -> int: return hash(self.target_id) +_INVALID_TOOL_NAME_CHARACTERS = re.compile(r"[^a-zA-Z0-9_-]") +_MAX_TOOL_NAME_LENGTH = 64 + + def get_handoff_tool_name(target_id: str) -> str: - """Get the standardized handoff tool name for a given target agent ID.""" - return f"handoff_to_{target_id}" + """Get the standardized handoff tool name for a given target agent ID. + + Characters that model providers reject in tool names, such as spaces, are replaced with underscores, + and the name is cut to the 64 characters that providers allow. + """ + return f"handoff_to_{_INVALID_TOOL_NAME_CHARACTERS.sub('_', target_id)}"[:_MAX_TOOL_NAME_LENGTH] HANDOFF_FUNCTION_RESULT_KEY = "handoff_to" @@ -372,6 +381,7 @@ def _apply_auto_tools(self, agent: Agent, targets: Sequence[HandoffConfiguration f"Handoff tool name '{handoff_tool.name}' conflicts with existing tool." "Please rename the existing tool or modify the target agent ID to avoid conflicts." ) + existing_names.add(handoff_tool.name) new_tools.append(handoff_tool) if new_tools: diff --git a/python/packages/orchestrations/tests/test_handoff.py b/python/packages/orchestrations/tests/test_handoff.py index 781e64b73b9..5410363524c 100644 --- a/python/packages/orchestrations/tests/test_handoff.py +++ b/python/packages/orchestrations/tests/test_handoff.py @@ -137,7 +137,7 @@ def _build_reply_contents( if handoff_to and call_id: contents.append( Content.from_function_call( - call_id=call_id, name=f"handoff_to_{handoff_to}", arguments={"handoff_to": handoff_to} + call_id=call_id, name=get_handoff_tool_name(handoff_to), arguments={"handoff_to": handoff_to} ) ) text = f"{agent_name} reply" @@ -1516,6 +1516,90 @@ async def before_run(self, **kwargs: Any) -> None: ) +@pytest.mark.parametrize( + ("target_id", "expected"), + [ + ("refund_agent", "handoff_to_refund_agent"), + ("order-status", "handoff_to_order-status"), + ("Billing Agent", "handoff_to_Billing_Agent"), + ("support.v2", "handoff_to_support_v2"), + ], +) +def test_handoff_tool_name_uses_only_valid_tool_name_characters(target_id: str, expected: str) -> None: + assert get_handoff_tool_name(target_id) == expected + + +def test_handoff_tool_name_stays_within_provider_length_limit() -> None: + long_name = "customer_billing_and_refund_escalation_specialist_for_enterprise_accounts" + + tool_name = get_handoff_tool_name(long_name) + + assert len(tool_name) == 64 + assert tool_name == f"handoff_to_{long_name}"[:64] + + +def test_handoff_builder_rejects_long_targets_with_the_same_tool_name() -> None: + prefix = "customer_billing_and_refund_escalation_specialist_for_enterprise" + triage = MockHandoffAgent(name="triage") + first = MockHandoffAgent(name=f"{prefix}_north") + second = MockHandoffAgent(name=f"{prefix}_south") + + with pytest.raises(ValueError, match="conflicts with existing tool"): + HandoffBuilder(participants=_as_handoff_agents(triage, first, second)).with_start_agent( + _as_handoff_agent(triage) + ).build() + + +class ToolNameRecordingClient(MockChatClient): + """Mock chat client that records the tool names offered to the model.""" + + def __init__(self, *, name: str, handoff_to: str | None = None) -> None: + super().__init__(name=name, handoff_to=handoff_to) + self.offered_tool_names: list[str] = [] + + def _inner_get_response( + self, + *, + messages: Sequence[Message], + stream: bool, + options: Mapping[str, Any], + **kwargs: Any, + ) -> Awaitable[ChatResponse] | ResponseStream[ChatResponseUpdate, ChatResponse]: + self.offered_tool_names.extend(tool.name for tool in options.get("tools") or []) + return super()._inner_get_response(messages=messages, stream=stream, options=options, **kwargs) + + +async def test_handoff_between_agents_with_spaces_in_names() -> None: + """Agent names with spaces still produce tool names that model providers accept.""" + triage_client = ToolNameRecordingClient(name="Triage Desk", handoff_to="Billing Agent") + billing_client = ToolNameRecordingClient(name="Billing Agent") + triage = Agent(client=triage_client, name="Triage Desk", require_per_service_call_history_persistence=True) + billing = Agent(client=billing_client, name="Billing Agent", require_per_service_call_history_persistence=True) + + workflow = ( + HandoffBuilder(participants=_as_handoff_agents(triage, billing)) + .with_start_agent(_as_handoff_agent(triage)) + .build() + ) + async for _ in workflow.run("I was charged twice", stream=True): + pass + + assert "handoff_to_Billing_Agent" in triage_client.offered_tool_names + assert all(re.fullmatch(r"[a-zA-Z0-9_-]+", name) for name in triage_client.offered_tool_names) + assert billing_client.received_messages + + +def test_handoff_builder_rejects_targets_with_the_same_tool_name() -> None: + triage = MockHandoffAgent(name="triage") + spaced = MockHandoffAgent(name="Billing Agent") + underscored = MockHandoffAgent(name="Billing_Agent") + + with pytest.raises(ValueError, match="handoff_to_Billing_Agent"): + HandoffBuilder(participants=_as_handoff_agents(triage, spaced, underscored)).with_start_agent( + _as_handoff_agent(triage) + ).build() + + def test_handoff_builder_accepts_all_instances_in_add_handoff(): """Test that add_handoff accepts all instances when using participants.""" triage = MockHandoffAgent(name="triage", handoff_to="specialist_a")