diff --git a/openhands-sdk/openhands/sdk/llm/message.py b/openhands-sdk/openhands/sdk/llm/message.py index 8b5ba0d8a2..6600ceb921 100644 --- a/openhands-sdk/openhands/sdk/llm/message.py +++ b/openhands-sdk/openhands/sdk/llm/message.py @@ -1,4 +1,5 @@ import json +import re from abc import abstractmethod from collections.abc import Sequence from typing import Any, ClassVar, Literal @@ -20,6 +21,17 @@ logger = get_logger(__name__) +# OpenAI Responses API requires function_call ids/call_ids to match +# ^[A-Za-z0-9_-]+$. Parallel MCP tool calls can produce ids containing ':' +# (e.g. "github_get_file_contents:1"), so we normalize any disallowed +# character to '_' at the canonical-id boundary. +_RESPONSES_ID_DISALLOWED_RE = re.compile(r"[^A-Za-z0-9_-]") + + +def normalize_tool_call_id(tool_call_id: str) -> str: + """Replace characters disallowed by the Responses API id regex with '_'.""" + return _RESPONSES_ID_DISALLOWED_RE.sub("_", tool_call_id) + class MessageToolCall(BaseModel): """Transport-agnostic tool call representation. @@ -55,7 +67,7 @@ def from_chat_tool_call( raise ValueError(f"tool_call.function.name is None for {tool_call=}") return cls( - id=tool_call.id, + id=normalize_tool_call_id(tool_call.id), name=tool_call.function.name, arguments=tool_call.function.arguments, origin="completion", @@ -63,15 +75,26 @@ def from_chat_tool_call( @classmethod def from_responses_function_call( - cls, item: ResponseFunctionToolCall | OutputFunctionToolCall + cls, item: ResponseFunctionToolCall | OutputFunctionToolCall | Any ) -> "MessageToolCall": - """Create a MessageToolCall from a typed OpenAI Responses function_call item. + """Create a MessageToolCall from an OpenAI Responses function_call item. + + Accepts typed items (``ResponseFunctionToolCall`` / + ``OutputFunctionToolCall``) as well as generic objects + (``BaseLiteLLMOpenAIResponseObject`` from streaming) or plain dicts. Note: OpenAI Responses function_call.arguments is already a JSON string. """ - call_id = item.call_id or item.id or "" - name = item.name or "" - arguments_str = item.arguments or "" + + def _get(obj: Any, key: str, default: Any = None) -> Any: + if isinstance(obj, dict): + return obj.get(key, default) + return getattr(obj, key, default) + + raw_item_id = _get(item, "id") + call_id = _get(item, "call_id") or raw_item_id or "" + name = _get(item, "name") or "" + arguments_str = _get(item, "arguments") or "" if not call_id: raise ValueError(f"Responses function_call missing call_id/id: {item!r}") @@ -79,8 +102,8 @@ def from_responses_function_call( raise ValueError(f"Responses function_call missing name: {item!r}") return cls( - id=str(call_id), - responses_item_id=str(item.id) if item.id else None, + id=normalize_tool_call_id(str(call_id)), + responses_item_id=str(raw_item_id) if raw_item_id else None, name=str(name), arguments=arguments_str, origin="responses", @@ -572,23 +595,10 @@ def _get(obj: Any, key: str, default: Any = None) -> Any: part_text = _get(part, "text") if part_type == "output_text" and part_text: assistant_text_parts.append(part_text) - elif ( - isinstance(item, (OutputFunctionToolCall, ResponseFunctionToolCall)) - and item_type == "function_call" - ): - tc = MessageToolCall.from_responses_function_call(item) - tool_calls.append(tc) elif item_type == "function_call": - # Handle generic objects (e.g., BaseLiteLLMOpenAIResponseObject - # from streaming) or dicts with function_call type - raw_item_id = _get(item, "id") - tc = MessageToolCall( - id=_get(item, "call_id") or raw_item_id or "", - responses_item_id=str(raw_item_id) if raw_item_id else None, - name=_get(item, "name", ""), - arguments=_get(item, "arguments", ""), - origin="responses", - ) + # Typed items, generic objects (e.g. BaseLiteLLMOpenAIResponseObject + # from streaming), or dicts with function_call type + tc = MessageToolCall.from_responses_function_call(item) tool_calls.append(tc) elif item_type == "reasoning": if isinstance(item, ResponseReasoningItem): diff --git a/tests/sdk/agent/test_tool_call_id_normalization.py b/tests/sdk/agent/test_tool_call_id_normalization.py new file mode 100644 index 0000000000..13963a12df --- /dev/null +++ b/tests/sdk/agent/test_tool_call_id_normalization.py @@ -0,0 +1,170 @@ +import json +import re +from collections.abc import Sequence +from typing import TYPE_CHECKING, Self +from unittest.mock import patch + +from litellm import ChatCompletionMessageToolCall +from litellm.types.utils import ( + Choices, + Function, + Message as LiteLLMMessage, + ModelResponse, +) +from pydantic import Field, SecretStr + +from openhands.sdk.agent import Agent +from openhands.sdk.conversation import Conversation +from openhands.sdk.event import ActionEvent, ObservationEvent +from openhands.sdk.llm import LLM, Message, TextContent +from openhands.sdk.tool import Action, Observation, Tool, ToolExecutor, register_tool +from openhands.sdk.tool.tool import DeclaredResources, ToolDefinition + + +if TYPE_CHECKING: + from openhands.sdk.conversation.base import BaseConversation + from openhands.sdk.conversation.state import ConversationState + + +RESPONSES_ID_RE = re.compile(r"^[A-Za-z0-9_-]+$") + + +class GithubGetFileContentsAction(Action): + path: str = Field(default="") + + +class GithubGetFileContentsObservation(Observation): + path: str = Field(default="") + + +class GithubGetFileContentsExecutor( + ToolExecutor[GithubGetFileContentsAction, GithubGetFileContentsObservation] +): + def __call__( + self, + action: GithubGetFileContentsAction, + conversation: "BaseConversation | None" = None, + ) -> GithubGetFileContentsObservation: + return GithubGetFileContentsObservation.from_text( + text=f"contents for {action.path}", + path=action.path, + ) + + +class GithubGetFileContentsTool( + ToolDefinition[GithubGetFileContentsAction, GithubGetFileContentsObservation] +): + name = "github_get_file_contents" + + def declared_resources(self, action: Action) -> DeclaredResources: + return DeclaredResources(keys=(), declared=True) + + @classmethod + def create(cls, conv_state: "ConversationState | None" = None) -> Sequence[Self]: + return [ + cls( + description="Read repository file contents", + action_type=GithubGetFileContentsAction, + observation_type=GithubGetFileContentsObservation, + executor=GithubGetFileContentsExecutor(), + ) + ] + + +register_tool("GithubGetFileContentsTool", GithubGetFileContentsTool) + + +def test_llm_colon_delimited_tool_call_ids_are_normalized_before_events(): + llm = LLM( + usage_id="test-llm", + model="gpt-4o", + api_key=SecretStr("test-key"), + base_url="http://test", + ) + agent = Agent( + llm=llm, + tools=[Tool(name="GithubGetFileContentsTool")], + include_default_tools=[], + tool_concurrency_limit=4, + ) + conversation = Conversation(agent=agent, visualizer=None, max_iteration_per_run=3) + responses = [ + ModelResponse( + id="mock-response-tools", + choices=[ + Choices( + index=0, + message=LiteLLMMessage( + role="assistant", + content="", + tool_calls=[ + ChatCompletionMessageToolCall( + id="github_get_file_contents:1", + type="function", + function=Function( + name="github_get_file_contents", + arguments=json.dumps({"path": "pyproject.toml"}), + ), + ), + ChatCompletionMessageToolCall( + id="github_get_file_contents:2", + type="function", + function=Function( + name="github_get_file_contents", + arguments=json.dumps({"path": "README.md"}), + ), + ), + ], + ), + finish_reason="tool_calls", + ) + ], + created=0, + model="gpt-4o", + object="chat.completion", + ), + ModelResponse( + id="mock-response-done", + choices=[ + Choices( + index=0, + message=LiteLLMMessage(role="assistant", content="done"), + finish_reason="stop", + ) + ], + created=0, + model="gpt-4o", + object="chat.completion", + ), + ] + + with patch( + "openhands.sdk.llm.llm.litellm_completion", + side_effect=responses, + ): + conversation.send_message( + Message(role="user", content=[TextContent(text="go")]) + ) + conversation.run() + + events = list(conversation.state.events) + action_events = [event for event in events if isinstance(event, ActionEvent)] + observation_events = [ + event for event in events if isinstance(event, ObservationEvent) + ] + + assert [event.tool_call_id for event in action_events] == [ + "github_get_file_contents_1", + "github_get_file_contents_2", + ] + assert [event.tool_call.id for event in action_events] == [ + "github_get_file_contents_1", + "github_get_file_contents_2", + ] + assert [event.tool_call_id for event in observation_events] == [ + "github_get_file_contents_1", + "github_get_file_contents_2", + ] + + for event in [*action_events, *observation_events]: + assert RESPONSES_ID_RE.fullmatch(event.tool_call_id)