Compare commits
1 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| d3fd8f0f1a |
@@ -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,7 +266,14 @@ class SandboxApplyPatchTool(CustomTool):
|
||||
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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user