Skip to content

Commit 3fabe3f

Browse files
fix(sdk): normalize qwen tool-call variants
Co-authored-by: openhands <openhands@all-hands.dev>
1 parent 9c8e899 commit 3fabe3f

File tree

2 files changed

+173
-25
lines changed

2 files changed

+173
-25
lines changed

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

Lines changed: 51 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -175,10 +175,19 @@ def fix_malformed_tool_arguments(
175175
"command": "terminal",
176176
"execute": "terminal",
177177
"execute_bash": "terminal",
178+
"run": "terminal",
179+
"straight": "terminal",
178180
"str_replace": "file_editor",
179181
"str_replace_editor": "file_editor",
182+
"str_view": "file_editor",
183+
"write": "file_editor",
180184
}
181185

186+
# Some models emit malformed tool names with trailing XML-ish fragments such as
187+
# ``str_replace\n</parameter``. Keep the normalization narrow by extracting only
188+
# the leading identifier token when the raw tool name is otherwise unknown.
189+
_TOOL_NAME_TOKEN_RE = re.compile(r"[A-Za-z_][A-Za-z0-9_-]*")
190+
182191
# This fallback is intentionally tiny: it only accepts exact, bare command names
183192
# that are useful as read-only defaults when some models emit them as tool names.
184193
_SHELL_TOOL_FALLBACK_COMMANDS = frozenset({"find", "ls", "pwd"})
@@ -214,6 +223,16 @@ def parse_tool_call_arguments(raw_arguments: str) -> dict[str, Any]:
214223
return _normalize_arguments(result)
215224

216225

