fix(sdk): Use concise error message for tool validation errors (#2748)

Author: VascoSch92

openhands-sdk/openhands/sdk/agent/agent.py

@@ -845,6 +845,7 @@ def _get_action_event(
 
         # Validate arguments
         security_risk: risk.SecurityRisk = risk.SecurityRisk.UNKNOWN
+        parsed_args: dict | None = None
         try:
             # Try parsing arguments as-is first.  Raw newlines / tabs are
             # legal JSON whitespace and many models emit them between tokens
@@ -853,13 +854,14 @@ def _get_action_event(
             # Fall back to sanitization only when the raw string is invalid
             # (handles models that emit raw control chars *inside* strings).
             try:
-                arguments = json.loads(tool_call.arguments)
+                parsed_args = json.loads(tool_call.arguments)
             except json.JSONDecodeError:
                 sanitized_args = sanitize_json_control_chars(tool_call.arguments)
-                arguments = json.loads(sanitized_args)
+                parsed_args = json.loads(sanitized_args)
 
             # Fix malformed arguments (e.g., JSON strings for list/dict fields)
-            arguments = fix_malformed_tool_arguments(arguments, tool.action_type)
+            assert isinstance(parsed_args, dict)
+            arguments = fix_malformed_tool_arguments(parsed_args, tool.action_type)
             security_risk = self._extract_security_risk(
                 arguments,
                 tool.name,
@@ -874,10 +876,14 @@ def _get_action_event(
 
             action: Action = tool.action_from_arguments(arguments)
         except (json.JSONDecodeError, ValidationError, ValueError) as e:
-            err = (
-                f"Error validating args {tool_call.arguments} for tool "
-                f"'{tool.name}': {e}"
+            # Build concise error message with parameter names only (not values)
+            keys = list(parsed_args.keys()) if isinstance(parsed_args, dict) else None
+            params = (
+                f"Parameters provided: {keys}"
+                if keys is not None
+                else "Arguments: unparseable JSON"
             )
+            err = f"Error validating tool '{tool.name}': {e}. {params}"
             # Persist assistant function_call so next turn has matching call_id
             tc_event = ActionEvent(
                 source="agent",

tests/sdk/agent/test_tool_validation_error_message.py

@@ -0,0 +1,199 @@
+"""Test that tool validation error messages are concise and don't include values."""
+
+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 SecretStr
+
+from openhands.sdk.agent import Agent
+from openhands.sdk.conversation import Conversation
+from openhands.sdk.event import AgentErrorEvent
+from openhands.sdk.llm import LLM, Message, TextContent
+from openhands.sdk.security.llm_analyzer import LLMSecurityAnalyzer
+from openhands.sdk.tool import Action, Observation, Tool, ToolExecutor, register_tool
+from openhands.sdk.tool.tool import ToolDefinition
+
+
+if TYPE_CHECKING:
+    from openhands.sdk.conversation.state import ConversationState
+
+
+class ValidationTestAction(Action):
+    """Action for validation testing."""
+
+    command: str = ""
+    path: str = ""
+    old_str: str = ""
+
+
+class ValidationTestObservation(Observation):
+    """Observation for validation testing."""
+
+    result: str = ""
+
+
+class ValidationTestExecutor(
+    ToolExecutor[ValidationTestAction, ValidationTestObservation]
+):
+    """Executor that just returns an observation."""
+
+    def __call__(
+        self, action: ValidationTestAction, conversation=None
+    ) -> ValidationTestObservation:
+        return ValidationTestObservation(result="ok")
+
+
+class ValidationTestTool(
+    ToolDefinition[ValidationTestAction, ValidationTestObservation]
+):
+    """Tool for testing validation error messages."""
+
+    name = "validation_test_tool"
+
+    @classmethod
+    def create(cls, conv_state: "ConversationState | None" = None) -> Sequence[Self]:
+        return [
+            cls(
+                description="A tool for testing validation errors",
+                action_type=ValidationTestAction,
+                observation_type=ValidationTestObservation,
+                executor=ValidationTestExecutor(),
+            )
+        ]
+
+
+register_tool("ValidationTestTool", ValidationTestTool)
+
+
+def test_validation_error_shows_keys_not_values():
+    """Error message should show parameter keys, not large argument values."""
+    llm = LLM(
+        usage_id="test-llm",
+        model="test-model",
+        api_key=SecretStr("test-key"),
+        base_url="http://test",
+    )
+    agent = Agent(llm=llm, tools=[Tool(name="ValidationTestTool")])
+
+    # Create tool call with large arguments but missing security_risk field
+    large_value = "x" * 1000
+    tool_args = f'{{"command": "view", "path": "/test", "old_str": "{large_value}"}}'
+
+    def mock_llm_response(messages, **kwargs):
+        return ModelResponse(
+            id="mock-1",
+            choices=[
+                Choices(
+                    index=0,
+                    message=LiteLLMMessage(
+                        role="assistant",
+                        content="I'll use the tool.",
+                        tool_calls=[
+                            ChatCompletionMessageToolCall(
+                                id="call_1",
+                                type="function",
+                                function=Function(
+                                    name="validation_test_tool", arguments=tool_args
+                                ),
+                            )
+                        ],
+                    ),
+                    finish_reason="tool_calls",
+                )
+            ],
+            created=0,
+            model="test-model",
+            object="chat.completion",
+        )
+
+    collected_events = []
+    conversation = Conversation(agent=agent, callbacks=[collected_events.append])
+    conversation.set_security_analyzer(LLMSecurityAnalyzer())
+
+    with patch(
+        "openhands.sdk.llm.llm.litellm_completion", side_effect=mock_llm_response
+    ):
+        conversation.send_message(
+            Message(role="user", content=[TextContent(text="Do something")])
+        )
+        agent.step(conversation, on_event=collected_events.append)
+
+    error_events = [e for e in collected_events if isinstance(e, AgentErrorEvent)]
+    assert len(error_events) == 1
+
+    error_msg = error_events[0].error
+    # Error should include tool name and parameter keys
+    assert "validation_test_tool" in error_msg
+    assert "Parameters provided:" in error_msg
+    assert "command" in error_msg
+    assert "path" in error_msg
+    assert "old_str" in error_msg
+    # Error should NOT include the large value (1000 x's)
+    assert large_value not in error_msg
+
+
+def test_unparseable_json_error_message():
+    """Error message should indicate unparseable JSON when parsing fails."""
+    llm = LLM(
+        usage_id="test-llm",
+        model="test-model",
+        api_key=SecretStr("test-key"),
+        base_url="http://test",
+    )
+    agent = Agent(llm=llm, tools=[Tool(name="ValidationTestTool")])
+
+    # Invalid JSON that cannot be parsed
+    invalid_json = "{invalid json syntax"
+
+    def mock_llm_response(messages, **kwargs):
+        return ModelResponse(
+            id="mock-1",
+            choices=[
+                Choices(
+                    index=0,
+                    message=LiteLLMMessage(
+                        role="assistant",
+                        content="I'll use the tool.",
+                        tool_calls=[
+                            ChatCompletionMessageToolCall(
+                                id="call_1",
+                                type="function",
+                                function=Function(
+                                    name="validation_test_tool", arguments=invalid_json
+                                ),
+                            )
+                        ],
+                    ),
+                    finish_reason="tool_calls",
+                )
+            ],
+            created=0,
+            model="test-model",
+            object="chat.completion",
+        )
+
+    collected_events = []
+    conversation = Conversation(agent=agent, callbacks=[collected_events.append])
+
+    with patch(
+        "openhands.sdk.llm.llm.litellm_completion", side_effect=mock_llm_response
+    ):
+        conversation.send_message(
+            Message(role="user", content=[TextContent(text="Do something")])
+        )
+        agent.step(conversation, on_event=collected_events.append)
+
+    error_events = [e for e in collected_events if isinstance(e, AgentErrorEvent)]
+    assert len(error_events) == 1
+
+    error_msg = error_events[0].error
+    assert "validation_test_tool" in error_msg
+    assert "unparseable JSON" in error_msg