Compare commits

...

1 Commits

Author SHA1 Message Date
Kazuhiro Sera d3fd8f0f1a fix(sandbox): support move-only apply patches 2026-08-17 12:59:13 +09:00
3 changed files with 114 additions and 11 deletions
+32 -4
View File
@@ -135,6 +135,27 @@ class WorkspaceEditor:
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 @@ class WorkspaceEditor:
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 @@ class WorkspaceEditor:
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,
@@ -28,7 +28,7 @@ end_patch: "*** End Patch" LF?
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 @@ Each operation starts with one of three headers:
*** Update File: <path> - patch an existing file in place (optionally with a rename).
May be immediately followed by *** Move to: <new path> 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 @@ End := "*** End Patch" NEWLINE
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,6 +266,13 @@ class SandboxApplyPatchTool(CustomTool):
if operation.type == "create_file":
result = await self.editor.create_file(operation)
elif operation.type == "update_file":
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)
@@ -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,
@@ -19,7 +19,11 @@ from agents.run_internal.run_steps import ToolRunCustom
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 @@ class TestSandboxApplyPatchTool:
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 @@ class TestSandboxApplyPatchTool:
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 @@ class TestSandboxApplyPatchTool:
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 @@ class TestSandboxApplyPatchTool:
)
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,