226+
def _sanitize_tool_name(tool_name: str) -> str:
227+
"""Return the leading identifier token from XML-ish tool-name artifacts."""
228+
stripped = tool_name.strip()
229+
if not any(marker in stripped for marker in ("\n", "<", ">")):
230+
return stripped
231+
232+
match = _TOOL_NAME_TOKEN_RE.match(stripped)
233+
return match.group(0) if match else stripped
234+
235+
217236
def _infer_file_editor_command(arguments: dict[str, Any]) -> str | None:
218237
if "command" in arguments:
219238
return None
@@ -303,26 +322,40 @@ def normalize_tool_call(
303322
# Only apply aliases for tool names that are not explicitly registered.
304323
# This prevents hijacking legitimate tools that share names with aliases.
305324
if tool_name not in available_tools:
306-
alias_target = TOOL_NAME_ALIASES.get(tool_name)
307-
if alias_target and alias_target in available_tools:
308-
normalized_tool_name = alias_target
309-
elif "terminal" in available_tools:
310-
terminal_command = _maybe_rewrite_as_terminal_command(
311-
tool_name,
312-
normalized_arguments,
313-
)
314-
if terminal_command is not None:
315-
normalized_tool_name = "terminal"
316-
# Preserve only terminal-relevant arguments (security_risk, summary)
317-
# along with the generated command
318-
normalized_arguments = {
319-
key: value
320-
for key, value in normalized_arguments.items()
321-
if key in {"security_risk", "summary"}
322-
}
323-
normalized_arguments["command"] = terminal_command
325+
sanitized_tool_name = _sanitize_tool_name(tool_name)
326+
if sanitized_tool_name in available_tools:
327+
normalized_tool_name = sanitized_tool_name
328+
else:
329+
alias_target = TOOL_NAME_ALIASES.get(sanitized_tool_name)
330+
if alias_target and alias_target in available_tools:
331+
normalized_tool_name = alias_target
332+
elif "terminal" in available_tools:
333+
terminal_command = _maybe_rewrite_as_terminal_command(
334+
sanitized_tool_name,
335+
normalized_arguments,
336+
)
337+
if terminal_command is not None:
338+
normalized_tool_name = "terminal"
339+
# Preserve only terminal-relevant arguments (security_risk,
340+
# summary) along with the generated command.
341+
normalized_arguments = {
342+
key: value
343+
for key, value in normalized_arguments.items()
344+
if key in {"security_risk", "summary"}
345+
}
346+
normalized_arguments["command"] = terminal_command
347+
348+
if normalized_tool_name == "think" and not normalized_arguments:
349+
normalized_arguments = {"thought": ""}
324350

325351
if normalized_tool_name == "file_editor":
352+
command = normalized_arguments.get("command")
353+
if isinstance(command, str):
354+
normalized_arguments = {
355+
**normalized_arguments,
356+
"command": _sanitize_tool_name(command),
357+
}
358+
326359
inferred_command = _infer_file_editor_command(normalized_arguments)
327360
if inferred_command is not None:
328361
normalized_arguments = {

tests/sdk/agent/test_tool_call_compatibility.py

Lines changed: 122 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
from pydantic import SecretStr
2020

2121
from openhands.sdk.agent import Agent
22+
from openhands.sdk.agent.utils import normalize_tool_call
2223
from openhands.sdk.conversation import Conversation, LocalConversation
2324
from openhands.sdk.event import ActionEvent, AgentErrorEvent, ObservationEvent
2425
from openhands.sdk.llm import LLM, Message, TextContent
@@ -103,6 +104,9 @@ def __call__(
103104
updated = path.read_text().replace(action.old_str, action.new_str or "", 1)
104105
path.write_text(updated)
105106
return _FileEditorObservation.from_text("replaced")
107+
if action.command == "create":
108+
path.write_text(action.file_text or "")
109+
return _FileEditorObservation.from_text("created")
106110
if action.command == "view":
107111
return _FileEditorObservation.from_text(path.read_text())
108112
raise ValueError(f"Unsupported file_editor command: {action.command}")
@@ -233,6 +237,77 @@ def test_str_replace_alias_infers_file_editor_command(tmp_path):
233237
assert test_file.read_text() == "value = 'new'\n"
234238

235239

240+
def test_malformed_str_replace_tool_name_is_sanitized(tmp_path):
241+
test_file = tmp_path / "sample.py"
242+
test_file.write_text("value = 'old'\n")
243+
244+
events = _run_tool_call(
245+
tmp_path,
246+
tool_name="str_replace\n</parameter",
247+
arguments={
248+
"path": str(test_file),
249+
"old_str": "'old'",
250+
"new_str": "'new'",
251+
},
252+
tool_names=(FILE_EDITOR_TOOL_SPEC,),
253+
)
254+
255+
action_event = next(e for e in events if isinstance(e, ActionEvent))
256+
errors = [e for e in events if isinstance(e, AgentErrorEvent)]
257+
258+
assert not errors
259+
assert action_event.tool_name == FILE_EDITOR_TOOL_NAME
260+
assert action_event.tool_call.name == FILE_EDITOR_TOOL_NAME
261+
assert action_event.action is not None
262+
assert getattr(action_event.action, "command") == "str_replace"
263+
assert test_file.read_text() == "value = 'new'\n"
264+
265+
266+
@pytest.mark.parametrize("tool_name", ["run", "straight"])
267+
def test_terminal_aliases_execute_terminal_tool(tmp_path, tool_name):
268+
events = _run_tool_call(
269+
tmp_path,
270+
tool_name=tool_name,
271+
arguments={"command": "printf hello"},
272+
tool_names=(TERMINAL_TOOL_SPEC,),
273+
)
274+
275+
action_event = next(e for e in events if isinstance(e, ActionEvent))
276+
observation_event = next(e for e in events if isinstance(e, ObservationEvent))
277+
errors = [e for e in events if isinstance(e, AgentErrorEvent)]
278+
279+
assert not errors
280+
assert action_event.tool_name == TERMINAL_TOOL_NAME
281+
assert action_event.tool_call.name == TERMINAL_TOOL_NAME
282+
assert action_event.action is not None
283+
assert getattr(action_event.action, "command") == "printf hello"
284+
assert "hello" in observation_event.observation.text
285+
286+
287+
def test_write_alias_infers_file_editor_create(tmp_path):
288+
created_file = tmp_path / "created.py"
289+
290+
events = _run_tool_call(
291+
tmp_path,
292+
tool_name="write",
293+
arguments={
294+
"path": str(created_file),
295+
"file_text": "print('hello')\n",
296+
},
297+
tool_names=(FILE_EDITOR_TOOL_SPEC,),
298+
)
299+
300+
action_event = next(e for e in events if isinstance(e, ActionEvent))
301+
errors = [e for e in events if isinstance(e, AgentErrorEvent)]
302+
303+
assert not errors
304+
assert action_event.tool_name == FILE_EDITOR_TOOL_NAME
305+
assert action_event.tool_call.name == FILE_EDITOR_TOOL_NAME
306+
assert action_event.action is not None
307+
assert getattr(action_event.action, "command") == "create"
308+
assert created_file.read_text() == "print('hello')\n"
309+
310+
236311
def test_shell_tool_name_falls_back_to_terminal(tmp_path):
237312
events = _run_tool_call(
238313
tmp_path,
@@ -447,10 +522,6 @@ def test_explicitly_registered_tool_not_hijacked_by_alias():
447522
rather than aliased to 'terminal'. This prevents legitimate tools from being
448523
silently overridden by the compatibility shim.
449524
"""
450-
from openhands.sdk.agent.utils import normalize_tool_call
451-
452-
# When 'bash' is explicitly registered alongside 'terminal',
453-
# normalize_tool_call should preserve 'bash', not alias to 'terminal'
454525
available_tools = {"bash", "terminal", "file_editor"}
455526

456527
# Test with 'bash' tool name - should NOT be aliased since it's registered
@@ -465,8 +536,52 @@ def test_explicitly_registered_tool_not_hijacked_by_alias():
465536
tool_name, args = normalize_tool_call("ls", {}, available_tools)
466537
assert tool_name == "terminal", "Unknown 'ls' should fallback to terminal"
467538

468-
# Test with 'str_replace' - should be aliased (alias target is registered)
539+
# Test with malformed XML-ish suffixes - should sanitize, then alias.
469540
tool_name, args = normalize_tool_call(
470-
"str_replace", {"old_str": "x", "new_str": "y"}, available_tools
541+
"str_replace\n</function",
542+
{"path": "/tmp/example.py", "old_str": "x", "new_str": "y"},
543+
available_tools,
544+
)
545+
assert tool_name == "file_editor"
546+
assert args["command"] == "str_replace"
547+
548+
549+
@pytest.mark.parametrize(
550+
("tool_name", "arguments", "expected_name", "expected_command"),
551+
[
552+
(
553+
"write",
554+
{"path": "/tmp/example.py", "file_text": "print('hi')\n"},
555+
"file_editor",
556+
"create",
557+
),
558+
("str_view", {"path": "/tmp/example.py"}, "file_editor", "view"),
559+
(
560+
"file_editor",
561+
{"command": "view\n</parameter", "path": "/tmp/example.py"},
562+
"file_editor",
563+
"view",
564+
),
565+
],
566+
)
567+
def test_file_editor_compatibility_normalization(
568+
tool_name,
569+
arguments,
570+
expected_name,
571+
expected_command,
572+
):
573+
normalized_name, normalized_args = normalize_tool_call(
574+
tool_name,
575+
arguments,
576+
{"file_editor", "terminal", "think"},
471577
)
472-
assert tool_name == "file_editor", "str_replace alias should map to file_editor"
578+
579+
assert normalized_name == expected_name
580+
assert normalized_args["command"] == expected_command
581+
582+
583+
def test_empty_think_arguments_are_normalized():
584+
tool_name, args = normalize_tool_call("think", {}, {"think"})
585+
586+
assert tool_name == "think"
587+
assert args == {"thought": ""}

0 commit comments

Comments
 (0)