refactor(runtime): Use openhands-aci file editor directly in runtime instead of execute it through ipython (#6671)

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

frontend/src/types/core/actions.ts

@@ -83,7 +83,9 @@ export interface FileReadAction extends OpenHandsActionEvent<"read"> {
   args: {
     path: string;
     thought: string;
-    translated_ipython_code: string | null;
+    security_risk: ActionSecurityRisk | null;
+    impl_source?: string;
+    view_range?: number[] | null;
   };
 }
 
@@ -100,7 +102,18 @@ export interface FileEditAction extends OpenHandsActionEvent<"edit"> {
   source: "agent";
   args: {
     path: string;
-    translated_ipython_code: string;
+    command?: string;
+    file_text?: string | null;
+    view_range?: number[] | null;
+    old_str?: string | null;
+    new_str?: string | null;
+    insert_line?: number | null;
+    content?: string;
+    start?: number;
+    end?: number;
+    thought: string;
+    security_risk: ActionSecurityRisk | null;
+    impl_source?: string;
   };
 }
 

openhands/agenthub/codeact_agent/function_calling.py

@@ -16,7 +16,6 @@ from openhands.core.exceptions import (
     FunctionCallNotExistsError,
     FunctionCallValidationError,
 )
-from openhands.core.logger import openhands_logger as logger
 from openhands.events.action import (
     Action,
     AgentDelegateAction,
@@ -541,26 +540,27 @@ def response_to_actions(response: ModelResponse) -> list[Action]:
                     raise FunctionCallValidationError(
                         f'Missing required argument "path" in tool call {tool_call.function.name}'
                     )
+                path = arguments['path']
+                command = arguments['command']
+                other_kwargs = {
+                    k: v for k, v in arguments.items() if k not in ['command', 'path']
+                }
 
-                # We implement this in agent_skills, which can be used via Jupyter
-                # convert tool_call.function.arguments to kwargs that can be passed to file_editor
-                code = f'print(file_editor(**{arguments}))'
-                logger.debug(
-                    f'TOOL CALL: str_replace_editor -> file_editor with code: {code}'
-                )
-
-                if arguments['command'] == 'view':
+                if command == 'view':
                     action = FileReadAction(
-                        path=arguments['path'],
-                        translated_ipython_code=code,
+                        path=path,
                         impl_source=FileReadSource.OH_ACI,
+                        view_range=other_kwargs.get('view_range', None),
                     )
                 else:
+                    if 'view_range' in other_kwargs:
+                        # Remove view_range from other_kwargs since it is not needed for FileEditAction
+                        other_kwargs.pop('view_range')
                     action = FileEditAction(
-                        path=arguments['path'],
-                        content='',  # dummy value -- we don't need it
-                        translated_ipython_code=code,
+                        path=path,
+                        command=command,
                         impl_source=FileEditSource.OH_ACI,
+                        **other_kwargs,
                     )
             elif tool_call.function.name == 'browser':
                 if 'code' not in arguments:

openhands/events/action/files.py

@@ -21,7 +21,7 @@ class FileReadAction(Action):
     runnable: ClassVar[bool] = True
     security_risk: ActionSecurityRisk | None = None
     impl_source: FileReadSource = FileReadSource.DEFAULT
-    translated_ipython_code: str = ''  # translated openhands-aci IPython code
+    view_range: list[int] | None = None  # ONLY used in OH_ACI mode
 
     @property
     def message(self) -> str:
@@ -60,29 +60,79 @@ class FileWriteAction(Action):
 
 @dataclass
 class FileEditAction(Action):
-    """Edits a file by provided a draft at a given path.
+    """Edits a file using various commands including view, create, str_replace, insert, and undo_edit.
 
-    Can be set to edit specific lines using start and end (1-index, inclusive) if the file is too long.
-    Default lines 1:-1 (whole file).
+    This class supports two main modes of operation:
+    1. LLM-based editing (impl_source = FileEditSource.LLM_BASED_EDIT)
+    2. ACI-based editing (impl_source = FileEditSource.OH_ACI)
 
-    If start is set to -1, the FileEditAction will simply append the content to the file.
+    Attributes:
+        path (str): The path to the file being edited. Works for both LLM-based and OH_ACI editing.
+        OH_ACI only arguments:
+            command (str): The editing command to be performed (view, create, str_replace, insert, undo_edit, write).
+            file_text (str): The content of the file to be created (used with 'create' command in OH_ACI mode).
+            old_str (str): The string to be replaced (used with 'str_replace' command in OH_ACI mode).
+            new_str (str): The string to replace old_str (used with 'str_replace' and 'insert' commands in OH_ACI mode).
+            insert_line (int): The line number after which to insert new_str (used with 'insert' command in OH_ACI mode).
+        LLM-based editing arguments:
+            content (str): The content to be written or edited in the file (used in LLM-based editing and 'write' command).
+            start (int): The starting line for editing (1-indexed, inclusive). Default is 1.
+            end (int): The ending line for editing (1-indexed, inclusive). Default is -1 (end of file).
+            thought (str): The reasoning behind the edit action.
+            action (str): The type of action being performed (always ActionType.EDIT).
+        runnable (bool): Indicates if the action can be executed (always True).
+        security_risk (ActionSecurityRisk | None): Indicates any security risks associated with the action.
+        impl_source (FileEditSource): The source of the implementation (LLM_BASED_EDIT or OH_ACI).
+
+    Usage:
+        - For LLM-based editing: Use path, content, start, and end attributes.
+        - For ACI-based editing: Use path, command, and the appropriate attributes for the specific command.
+
+    Note:
+        - If start is set to -1 in LLM-based editing, the content will be appended to the file.
+        - The 'write' command behaves similarly to LLM-based editing, using content, start, and end attributes.
     """
 
     path: str
-    content: str
+
+    # OH_ACI arguments
+    command: str = ''
+    file_text: str | None = None
+    old_str: str | None = None
+    new_str: str | None = None
+    insert_line: int | None = None
+
+    # LLM-based editing arguments
+    content: str = ''
     start: int = 1
     end: int = -1
+
+    # Shared arguments
     thought: str = ''
     action: str = ActionType.EDIT
     runnable: ClassVar[bool] = True
     security_risk: ActionSecurityRisk | None = None
-    impl_source: FileEditSource = FileEditSource.LLM_BASED_EDIT
-    translated_ipython_code: str = ''
+    impl_source: FileEditSource = FileEditSource.OH_ACI
 
     def __repr__(self) -> str:
         ret = '**FileEditAction**\n'
-        ret += f'Thought: {self.thought}\n'
-        ret += f'Range: [L{self.start}:L{self.end}]\n'
         ret += f'Path: [{self.path}]\n'
-        ret += f'Content:\n```\n{self.content}\n```\n'
+        ret += f'Thought: {self.thought}\n'
+
+        if self.impl_source == FileEditSource.LLM_BASED_EDIT:
+            ret += f'Range: [L{self.start}:L{self.end}]\n'
+            ret += f'Content:\n```\n{self.content}\n```\n'
+        else:  # OH_ACI mode
+            ret += f'Command: {self.command}\n'
+            if self.command == 'create':
+                ret += f'Created File with Text:\n```\n{self.file_text}\n```\n'
+            elif self.command == 'str_replace':
+                ret += f'Old String: ```\n{self.old_str}\n```\n'
+                ret += f'New String: ```\n{self.new_str}\n```\n'
+            elif self.command == 'insert':
+                ret += f'Insert Line: {self.insert_line}\n'
+                ret += f'New String: ```\n{self.new_str}\n```\n'
+            elif self.command == 'undo_edit':
+                ret += 'Undo Edit\n'
+            # We ignore "view" command because it will be mapped to a FileReadAction
         return ret

openhands/events/observation/files.py

@@ -50,15 +50,18 @@ class FileEditObservation(Observation):
     The observation includes both the old and new content of the file, and can
     generate a diff visualization showing the changes. The diff is computed lazily
     and cached to improve performance.
+
+    The .content property can either be:
+      - Git diff in LLM-based editing mode
+      - the rendered message sent to the LLM in OH_ACI mode (e.g., "The file /path/to/file.txt is created with the provided content.")
     """
 
-    path: str
-    prev_exist: bool
-    old_content: str
-    new_content: str
+    path: str = ''
+    prev_exist: bool = False
+    old_content: str | None = None
+    new_content: str | None = None
     observation: str = ObservationType.EDIT
     impl_source: FileEditSource = FileEditSource.LLM_BASED_EDIT
-    formatted_output_and_error: str = ''
     _diff_cache: str | None = None  # Cache for the diff visualization
 
     @property
@@ -75,6 +78,8 @@ class FileEditObservation(Observation):
         Returns:
             A list of edit groups, where each group contains before/after edits.
         """
+        if self.old_content is None or self.new_content is None:
+            return []
         old_lines = self.old_content.split('\n')
         new_lines = self.new_content.split('\n')
         # Borrowed from difflib.unified_diff to directly parse into structured format
@@ -173,7 +178,7 @@ class FileEditObservation(Observation):
     def __str__(self) -> str:
         """Get a string representation of the file edit observation."""
         if self.impl_source == FileEditSource.OH_ACI:
-            return self.formatted_output_and_error
+            return self.content
 
         if not self.prev_exist:
             assert (

openhands/events/serialization/action.py

@@ -1,3 +1,5 @@
+import re
+
 from openhands.core.exceptions import LLMMalformedActionError
 from openhands.events.action.action import Action
 from openhands.events.action.agent import (
@@ -38,6 +40,38 @@ actions = (
 ACTION_TYPE_TO_CLASS = {action_class.action: action_class for action_class in actions}  # type: ignore[attr-defined]
 
 
+def handle_action_deprecated_args(args: dict) -> dict:
+    # keep_prompt has been deprecated in https://github.com/All-Hands-AI/OpenHands/pull/4881
+    if 'keep_prompt' in args:
+        args.pop('keep_prompt')
+
+    # Handle translated_ipython_code deprecation
+    if 'translated_ipython_code' in args:
+        code = args.pop('translated_ipython_code')
+
+        # Check if it's a file_editor call
+        file_editor_pattern = r'print\(file_editor\(\*\*(.*?)\)\)'
+        if code is not None and (match := re.match(file_editor_pattern, code)):
+            try:
+                # Extract and evaluate the dictionary string
+                import ast
+
+                file_args = ast.literal_eval(match.group(1))
+
+                # Update args with the extracted file editor arguments
+                args.update(file_args)
+            except (ValueError, SyntaxError):
+                # If parsing fails, just remove the translated_ipython_code
+                pass
+
+        if args.get('command') == 'view':
+            args.pop(
+                'command'
+            )  # "view" will be translated to FileReadAction which doesn't have a command argument
+
+    return args
+
+
 def action_from_dict(action: dict) -> Action:
     if not isinstance(action, dict):
         raise LLMMalformedActionError('action must be a dictionary')
@@ -67,9 +101,8 @@ def action_from_dict(action: dict) -> Action:
     if 'images_urls' in args:
         args['image_urls'] = args.pop('images_urls')
 
-    # keep_prompt has been deprecated in https://github.com/All-Hands-AI/OpenHands/pull/4881
-    if 'keep_prompt' in args:
-        args.pop('keep_prompt')
+    # handle deprecated args
+    args = handle_action_deprecated_args(args)
 
     try:
         decoded_action = action_class(**args)

openhands/events/serialization/observation.py

@@ -64,6 +64,23 @@ def _update_cmd_output_metadata(
     return metadata
 
 
+def handle_observation_deprecated_extras(extras: dict) -> dict:
+    # These are deprecated in https://github.com/All-Hands-AI/OpenHands/pull/4881
+    if 'exit_code' in extras:
+        extras['metadata'] = _update_cmd_output_metadata(
+            extras.get('metadata', None), exit_code=extras.pop('exit_code')
+        )
+    if 'command_id' in extras:
+        extras['metadata'] = _update_cmd_output_metadata(
+            extras.get('metadata', None), pid=extras.pop('command_id')
+        )
+
+    # formatted_output_and_error has been deprecated in https://github.com/All-Hands-AI/OpenHands/pull/6671
+    if 'formatted_output_and_error' in extras:
+        extras.pop('formatted_output_and_error')
+    return extras
+
+
 def observation_from_dict(observation: dict) -> Observation:
     observation = observation.copy()
     if 'observation' not in observation:
@@ -78,15 +95,8 @@ def observation_from_dict(observation: dict) -> Observation:
     content = observation.pop('content', '')
     extras = copy.deepcopy(observation.pop('extras', {}))
 
-    # Handle legacy attributes for CmdOutputObservation
-    if 'exit_code' in extras:
-        extras['metadata'] = _update_cmd_output_metadata(
-            extras.get('metadata', None), exit_code=extras.pop('exit_code')
-        )
-    if 'command_id' in extras:
-        extras['metadata'] = _update_cmd_output_metadata(
-            extras.get('metadata', None), pid=extras.pop('command_id')
-        )
+    extras = handle_observation_deprecated_extras(extras)
+
     # convert metadata to CmdOutputMetadata if it is a dict
     if observation_class is CmdOutputObservation:
         if 'metadata' in extras and isinstance(extras['metadata'], dict):

openhands/runtime/action_execution_server.py

@@ -9,10 +9,8 @@ import argparse
 import asyncio
 import base64
 import io
-import json
 import mimetypes
 import os
-import re
 import shutil
 import tempfile
 import time
@@ -26,7 +24,9 @@ from fastapi import Depends, FastAPI, HTTPException, Request, UploadFile
 from fastapi.exceptions import RequestValidationError
 from fastapi.responses import JSONResponse, StreamingResponse
 from fastapi.security import APIKeyHeader
-from openhands_aci.utils.diff import get_diff
+from openhands_aci.editor.editor import OHEditor
+from openhands_aci.editor.exceptions import ToolError
+from openhands_aci.editor.results import ToolResult
 from pydantic import BaseModel
 from starlette.exceptions import HTTPException as StarletteHTTPException
 from uvicorn import run
@@ -37,6 +37,7 @@ from openhands.events.action import (
     BrowseInteractiveAction,
     BrowseURLAction,
     CmdRunAction,
+    FileEditAction,
     FileReadAction,
     FileWriteAction,
     IPythonRunCellAction,
@@ -78,6 +79,58 @@ def verify_api_key(api_key: str = Depends(api_key_header)):
     return api_key
 
 
+def _execute_file_editor(
+    editor: OHEditor,
+    command: str,
+    path: str,
+    file_text: str | None = None,
+    view_range: list[int] | None = None,
+    old_str: str | None = None,
+    new_str: str | None = None,
+    insert_line: int | None = None,
+    enable_linting: bool = False,
+) -> str:
+    """Execute file editor command and handle exceptions.
+
+    Args:
+        editor: The OHEditor instance
+        command: Editor command to execute
+        path: File path
+        file_text: Optional file text content
+        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
+        enable_linting: Whether to enable linting
+
+    Returns:
+        str: Result string from the editor operation
+    """
+    result: ToolResult | None = None
+    try:
+        result = editor(
+            command=command,
+            path=path,
+            file_text=file_text,
+            view_range=view_range,
+            old_str=old_str,
+            new_str=new_str,
+            insert_line=insert_line,
+            enable_linting=enable_linting,
+        )
+    except ToolError as e:
+        result = ToolResult(error=e.message)
+
+    if result.error:
+        return f'ERROR:\n{result.error}'
+
+    if not result.output:
+        logger.warning(f'No output from file_editor for {path}')
+        return ''
+
+    return result.output
+
+
 class ActionExecutor:
     """ActionExecutor is running inside docker sandbox.
     It is responsible for executing actions received from OpenHands backend and producing observations.
@@ -104,6 +157,7 @@ class ActionExecutor:
         self.bash_session: BashSession | None = None
         self.lock = asyncio.Lock()
         self.plugins: dict[str, Plugin] = {}
+        self.file_editor = OHEditor()
         self.browser = BrowserEnv(browsergym_eval_env)
         self.start_time = time.time()
         self.last_execution_time = self.start_time
@@ -237,65 +291,6 @@ class ActionExecutor:
 
             obs: IPythonRunCellObservation = await _jupyter_plugin.run(action)
             obs.content = obs.content.rstrip()
-            matches = re.findall(
-                r'<oh_aci_output_[0-9a-f]{32}>(.*?)</oh_aci_output_[0-9a-f]{32}>',
-                obs.content,
-                re.DOTALL,
-            )
-            if matches:
-                results: list[str] = []
-                if len(matches) == 1:
-                    # Use specific actions/observations types
-                    match = matches[0]
-                    try:
-                        result_dict = json.loads(match)
-                        if result_dict.get('path'):  # Successful output
-                            if (
-                                result_dict['new_content'] is not None
-                            ):  # File edit commands
-                                diff = get_diff(
-                                    old_contents=result_dict['old_content']
-                                    or '',  # old_content is None when file is created
-                                    new_contents=result_dict['new_content'],
-                                    filepath=result_dict['path'],
-                                )
-                                return FileEditObservation(
-                                    content=diff,
-                                    path=result_dict['path'],
-                                    old_content=result_dict['old_content'],
-                                    new_content=result_dict['new_content'],
-                                    prev_exist=result_dict['prev_exist'],
-                                    impl_source=FileEditSource.OH_ACI,
-                                    formatted_output_and_error=result_dict[
-                                        'formatted_output_and_error'
-                                    ],
-                                )
-                            else:  # File view commands
-                                return FileReadObservation(
-                                    content=result_dict['formatted_output_and_error'],
-                                    path=result_dict['path'],
-                                    impl_source=FileReadSource.OH_ACI,
-                                )
-                        else:  # Error output
-                            results.append(result_dict['formatted_output_and_error'])
-                    except json.JSONDecodeError:
-                        # Handle JSON decoding errors if necessary
-                        results.append(
-                            f"Invalid JSON in 'openhands-aci' output: {match}"
-                        )
-                else:
-                    for match in matches:
-                        try:
-                            result_dict = json.loads(match)
-                            results.append(result_dict['formatted_output_and_error'])
-                        except json.JSONDecodeError:
-                            # Handle JSON decoding errors if necessary
-                            results.append(
-                                f"Invalid JSON in 'openhands-aci' output: {match}"
-                            )
-
-                # Combine the results (e.g., join them) or handle them as required
-                obs.content = '\n'.join(str(result) for result in results)
 
             if action.include_extra:
                 obs.content += (
@@ -317,11 +312,17 @@ class ActionExecutor:
     async def read(self, action: FileReadAction) -> Observation:
         assert self.bash_session is not None
         if action.impl_source == FileReadSource.OH_ACI:
-            return await self.run_ipython(
-                IPythonRunCellAction(
-                    code=action.translated_ipython_code,
-                    include_extra=False,
-                )
+            result_str = _execute_file_editor(
+                self.file_editor,
+                command='view',
+                path=action.path,
+                view_range=action.view_range,
+            )
+
+            return FileReadObservation(
+                content=result_str,
+                path=action.path,
+                impl_source=FileReadSource.OH_ACI,
             )
 
         # NOTE: the client code is running inside the sandbox,
@@ -378,56 +379,75 @@ class ActionExecutor:
         filepath = self._resolve_path(action.path, working_dir)
 
         insert = action.content.split('\n')
+        if not os.path.exists(os.path.dirname(filepath)):
+            os.makedirs(os.path.dirname(filepath))
+
+        file_exists = os.path.exists(filepath)
+        if file_exists:
+            file_stat = os.stat(filepath)
+        else:
+            file_stat = None
+
+        mode = 'w' if not file_exists else 'r+'
         try:
-            if not os.path.exists(os.path.dirname(filepath)):
-                os.makedirs(os.path.dirname(filepath))
-
-            file_exists = os.path.exists(filepath)
-            if file_exists:
-                file_stat = os.stat(filepath)
-            else:
-                file_stat = None
-
-            mode = 'w' if not file_exists else 'r+'
-            try:
-                with open(filepath, mode, encoding='utf-8') as file:
-                    if mode != 'w':
-                        all_lines = file.readlines()
-                        new_file = insert_lines(
-                            insert, all_lines, action.start, action.end
-                        )
-                    else:
-                        new_file = [i + '\n' for i in insert]
-
-                    file.seek(0)
-                    file.writelines(new_file)
-                    file.truncate()
-
-                # Handle file permissions
-                if file_exists:
-                    assert file_stat is not None
-                    # restore the original file permissions if the file already exists
-                    os.chmod(filepath, file_stat.st_mode)
-                    os.chown(filepath, file_stat.st_uid, file_stat.st_gid)
+            with open(filepath, mode, encoding='utf-8') as file:
+                if mode != 'w':
+                    all_lines = file.readlines()
+                    new_file = insert_lines(insert, all_lines, action.start, action.end)
                 else:
-                    # set the new file permissions if the file is new
-                    os.chmod(filepath, 0o664)
-                    os.chown(filepath, self.user_id, self.user_id)
+                    new_file = [i + '\n' for i in insert]
 
-            except FileNotFoundError:
-                return ErrorObservation(f'File not found: {filepath}')
-            except IsADirectoryError:
-                return ErrorObservation(
-                    f'Path is a directory: {filepath}. You can only write to files'
-                )
-            except UnicodeDecodeError:
-                return ErrorObservation(
-                    f'File could not be decoded as utf-8: {filepath}'
-                )
-        except PermissionError:
-            return ErrorObservation(f'Malformed paths not permitted: {filepath}')
+                file.seek(0)
+                file.writelines(new_file)
+                file.truncate()
+
+        except FileNotFoundError:
+            return ErrorObservation(f'File not found: {filepath}')
+        except IsADirectoryError:
+            return ErrorObservation(
+                f'Path is a directory: {filepath}. You can only write to files'
+            )
+        except UnicodeDecodeError:
+            return ErrorObservation(f'File could not be decoded as utf-8: {filepath}')
+
+        # Attempt to handle file permissions
+        try:
+            if file_exists:
+                assert file_stat is not None
+                # restore the original file permissions if the file already exists
+                os.chmod(filepath, file_stat.st_mode)
+                os.chown(filepath, file_stat.st_uid, file_stat.st_gid)
+            else:
+                # set the new file permissions if the file is new
+                os.chmod(filepath, 0o664)
+                os.chown(filepath, self.user_id, self.user_id)
+        except PermissionError as e:
+            return ErrorObservation(
+                f'File {filepath} written, but failed to change ownership and permissions: {e}'
+            )
         return FileWriteObservation(content='', path=filepath)
 
+    async def edit(self, action: FileEditAction) -> Observation:
+        assert action.impl_source == FileEditSource.OH_ACI
+        result_str = _execute_file_editor(
+            self.file_editor,
+            command=action.command,
+            path=action.path,
+            file_text=action.file_text,
+            old_str=action.old_str,
+            new_str=action.new_str,
+            insert_line=action.insert_line,
+            enable_linting=False,
+        )
+
+        return FileEditObservation(
+            content=result_str,
+            path=action.path,
+            old_content=action.old_str,
+            new_content=action.new_str,
+            impl_source=FileEditSource.OH_ACI,
+        )
+
     async def browse(self, action: BrowseURLAction) -> Observation:
         return await browse(action, self.browser)
 

openhands/runtime/impl/action_execution/action_execution_client.py

@@ -24,6 +24,7 @@ from openhands.events.action import (
     IPythonRunCellAction,
 )
 from openhands.events.action.action import Action
+from openhands.events.action.files import FileEditSource
 from openhands.events.observation import (
     ErrorObservation,
     NullObservation,
@@ -217,8 +218,11 @@ class ActionExecutionClient(Runtime):
             return ''
 
     def send_action_for_execution(self, action: Action) -> Observation:
-        if isinstance(action, FileEditAction):
-            return self.edit(action)
+        if (
+            isinstance(action, FileEditAction)
+            and action.impl_source == FileEditSource.LLM_BASED_EDIT
+        ):
+            return self.llm_based_edit(action)
 
         # set timeout to default if not set
         if action.timeout is None:
@@ -281,6 +285,9 @@ class ActionExecutionClient(Runtime):
     def write(self, action: FileWriteAction) -> Observation:
         return self.send_action_for_execution(action)
 
+    def edit(self, action: FileEditAction) -> Observation:
+        return self.send_action_for_execution(action)
+
     def browse(self, action: BrowseURLAction) -> Observation:
         return self.send_action_for_execution(action)
 

openhands/runtime/utils/edit.py

@@ -13,7 +13,6 @@ from openhands.events.action import (
     FileWriteAction,
     IPythonRunCellAction,
 )
-from openhands.events.event import FileEditSource
 from openhands.events.observation import (
     ErrorObservation,
     FileEditObservation,
@@ -205,16 +204,7 @@ class FileEditRuntimeMixin(FileEditRuntimeInterface):
                 return ErrorObservation(error_message)
         return None
 
-    def edit(self, action: FileEditAction) -> Observation:
-        if action.impl_source == FileEditSource.OH_ACI:
-            # Translate to ipython command to file_editor
-            return self.run_ipython(
-                IPythonRunCellAction(
-                    code=action.translated_ipython_code,
-                    include_extra=False,
-                )
-            )
-
+    def llm_based_edit(self, action: FileEditAction) -> Observation:
         obs = self.read(FileReadAction(path=action.path))
         if (
             isinstance(obs, ErrorObservation)

openhands/runtime/utils/files.py

@@ -140,6 +140,6 @@ async def write_file(
             )
         except UnicodeDecodeError:
             return ErrorObservation(f'File could not be decoded as utf-8: {path}')
-    except PermissionError:
-        return ErrorObservation(f'Malformed paths not permitted: {path}')
+    except PermissionError as e:
+        return ErrorObservation(f'Permission error on {path}: {e}')
     return FileWriteObservation(content='', path=path)

tests/runtime/test_aci_edit.py

@@ -0,0 +1,692 @@
+"""Editor-related tests for the DockerRuntime."""
+
+import os
+
+from conftest import _close_test_runtime, _load_runtime
+
+from openhands.core.logger import openhands_logger as logger
+from openhands.events.action import FileEditAction, FileWriteAction
+
+
+def test_view_file(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        # Create test file
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='This is a test file.\nThis file is for testing purposes.',
+            path=test_file,
+        )
+        obs = runtime.run_action(action)
+
+        # Test view command
+        action = FileEditAction(
+            command='view',
+            path=test_file,
+        )
+        obs = runtime.run_action(action)
+
+        assert f"Here's the result of running `cat -n` on {test_file}:" in obs.content
+        assert '1\tThis is a test file.' in obs.content
+        assert '2\tThis file is for testing purposes.' in obs.content
+
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_view_directory(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        # Create test file
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='This is a test file.\nThis file is for testing purposes.',
+            path=test_file,
+        )
+        obs = runtime.run_action(action)
+
+        # Test view command
+        action = FileEditAction(
+            command='view',
+            path=config.workspace_mount_path_in_sandbox,
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert (
+            obs.content
+            == f"""Here's the files and directories up to 2 levels deep in {config.workspace_mount_path_in_sandbox}, excluding hidden items:
+{config.workspace_mount_path_in_sandbox}/
+{config.workspace_mount_path_in_sandbox}/test.txt"""
+        )
+
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_create_file(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        new_file = os.path.join(config.workspace_mount_path_in_sandbox, 'new_file.txt')
+        action = FileEditAction(
+            command='create',
+            path=new_file,
+            file_text='New file content',
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert 'File created successfully' in obs.content
+
+        # Verify file content
+        action = FileEditAction(
+            command='view',
+            path=new_file,
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert 'New file content' in obs.content
+
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_create_file_with_empty_content(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        new_file = os.path.join(config.workspace_mount_path_in_sandbox, 'new_file.txt')
+        action = FileEditAction(
+            command='create',
+            path=new_file,
+            file_text='',
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert 'File created successfully' in obs.content
+
+        # Verify file content
+        action = FileEditAction(
+            command='view',
+            path=new_file,
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert '1\t' in obs.content
+
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_create_with_none_file_text(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        new_file = os.path.join(
+            config.workspace_mount_path_in_sandbox, 'none_content.txt'
+        )
+        action = FileEditAction(
+            command='create',
+            path=new_file,
+            file_text=None,
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert (
+            obs.content
+            == 'ERROR:\nParameter `file_text` is required for command: create.'
+        )
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_str_replace(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        # Create test file
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='This is a test file.\nThis file is for testing purposes.',
+            path=test_file,
+        )
+        runtime.run_action(action)
+
+        # Test str_replace command
+        action = FileEditAction(
+            command='str_replace',
+            path=test_file,
+            old_str='test file',
+            new_str='sample file',
+        )
+        obs = runtime.run_action(action)
+        assert f'The file {test_file} has been edited' in obs.content
+
+        # Verify file content
+        action = FileEditAction(
+            command='view',
+            path=test_file,
+        )
+        obs = runtime.run_action(action)
+        assert 'This is a sample file.' in obs.content
+
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_str_replace_multi_line(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='This is a test file.\nThis file is for testing purposes.',
+            path=test_file,
+        )
+        runtime.run_action(action)
+
+        # Test str_replace command
+        action = FileEditAction(
+            command='str_replace',
+            path=test_file,
+            old_str='This is a test file.\nThis file is for testing purposes.',
+            new_str='This is a sample file.\nThis file is for testing purposes.',
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert f'The file {test_file} has been edited.' in obs.content
+        assert 'This is a sample file.' in obs.content
+        assert 'This file is for testing purposes.' in obs.content
+
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_str_replace_multi_line_with_tabs(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='def test():\n\tprint("Hello, World!")',
+            path=test_file,
+        )
+        runtime.run_action(action)
+
+        # Test str_replace command
+        action = FileEditAction(
+            command='str_replace',
+            path=test_file,
+            old_str='def test():\n\tprint("Hello, World!")',
+            new_str='def test():\n\tprint("Hello, Universe!")',
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert (
+            obs.content
+            == f"""The file {test_file} has been edited. Here's the result of running `cat -n` on a snippet of {test_file}:
+     1\tdef test():
+     2\t{'\t'.expandtabs()}print("Hello, Universe!")
+     3\t
+Review the changes and make sure they are as expected. Edit the file again if necessary."""
+        )
+
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_str_replace_error_multiple_occurrences(
+    temp_dir, runtime_cls, run_as_openhands
+):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='This is a test file.\nThis file is for testing purposes.',
+            path=test_file,
+        )
+        runtime.run_action(action)
+
+        action = FileEditAction(
+            command='str_replace', path=test_file, old_str='test', new_str='sample'
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert 'Multiple occurrences of old_str `test`' in obs.content
+        assert '[1, 2]' in obs.content  # Should show both line numbers
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_str_replace_error_multiple_multiline_occurrences(
+    temp_dir, runtime_cls, run_as_openhands
+):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        # Create a file with two identical multi-line blocks
+        multi_block = """def example():
+        print("Hello")
+        return True"""
+        content = f"{multi_block}\n\nprint('separator')\n\n{multi_block}"
+        action = FileWriteAction(
+            content=content,
+            path=test_file,
+        )
+        runtime.run_action(action)
+
+        # Test str_replace command
+        action = FileEditAction(
+            command='str_replace',
+            path=test_file,
+            old_str=multi_block,
+            new_str='def new():\n    print("World")',
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert 'Multiple occurrences of old_str' in obs.content
+        assert '[1, 7]' in obs.content  # Should show correct starting line numbers
+
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_str_replace_nonexistent_string(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='Line 1\nLine 2',
+            path=test_file,
+        )
+        runtime.run_action(action)
+        action = FileEditAction(
+            command='str_replace',
+            path=test_file,
+            old_str='Non-existent Line',
+            new_str='New Line',
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert 'No replacement was performed' in obs.content
+        assert (
+            f'old_str `Non-existent Line` did not appear verbatim in {test_file}'
+            in obs.content
+        )
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_str_replace_with_empty_new_str(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='Line 1\nLine to remove\nLine 3',
+            path=test_file,
+        )
+        runtime.run_action(action)
+        action = FileEditAction(
+            command='str_replace',
+            path=test_file,
+            old_str='Line to remove\n',
+            new_str='',
+        )
+        obs = runtime.run_action(action)
+        assert 'Line to remove' not in obs.content
+        assert 'Line 1' in obs.content
+        assert 'Line 3' in obs.content
+
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_str_replace_with_empty_old_str(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='Line 1\nLine 2\nLine 3',
+            path=test_file,
+        )
+        runtime.run_action(action)
+        action = FileEditAction(
+            command='str_replace',
+            path=test_file,
+            old_str='',
+            new_str='New string',
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert (
+            'No replacement was performed. Multiple occurrences of old_str `` in lines [1, 2, 3, 4]. Please ensure it is unique.'
+            in obs.content
+        )
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_str_replace_with_none_old_str(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='Line 1\nLine 2\nLine 3',
+            path=test_file,
+        )
+        runtime.run_action(action)
+
+        action = FileEditAction(
+            command='str_replace',
+            path=test_file,
+            old_str=None,
+            new_str='new content',
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert 'old_str' in obs.content
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_insert(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        # Create test file
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='Line 1\nLine 2',
+            path=test_file,
+        )
+        runtime.run_action(action)
+
+        # Test insert command
+        action = FileEditAction(
+            command='insert',
+            path=test_file,
+            insert_line=1,
+            new_str='Inserted line',
+        )
+        obs = runtime.run_action(action)
+        assert f'The file {test_file} has been edited' in obs.content
+
+        # Verify file content
+        action = FileEditAction(
+            command='view',
+            path=test_file,
+        )
+        obs = runtime.run_action(action)
+        assert 'Line 1' in obs.content
+        assert 'Inserted line' in obs.content
+        assert 'Line 2' in obs.content
+
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_insert_invalid_line(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='Line 1\nLine 2',
+            path=test_file,
+        )
+        runtime.run_action(action)
+        action = FileEditAction(
+            command='insert',
+            path=test_file,
+            insert_line=10,
+            new_str='Invalid Insert',
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert 'Invalid `insert_line` parameter' in obs.content
+        assert 'It should be within the range of lines of the file' in obs.content
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_insert_with_empty_string(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='Line 1\nLine 2',
+            path=test_file,
+        )
+        runtime.run_action(action)
+        action = FileEditAction(
+            command='insert',
+            path=test_file,
+            insert_line=1,
+            new_str='',
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert '1\tLine 1' in obs.content
+        assert '2\t\n' in obs.content
+        assert '3\tLine 2' in obs.content
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_insert_with_none_new_str(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='Line 1\nLine 2',
+            path=test_file,
+        )
+        runtime.run_action(action)
+
+        action = FileEditAction(
+            command='insert',
+            path=test_file,
+            insert_line=1,
+            new_str=None,
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert 'ERROR' in obs.content
+        assert 'Parameter `new_str` is required for command: insert' in obs.content
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_undo_edit(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        # Create test file
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='This is a test file.',
+            path=test_file,
+        )
+        runtime.run_action(action)
+
+        # Make an edit
+        action = FileEditAction(
+            command='str_replace',
+            path=test_file,
+            old_str='test',
+            new_str='sample',
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert 'This is a sample file.' in obs.content
+
+        # Undo the edit
+        action = FileEditAction(
+            command='undo_edit',
+            path=test_file,
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert 'Last edit to' in obs.content
+        assert 'This is a test file.' in obs.content
+
+        # Verify file content
+        action = FileEditAction(
+            command='view',
+            path=test_file,
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert 'This is a test file.' in obs.content
+
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_validate_path_invalid(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        invalid_file = os.path.join(
+            config.workspace_mount_path_in_sandbox, 'nonexistent.txt'
+        )
+        action = FileEditAction(
+            command='view',
+            path=invalid_file,
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert 'Invalid `path` parameter' in obs.content
+        assert f'The path {invalid_file} does not exist' in obs.content
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_create_existing_file_error(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='Line 1\nLine 2',
+            path=test_file,
+        )
+        runtime.run_action(action)
+        action = FileEditAction(
+            command='create',
+            path=test_file,
+            file_text='New content',
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert 'File already exists' in obs.content
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_str_replace_missing_old_str(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='Line 1\nLine 2',
+            path=test_file,
+        )
+        runtime.run_action(action)
+        action = FileEditAction(
+            command='str_replace',
+            path=test_file,
+            old_str='',
+            new_str='sample',
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert (
+            'No replacement was performed. Multiple occurrences of old_str ``'
+            in obs.content
+        )
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_str_replace_new_str_and_old_str_same(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='Line 1\nLine 2',
+            path=test_file,
+        )
+        runtime.run_action(action)
+        action = FileEditAction(
+            command='str_replace',
+            path=test_file,
+            old_str='test file',
+            new_str='test file',
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert (
+            'No replacement was performed. `new_str` and `old_str` must be different.'
+            in obs.content
+        )
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_insert_missing_line_param(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        test_file = os.path.join(config.workspace_mount_path_in_sandbox, 'test.txt')
+        action = FileWriteAction(
+            content='Line 1\nLine 2',
+            path=test_file,
+        )
+        runtime.run_action(action)
+        action = FileEditAction(
+            command='insert',
+            path=test_file,
+            new_str='Missing insert line',
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert 'Parameter `insert_line` is required for command: insert' in obs.content
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_undo_edit_no_history_error(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        empty_file = os.path.join(config.workspace_mount_path_in_sandbox, 'empty.txt')
+        action = FileWriteAction(
+            content='',
+            path=empty_file,
+        )
+        runtime.run_action(action)
+
+        action = FileEditAction(
+            command='undo_edit',
+            path=empty_file,
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert 'No edit history found for' in obs.content
+    finally:
+        _close_test_runtime(runtime)
+
+
+def test_view_large_file_with_truncation(temp_dir, runtime_cls, run_as_openhands):
+    runtime, config = _load_runtime(temp_dir, runtime_cls, run_as_openhands)
+    try:
+        # Create a large file to trigger truncation
+        large_file = os.path.join(
+            config.workspace_mount_path_in_sandbox, 'large_test.txt'
+        )
+        large_content = 'Line 1\n' * 16000  # 16000 lines should trigger truncation
+        action = FileWriteAction(
+            content=large_content,
+            path=large_file,
+        )
+        runtime.run_action(action)
+
+        action = FileEditAction(
+            command='view',
+            path=large_file,
+        )
+        obs = runtime.run_action(action)
+        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
+        assert (
+            'Due to the max output limit, only part of this file has been shown to you.'
+            in obs.content
+        )
+    finally:
+        _close_test_runtime(runtime)

tests/runtime/test_ipython.py

@@ -10,12 +10,10 @@ from conftest import (
 from openhands.core.logger import openhands_logger as logger
 from openhands.events.action import (
     CmdRunAction,
-    FileEditAction,
     FileReadAction,
     FileWriteAction,
     IPythonRunCellAction,
 )
-from openhands.events.event import FileEditSource
 from openhands.events.observation import (
     CmdOutputObservation,
     ErrorObservation,
@@ -310,65 +308,3 @@ print(file_editor(command='undo_edit', path='/workspace/test.txt'))
     assert obs.exit_code == 0
 
     _close_test_runtime(runtime)
-
-
-def test_file_read_and_edit_via_oh_aci(runtime_cls, run_as_openhands):
-    runtime, config = _load_runtime(None, runtime_cls, run_as_openhands)
-    sandbox_dir = '/workspace'
-
-    actions = [
-        {
-            'command': 'create',
-            'test_code': f"print(file_editor(command='create', path='{sandbox_dir}/test.txt', file_text='Line 1\\nLine 2\\nLine 3'))",
-            'action_cls': FileEditAction,
-            'assertions': ['File created successfully'],
-        },
-        {
-            'command': 'view',
-            'test_code': f"print(file_editor(command='view', path='{sandbox_dir}/test.txt'))",
-            'action_cls': FileReadAction,
-            'assertions': ['Line 1', 'Line 2', 'Line 3'],
-        },
-        {
-            'command': 'str_replace',
-            'test_code': f"print(file_editor(command='str_replace', path='{sandbox_dir}/test.txt', old_str='Line 2', new_str='New Line 2'))",
-            'action_cls': FileEditAction,
-            'assertions': ['New Line 2'],
-        },
-        {
-            'command': 'undo_edit',
-            'test_code': f"print(file_editor(command='undo_edit', path='{sandbox_dir}/test.txt'))",
-            'action_cls': FileEditAction,
-            'assertions': ['Last edit to', 'undone successfully'],
-        },
-        {
-            'command': 'insert',
-            'test_code': f"print(file_editor(command='insert', path='{sandbox_dir}/test.txt', insert_line=2, new_str='Line 4'))",
-            'action_cls': FileEditAction,
-            'assertions': ['Line 4'],
-        },
-    ]
-
-    for action_info in actions:
-        action_cls = action_info['action_cls']
-
-        kwargs = {
-            'path': f'{sandbox_dir}/test.txt',
-            'translated_ipython_code': action_info['test_code'],
-            'impl_source': FileEditSource.OH_ACI,
-        }
-        if action_info['action_cls'] == FileEditAction:
-            kwargs['content'] = ''  # dummy value required for FileEditAction
-
-        action = action_cls(**kwargs)
-
-        logger.info(action, extra={'msg_type': 'ACTION'})
-        obs = runtime.run_action(action)
-        logger.info(obs, extra={'msg_type': 'OBSERVATION'})
-        for assertion in action_info['assertions']:
-            if action_cls == FileReadAction:
-                assert assertion in obs.content
-            else:
-                assert assertion in str(obs)
-
-    _close_test_runtime(runtime)

tests/unit/test_action_serialization.py

@@ -5,11 +5,13 @@ from openhands.events.action import (
     BrowseInteractiveAction,
     BrowseURLAction,
     CmdRunAction,
+    FileEditAction,
     FileReadAction,
     FileWriteAction,
     MessageAction,
 )
 from openhands.events.action.action import ActionConfirmationStatus
+from openhands.events.action.files import FileEditSource, FileReadSource
 from openhands.events.serialization import (
     event_from_dict,
     event_to_dict,
@@ -135,7 +137,7 @@ def test_file_read_action_serialization_deserialization():
             'end': -1,
             'thought': 'None',
             'impl_source': 'default',
-            'translated_ipython_code': '',
+            'view_range': None,
         },
     }
     serialization_deserialization(original_action_dict, FileReadAction)
@@ -155,7 +157,47 @@ def test_file_write_action_serialization_deserialization():
     serialization_deserialization(original_action_dict, FileWriteAction)
 
 
-def test_legacy_serialization():
+def test_file_edit_action_aci_serialization_deserialization():
+    original_action_dict = {
+        'action': 'edit',
+        'args': {
+            'path': '/path/to/file.txt',
+            'command': 'str_replace',
+            'file_text': None,
+            'old_str': 'old text',
+            'new_str': 'new text',
+            'insert_line': None,
+            'content': '',
+            'start': 1,
+            'end': -1,
+            'thought': 'Replacing text',
+            'impl_source': 'oh_aci',
+        },
+    }
+    serialization_deserialization(original_action_dict, FileEditAction)
+
+
+def test_file_edit_action_llm_serialization_deserialization():
+    original_action_dict = {
+        'action': 'edit',
+        'args': {
+            'path': '/path/to/file.txt',
+            'command': None,
+            'file_text': None,
+            'old_str': None,
+            'new_str': None,
+            'insert_line': None,
+            'content': 'Updated content',
+            'start': 1,
+            'end': 10,
+            'thought': 'Updating file content',
+            'impl_source': 'llm_based_edit',
+        },
+    }
+    serialization_deserialization(original_action_dict, FileEditAction)
+
+
+def test_cmd_run_action_legacy_serialization():
     original_action_dict = {
         'action': 'run',
         'args': {
@@ -183,3 +225,167 @@ def test_legacy_serialization():
     assert event_dict['args']['command'] == 'echo "Hello world"'
     assert event_dict['args']['thought'] == ''
     assert event_dict['args']['is_input'] is False
+
+
+def test_file_llm_based_edit_action_legacy_serialization():
+    original_action_dict = {
+        'action': 'edit',
+        'args': {
+            'path': '/path/to/file.txt',
+            'content': 'dummy content',
+            'start': 1,
+            'end': -1,
+            'thought': 'Replacing text',
+            'impl_source': 'oh_aci',
+            'translated_ipython_code': None,
+        },
+    }
+    event = event_from_dict(original_action_dict)
+    assert isinstance(event, Action)
+    assert isinstance(event, FileEditAction)
+
+    # Common arguments
+    assert event.path == '/path/to/file.txt'
+    assert event.thought == 'Replacing text'
+    assert event.impl_source == FileEditSource.OH_ACI
+    assert not hasattr(event, 'translated_ipython_code')
+
+    # OH_ACI arguments
+    assert event.command == ''
+    assert event.file_text is None
+    assert event.old_str is None
+    assert event.new_str is None
+    assert event.insert_line is None
+
+    # LLM-based editing arguments
+    assert event.content == 'dummy content'
+    assert event.start == 1
+    assert event.end == -1
+
+    event_dict = event_to_dict(event)
+    assert 'translated_ipython_code' not in event_dict['args']
+
+    # Common arguments
+    assert event_dict['args']['path'] == '/path/to/file.txt'
+    assert event_dict['args']['impl_source'] == 'oh_aci'
+    assert event_dict['args']['thought'] == 'Replacing text'
+
+    # OH_ACI arguments
+    assert event_dict['args']['command'] == ''
+    assert event_dict['args']['file_text'] is None
+    assert event_dict['args']['old_str'] is None
+    assert event_dict['args']['new_str'] is None
+    assert event_dict['args']['insert_line'] is None
+
+    # LLM-based editing arguments
+    assert event_dict['args']['content'] == 'dummy content'
+    assert event_dict['args']['start'] == 1
+    assert event_dict['args']['end'] == -1
+
+
+def test_file_ohaci_edit_action_legacy_serialization():
+    original_action_dict = {
+        'action': 'edit',
+        'args': {
+            'path': '/workspace/game_2048.py',
+            'content': '',
+            'start': 1,
+            'end': -1,
+            'thought': "I'll help you create a simple 2048 game in Python. I'll use the str_replace_editor to create the file.",
+            'impl_source': 'oh_aci',
+            'translated_ipython_code': "print(file_editor(**{'command': 'create', 'path': '/workspace/game_2048.py', 'file_text': 'New file content'}))",
+        },
+    }
+    event = event_from_dict(original_action_dict)
+    assert isinstance(event, Action)
+    assert isinstance(event, FileEditAction)
+
+    # Common arguments
+    assert event.path == '/workspace/game_2048.py'
+    assert (
+        event.thought
+        == "I'll help you create a simple 2048 game in Python. I'll use the str_replace_editor to create the file."
+    )
+    assert event.impl_source == FileEditSource.OH_ACI
+    assert not hasattr(event, 'translated_ipython_code')
+
+    # OH_ACI arguments
+    assert event.command == 'create'
+    assert event.file_text == 'New file content'
+    assert event.old_str is None
+    assert event.new_str is None
+    assert event.insert_line is None
+
+    # LLM-based editing arguments
+    assert event.content == ''
+    assert event.start == 1
+    assert event.end == -1
+
+    event_dict = event_to_dict(event)
+    assert 'translated_ipython_code' not in event_dict['args']
+
+    # Common arguments
+    assert event_dict['args']['path'] == '/workspace/game_2048.py'
+    assert event_dict['args']['impl_source'] == 'oh_aci'
+    assert (
+        event_dict['args']['thought']
+        == "I'll help you create a simple 2048 game in Python. I'll use the str_replace_editor to create the file."
+    )
+
+    # OH_ACI arguments
+    assert event_dict['args']['command'] == 'create'
+    assert event_dict['args']['file_text'] == 'New file content'
+    assert event_dict['args']['old_str'] is None
+    assert event_dict['args']['new_str'] is None
+    assert event_dict['args']['insert_line'] is None
+
+    # LLM-based editing arguments
+    assert event_dict['args']['content'] == ''
+    assert event_dict['args']['start'] == 1
+    assert event_dict['args']['end'] == -1
+
+
+def test_file_read_action_legacy_serialization():
+    original_action_dict = {
+        'action': 'read',
+        'args': {
+            'path': '/workspace/test.txt',
+            'start': 0,
+            'end': -1,
+            'thought': 'Reading the file contents',
+            'impl_source': 'oh_aci',
+            'translated_ipython_code': "print(file_editor(**{'command': 'view', 'path': '/workspace/test.txt'}))",
+        },
+    }
+
+    event = event_from_dict(original_action_dict)
+    assert isinstance(event, Action)
+    assert isinstance(event, FileReadAction)
+
+    # Common arguments
+    assert event.path == '/workspace/test.txt'
+    assert event.thought == 'Reading the file contents'
+    assert event.impl_source == FileReadSource.OH_ACI
+    assert not hasattr(event, 'translated_ipython_code')
+    assert not hasattr(
+        event, 'command'
+    )  # FileReadAction should not have command attribute
+
+    # Read-specific arguments
+    assert event.start == 0
+    assert event.end == -1
+
+    event_dict = event_to_dict(event)
+    assert 'translated_ipython_code' not in event_dict['args']
+    assert (
+        'command' not in event_dict['args']
+    )  # command should not be in serialized args
+
+    # Common arguments in serialized form
+    assert event_dict['args']['path'] == '/workspace/test.txt'
+    assert event_dict['args']['impl_source'] == 'oh_aci'
+    assert event_dict['args']['thought'] == 'Reading the file contents'
+
+    # Read-specific arguments in serialized form
+    assert event_dict['args']['start'] == 0
+    assert event_dict['args']['end'] == -1

tests/unit/test_observation_serialization.py

@@ -1,6 +1,8 @@
+from openhands.events.action.files import FileEditSource
 from openhands.events.observation import (
     CmdOutputMetadata,
     CmdOutputObservation,
+    FileEditObservation,
     Observation,
 )
 from openhands.events.serialization import (
@@ -146,3 +148,88 @@ def test_legacy_serialization():
     assert event_dict['extras']['metadata']['pid'] == 3
     assert event_dict['extras']['command'] == 'ls -l'
     assert event_dict['extras']['hidden'] is False
+
+
+def test_file_edit_observation_serialization():
+    original_observation_dict = {
+        'observation': 'edit',
+        'extras': {
+            '_diff_cache': None,
+            'impl_source': FileEditSource.LLM_BASED_EDIT,
+            'new_content': None,
+            'old_content': None,
+            'path': '',
+            'prev_exist': False,
+        },
+        'message': 'I edited the file .',
+        'content': '[Existing file /path/to/file.txt is edited with 1 changes.]',
+    }
+    serialization_deserialization(original_observation_dict, FileEditObservation)
+
+
+def test_file_edit_observation_new_file_serialization():
+    original_observation_dict = {
+        'observation': 'edit',
+        'content': '[New file /path/to/newfile.txt is created with the provided content.]',
+        'extras': {
+            '_diff_cache': None,
+            'impl_source': FileEditSource.LLM_BASED_EDIT,
+            'new_content': None,
+            'old_content': None,
+            'path': '',
+            'prev_exist': False,
+        },
+        'message': 'I edited the file .',
+    }
+
+    serialization_deserialization(original_observation_dict, FileEditObservation)
+
+
+def test_file_edit_observation_oh_aci_serialization():
+    original_observation_dict = {
+        'observation': 'edit',
+        'content': 'The file /path/to/file.txt is edited with the provided content.',
+        'extras': {
+            '_diff_cache': None,
+            'impl_source': FileEditSource.LLM_BASED_EDIT,
+            'new_content': None,
+            'old_content': None,
+            'path': '',
+            'prev_exist': False,
+        },
+        'message': 'I edited the file .',
+    }
+    serialization_deserialization(original_observation_dict, FileEditObservation)
+
+
+def test_file_edit_observation_legacy_serialization():
+    original_observation_dict = {
+        'observation': 'edit',
+        'content': 'content',
+        'extras': {
+            'path': '/workspace/game_2048.py',
+            'prev_exist': False,
+            'old_content': None,
+            'new_content': 'new content',
+            'impl_source': 'oh_aci',
+            'formatted_output_and_error': 'File created successfully at: /workspace/game_2048.py',
+        },
+    }
+
+    event = event_from_dict(original_observation_dict)
+    assert isinstance(event, Observation)
+    assert isinstance(event, FileEditObservation)
+    assert event.impl_source == FileEditSource.OH_ACI
+    assert event.path == '/workspace/game_2048.py'
+    assert event.prev_exist is False
+    assert event.old_content is None
+    assert event.new_content == 'new content'
+    assert not hasattr(event, 'formatted_output_and_error')
+
+    event_dict = event_to_dict(event)
+    assert event_dict['extras']['impl_source'] == 'oh_aci'
+    assert event_dict['extras']['path'] == '/workspace/game_2048.py'
+    assert event_dict['extras']['prev_exist'] is False
+    assert event_dict['extras']['old_content'] is None
+    assert event_dict['extras']['new_content'] == 'new content'
+    assert 'formatted_output_and_error' not in event_dict['extras']