Fix issue #8369: Handle invalid arguments in model tool calls (#8370)

Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: Engel Nyst <enyst@users.noreply.github.com>

openhands/agenthub/codeact_agent/function_calling.py

@@ -163,11 +163,29 @@ def response_to_actions(
                     if 'view_range' in other_kwargs:
                         # Remove view_range from other_kwargs since it is not needed for FileEditAction
                         other_kwargs.pop('view_range')
+
+                    # Filter out unexpected arguments
+                    valid_kwargs = {}
+                    # Get valid parameters from the str_replace_editor tool definition
+                    str_replace_editor_tool = create_str_replace_editor_tool()
+                    valid_params = set(
+                        str_replace_editor_tool['function']['parameters'][
+                            'properties'
+                        ].keys()
+                    )
+                    for key, value in other_kwargs.items():
+                        if key in valid_params:
+                            valid_kwargs[key] = value
+                        else:
+                            raise FunctionCallValidationError(
+                                f'Unexpected argument {key} in tool call {tool_call.function.name}. Allowed arguments are: {valid_params}'
+                            )
+
                     action = FileEditAction(
                         path=path,
                         command=command,
                         impl_source=FileEditSource.OH_ACI,
-                        **other_kwargs,
+                        **valid_kwargs,
                     )
             # ================================================
             # AgentThinkAction

openhands/runtime/action_execution_server.py

@@ -105,7 +105,7 @@ def _execute_file_editor(
     view_range: list[int] | None = None,
     old_str: str | None = None,
     new_str: str | None = None,
-    insert_line: int | None = None,
+    insert_line: int | str | None = None,
     enable_linting: bool = False,
 ) -> tuple[str, tuple[str | None, str | None]]:
     """Execute file editor command and handle exceptions.
@@ -118,13 +118,24 @@ def _execute_file_editor(
         view_range: Optional view range tuple (start, end)
         old_str: Optional string to replace
         new_str: Optional replacement string
-        insert_line: Optional line number for insertion
+        insert_line: Optional line number for insertion (can be int or str)
         enable_linting: Whether to enable linting
 
     Returns:
         tuple: A tuple containing the output string and a tuple of old and new file content
     """
     result: ToolResult | None = None
+
+    # Convert insert_line from string to int if needed
+    if insert_line is not None and isinstance(insert_line, str):
+        try:
+            insert_line = int(insert_line)
+        except ValueError:
+            return (
+                f"ERROR:\nInvalid insert_line value: '{insert_line}'. Expected an integer.",
+                (None, None),
+            )
+
     try:
         result = editor(
             command=command,
@@ -138,6 +149,9 @@ def _execute_file_editor(
         )
     except ToolError as e:
         result = ToolResult(error=e.message)
+    except TypeError as e:
+        # Handle unexpected arguments or type errors
+        return f'ERROR:\n{str(e)}', (None, None)
 
     if result.error:
         return f'ERROR:\n{result.error}', (None, None)

openhands/runtime/utils/bash.py

@@ -25,7 +25,13 @@ def split_bash_commands(commands: str) -> list[str]:
         return ['']
     try:
         parsed = bashlex.parse(commands)
-    except (bashlex.errors.ParsingError, NotImplementedError, TypeError):
+    except (
+        bashlex.errors.ParsingError,
+        NotImplementedError,
+        TypeError,
+        AttributeError,
+    ):
+        # Added AttributeError to catch 'str' object has no attribute 'kind' error (issue #8369)
         logger.debug(
             f'Failed to parse bash commands\n'
             f'[input]: {commands}\n'

tests/runtime/test_aci_edit.py

@@ -1,11 +1,13 @@
 """Editor-related tests for the DockerRuntime."""
 
 import os
+from unittest.mock import MagicMock
 
 from conftest import _close_test_runtime, _load_runtime
 
 from openhands.core.logger import openhands_logger as logger
 from openhands.events.action import FileEditAction, FileWriteAction
+from openhands.runtime.action_execution_server import _execute_file_editor
 
 
 def test_view_file(temp_dir, runtime_cls, run_as_openhands):
@@ -690,3 +692,32 @@ def test_view_large_file_with_truncation(temp_dir, runtime_cls, run_as_openhands
         )
     finally:
         _close_test_runtime(runtime)
+
+
+def test_insert_line_string_conversion():
+    """Test that insert_line is properly converted from string to int.
+
+    This test reproduces issue #8369 Example 2 where a string value for insert_line
+    causes a TypeError in the editor.
+    """
+    # Mock the OHEditor
+    mock_editor = MagicMock()
+    mock_editor.return_value = MagicMock(
+        error=None, output='Success', old_content=None, new_content=None
+    )
+
+    # Test with string insert_line
+    result, _ = _execute_file_editor(
+        editor=mock_editor,
+        command='insert',
+        path='/test/path.py',
+        insert_line='185',  # String instead of int
+        new_str='test content',
+    )
+
+    # Verify the editor was called with the correct parameters (insert_line converted to int)
+    mock_editor.assert_called_once()
+    args, kwargs = mock_editor.call_args
+    assert isinstance(kwargs['insert_line'], int)
+    assert kwargs['insert_line'] == 185
+    assert result == 'Success'

tests/unit/test_bash_parsing.py

@@ -180,6 +180,21 @@ def test_unclosed_backtick():
         raise e
 
 
+def test_over_escaped_command():
+    # This test reproduces issue #8369 Example 1
+    # The issue occurs when parsing a command with over-escaped quotes
+    over_escaped_command = r'# 0. Setup directory\\nrm -rf /workspace/repro_sphinx_bug && mkdir -p /workspace/repro_sphinx_bug && cd /workspace/repro_sphinx_bug\\n\\n# 1. Run sphinx-quickstart\\nsphinx-quickstart --no-sep --project myproject --author me -v 0.1.0 --release 0.1.0 --language en . -q\\n\\n# 2. Create index.rst\\necho -e \'Welcome\\\\\\\\n=======\\\\\\\\n\\\\\\\\n.. toctree::\\\\n   :maxdepth: 2\\\\\\\\n\\\\\\\\n   mypackage_file\\\\\\\\n\' > index.rst'
+
+    # Should not raise any exception
+    try:
+        result = split_bash_commands(over_escaped_command)
+        # If parsing fails, it should return the original command
+        assert result == [over_escaped_command]
+    except Exception as e:
+        # This is the error we're trying to fix
+        pytest.fail(f'split_bash_commands raised {type(e).__name__} unexpectedly: {e}')
+
+
 @pytest.fixture
 def sample_commands():
     return [

tests/unit/test_function_calling.py

@@ -218,3 +218,29 @@ def test_invalid_json_arguments():
     with pytest.raises(FunctionCallValidationError) as exc_info:
         response_to_actions(response)
     assert 'Failed to parse tool call arguments' in str(exc_info.value)
+
+
+def test_unexpected_argument_handling():
+    """Test that unexpected arguments in function calls are properly handled.
+
+    This test reproduces issue #8369 Example 4 where an unexpected argument
+    (old_str_prefix) causes a TypeError.
+    """
+    response = create_mock_response(
+        'str_replace_editor',
+        {
+            'command': 'str_replace',
+            'path': '/test/file.py',
+            'old_str': 'def test():\n    pass',
+            'new_str': 'def test():\n    return True',
+            'old_str_prefix': 'some prefix',  # Unexpected argument
+        },
+    )
+
+    # Test that the function raises a FunctionCallValidationError
+    with pytest.raises(FunctionCallValidationError) as exc_info:
+        response_to_actions(response)
+
+    # Verify the error message mentions the unexpected argument
+    assert 'old_str_prefix' in str(exc_info.value)
+    assert 'Unexpected argument' in str(exc_info.value)