Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 12 additions & 6 deletions openhands-sdk/openhands/sdk/agent/agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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",
Expand Down
199 changes: 199 additions & 0 deletions tests/sdk/agent/test_tool_validation_error_message.py
Original file line number Diff line number Diff line change
@@ -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
Loading