diff --git a/src/agents/sandbox/apply_patch.py b/src/agents/sandbox/apply_patch.py index 30623fdf82..82d12a2b52 100644 --- a/src/agents/sandbox/apply_patch.py +++ b/src/agents/sandbox/apply_patch.py @@ -135,6 +135,27 @@ async def apply_operation( path=operation.path, ) + async def _move_file(self, operation: ApplyPatchOperation) -> ApplyPatchResult: + if operation.move_to is None: + raise ApplyPatchDiffError( + message=f"Missing move destination for path {operation.path}", + path=operation.path, + ) + + relative_path, display_path = self._resolve_path(operation.path) + destination = self._session.normalize_path(relative_path) + moved_relative_path, moved_display_path = self._resolve_path(operation.move_to) + moved_destination = self._session.normalize_path(moved_relative_path) + payload = await self._read_payload(destination, op_path=operation.path) + + if moved_destination != destination: + await self._session.mkdir(moved_destination.parent, parents=True, user=self._user) + data = io.StringIO(payload) if isinstance(payload, str) else io.BytesIO(payload) + await self._session.write(moved_destination, data, user=self._user) + await self._session.rm(destination, user=self._user) + + return ApplyPatchResult(output=f"Moved {display_path} to {moved_display_path}") + def normalize_operation(self, operation: ApplyPatchOperation) -> ApplyPatchOperation: """Return an operation whose paths use the workspace policy's canonical form.""" normalized_path = self._validate_path(operation.path).as_posix() @@ -187,6 +208,16 @@ async def _ensure_exists(self, destination: Path, *, display_path: str) -> None: handle.close() async def _read_text(self, destination: Path, *, op_path: str, decode_path: Path) -> str: + payload = await self._read_payload(destination, op_path=op_path) + + if isinstance(payload, str): + return payload + try: + return payload.decode("utf-8") + except UnicodeDecodeError as exc: + raise ApplyPatchDecodeError(path=decode_path, cause=exc) from exc + + async def _read_payload(self, destination: Path, *, op_path: str) -> str | bytes: try: handle = await self._session.read(destination, user=self._user) except (FileNotFoundError, WorkspaceReadNotFoundError) as exc: @@ -200,10 +231,7 @@ async def _read_text(self, destination: Path, *, op_path: str, decode_path: Path if isinstance(payload, str): return payload if isinstance(payload, bytes | bytearray): - try: - return bytes(payload).decode("utf-8") - except UnicodeDecodeError as exc: - raise ApplyPatchDecodeError(path=decode_path, cause=exc) from exc + return bytes(payload) raise ApplyPatchDiffError( message=f"apply_patch read() returned non-text content: {type(payload).__name__}", path=op_path, diff --git a/src/agents/sandbox/capabilities/tools/apply_patch_tool.py b/src/agents/sandbox/capabilities/tools/apply_patch_tool.py index 6bfeeb4163..44add244ab 100644 --- a/src/agents/sandbox/capabilities/tools/apply_patch_tool.py +++ b/src/agents/sandbox/capabilities/tools/apply_patch_tool.py @@ -28,7 +28,7 @@ hunk: add_hunk | delete_hunk | update_hunk add_hunk: "*** Add File: " filename LF add_line+ delete_hunk: "*** Delete File: " filename LF -update_hunk: "*** Update File: " filename LF change_move? change? +update_hunk: "*** Update File: " filename LF (change_move change? | change) filename: /(.+)/ add_line: "+" /(.*)/ LF -> line @@ -60,7 +60,8 @@ *** Update File: - patch an existing file in place (optionally with a rename). May be immediately followed by *** Move to: if you want to rename the file. -Then one or more hunks, each introduced by @@ (optionally followed by a hunk header). +Content hunks are optional for a rename. Otherwise, include one or more hunks, each +introduced by @@ (optionally followed by a hunk header). Within a hunk, each line starts with a space, -, or +. For context lines: @@ -94,7 +95,7 @@ FileOp := AddFile | DeleteFile | UpdateFile AddFile := "*** Add File: " path NEWLINE { "+" line NEWLINE } DeleteFile := "*** Delete File: " path NEWLINE -UpdateFile := "*** Update File: " path NEWLINE [ MoveTo ] { Hunk } +UpdateFile := "*** Update File: " path NEWLINE (MoveTo { Hunk } | Hunk { Hunk }) MoveTo := "*** Move to: " newPath NEWLINE Hunk := "@@" [ header ] NEWLINE { HunkLine } [ "*** End of File" NEWLINE ] HunkLine := (" " | "-" | "+") text NEWLINE @@ -265,7 +266,14 @@ async def _on_invoke_tool(self, ctx: ToolContext[Any], raw_input: str) -> str: if operation.type == "create_file": result = await self.editor.create_file(operation) elif operation.type == "update_file": - result = await self.editor.update_file(operation) + if operation.diff is None and operation.move_to is not None: + result = await WorkspaceEditor( + self.session, + user=self.editor.user, + workspace_scope=self.workspace_scope, + )._move_file(operation) + else: + result = await self.editor.update_file(operation) elif operation.type == "delete_file": result = await self.editor.delete_file(operation) else: @@ -389,13 +397,13 @@ def _parse_update_file(lines: list[str], index: int) -> tuple[ApplyPatchOperatio while index < len(lines) - 1 and not _is_file_operation_header(lines[index]): diff_lines.append(lines[index]) index += 1 - if not diff_lines: + if not diff_lines and move_to is None: raise ValueError(f"Update File patch for {path} must include a hunk") return ( ApplyPatchOperation( type="update_file", path=path, - diff=_join_diff(diff_lines), + diff=_join_diff(diff_lines) if diff_lines else None, move_to=move_to, ), index, diff --git a/tests/sandbox/capabilities/test_apply_patch_tool.py b/tests/sandbox/capabilities/test_apply_patch_tool.py index db877dea7f..ccbd355997 100644 --- a/tests/sandbox/capabilities/test_apply_patch_tool.py +++ b/tests/sandbox/capabilities/test_apply_patch_tool.py @@ -19,7 +19,11 @@ from agents.run_internal.tool_actions import CustomToolAction from agents.sandbox import SandboxWorkspaceScope from agents.sandbox.capabilities.tools import SandboxApplyPatchTool -from agents.sandbox.errors import ApplyPatchDecodeError, ApplyPatchFileNotFoundError +from agents.sandbox.errors import ( + ApplyPatchDecodeError, + ApplyPatchDiffError, + ApplyPatchFileNotFoundError, +) from agents.sandbox.types import User from agents.testing import scripted_sandbox_session from tests.sandbox._apply_patch_test_session import ( @@ -40,6 +44,10 @@ def test_exposes_custom_apply_patch_tool(self) -> None: assert tool.tool_config["name"] == "apply_patch" assert tool.tool_config["format"]["type"] == "grammar" assert tool.tool_config["format"]["syntax"] == "lark" + assert ( + 'update_hunk: "*** Update File: " filename LF (change_move change? | change)' + in tool.tool_config["format"]["definition"] + ) def test_converter_uses_sandbox_custom_apply_patch_tool_config(self) -> None: tool = SandboxApplyPatchTool(session=scripted_sandbox_session()) @@ -221,6 +229,22 @@ async def test_invalid_patch_input_surfaces_tool_error_after_approval_precheck(s assert isinstance(result, ToolCallOutputItem) assert "apply_patch input must start with '*** Begin Patch'" in result.output + @pytest.mark.asyncio + async def test_empty_update_surfaces_tool_error_without_modifying_file(self) -> None: + session = ApplyPatchSession() + session.files[Path("/workspace/notes.txt")] = b"hello\n" + tool = SandboxApplyPatchTool(session=session) + + result = await _execute_custom_tool_call( + tool, + context_wrapper=make_context_wrapper(), + raw_input=("*** Begin Patch\n*** Update File: notes.txt\n*** End Patch\n"), + ) + + assert isinstance(result, ToolCallOutputItem) + assert "Update File patch for notes.txt must include a hunk" in result.output + assert session.files[Path("/workspace/notes.txt")] == b"hello\n" + @pytest.mark.asyncio async def test_editor_create_update_delete_round_trip(self) -> None: session = ApplyPatchSession() @@ -558,6 +582,27 @@ async def test_editor_move_to_same_path_does_not_remove_the_file(self) -> None: assert session.rm_users == [] assert session.files[Path("/workspace/existing.txt")] == b"new\n" + @pytest.mark.asyncio + async def test_editor_rejects_move_without_diff_before_filesystem_access(self) -> None: + session = ApplyPatchSession() + session.files[Path("/workspace/existing.txt")] = b"old\n" + tool = SandboxApplyPatchTool(session=session) + + with pytest.raises(ApplyPatchDiffError, match="Missing diff"): + await cast( + Awaitable[ApplyPatchResult], + tool.editor.update_file( + ApplyPatchOperation( + type="update_file", + path="existing.txt", + diff=None, + move_to="moved.txt", + ) + ), + ) + + assert session.files == {Path("/workspace/existing.txt"): b"old\n"} + @pytest.mark.asyncio async def test_custom_tool_input_create_update_move_delete(self) -> None: session = ApplyPatchSession() @@ -600,6 +645,28 @@ async def test_custom_tool_input_create_update_move_delete(self) -> None: ) assert Path("/workspace/moved.txt") not in session.files + @pytest.mark.asyncio + async def test_custom_tool_input_moves_file_without_content_hunk(self) -> None: + session = ApplyPatchSession() + original = b"\xffhello\r\nworld\n" + session.files[Path("/workspace/notes.txt")] = original + tool = SandboxApplyPatchTool(session=session) + + result = await _execute_custom_tool_call( + tool, + context_wrapper=make_context_wrapper(), + raw_input=( + "*** Begin Patch\n" + "*** Update File: notes.txt\n" + "*** Move to: moved.txt\n" + "*** End Patch\n" + ), + ) + + assert result.output == "Moved notes.txt to moved.txt" + assert Path("/workspace/notes.txt") not in session.files + assert session.files[Path("/workspace/moved.txt")] == original + async def _execute_custom_tool_call( tool: SandboxApplyPatchTool,