From 2bf1a313aede2f245a2dee19d22dac93d0f2fda5 Mon Sep 17 00:00:00 2001
From: wolfgang-aura <169568318+wolfgang-aura@users.noreply.github.com>
Date: Sun, 6 Sep 2026 21:22:06 +0800
Subject: [PATCH 1/4] fix: keep the file when apply_patch does a case-only
 rename

On a filesystem that folds case, `notes.txt` and `Notes.txt` are the same
file. `_apply_update` wrote the new text to the destination and then removed
the source, and because the path comparison is case-sensitive the removal
deleted the file that had just been written. The user's edit was lost.

For a case-only rename, remove the source before writing instead. That end
state is correct on a folding filesystem and on a case-sensitive one, so the
SDK does not have to know which kind it is talking to. The write is wrapped
so a failure restores the original text at the source path.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 src/agents/sandbox/apply_patch.py          | 51 +++++++++++++-
 tests/sandbox/_apply_patch_test_session.py | 80 +++++++++++++++++++++-
 tests/sandbox/test_apply_patch.py          | 61 ++++++++++++++++-
 3 files changed, 188 insertions(+), 4 deletions(-)

diff --git a/src/agents/sandbox/apply_patch.py b/src/agents/sandbox/apply_patch.py
index 30623fdf82..a5c1c62ef8 100644
--- a/src/agents/sandbox/apply_patch.py
+++ b/src/agents/sandbox/apply_patch.py
@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import contextlib
 import io
 from pathlib import Path
 from typing import TYPE_CHECKING, Any, Literal, Protocol, cast, runtime_checkable
@@ -111,9 +112,23 @@ async def apply_operation(
 
             moved_relative_path, moved_display_path = self._resolve_path(operation.move_to)
             moved_destination = self._session.normalize_path(moved_relative_path)
-            await self._write_text(moved_destination, updated_text)
-            if moved_destination != destination:
+            # A sandbox filesystem that folds case stores notes.txt and Notes.txt as one file, so
+            # removing the source after the write would delete the text that was just written.
+            # Removing the source first renames the file on a case-folding filesystem and on a
+            # case-sensitive one. Every other rename keeps the write-then-remove order so a failed
+            # write leaves the source intact.
+            if _is_case_only_rename(destination, moved_destination):
                 await self._session.rm(destination, user=self._user)
+                await self._write_moved_text(
+                    moved_destination,
+                    updated_text,
+                    restore_destination=destination,
+                    restore_text=original_text,
+                )
+            else:
+                await self._write_text(moved_destination, updated_text)
+                if moved_destination != destination:
+                    await self._session.rm(destination, user=self._user)
             return ApplyPatchResult(
                 output=f"Updated {display_path}\nMoved {display_path} to {moved_display_path}"
             )
@@ -209,6 +224,28 @@ async def _read_text(self, destination: Path, *, op_path: str, decode_path: Path
             path=op_path,
         )
 
+    async def _write_moved_text(
+        self,
+        destination: Path,
+        text: str,
+        *,
+        restore_destination: Path,
+        restore_text: str,
+    ) -> None:
+        """Write a rename destination whose source was already removed, restoring it on failure.
+
+        The source is gone while this write is in flight, so a sandbox write that fails partway
+        through, for example on a dropped connection to a remote session, would otherwise leave
+        the file in neither place. The original text is still in memory, so put it back before
+        the failure propagates.
+        """
+        try:
+            await self._write_text(destination, text)
+        except BaseException:
+            with contextlib.suppress(Exception):
+                await self._write_text(restore_destination, restore_text)
+            raise
+
     async def _write_text(self, destination: Path, text: str) -> None:
         await self._session.mkdir(destination.parent, parents=True, user=self._user)
         await self._session.write(
@@ -218,6 +255,16 @@ async def _write_text(self, destination: Path, text: str) -> None:
         )
 
 
+def _is_case_only_rename(source: Path, destination: Path) -> bool:
+    """Return whether two sandbox paths are distinct and differ only in character case."""
+
+    source_posix = source.as_posix()
+    destination_posix = destination.as_posix()
+    if source_posix == destination_posix:
+        return False
+    return source_posix.casefold() == destination_posix.casefold()
+
+
 def _coerce_operations(
     operations: ApplyPatchOperation
     | dict[str, object]
diff --git a/tests/sandbox/_apply_patch_test_session.py b/tests/sandbox/_apply_patch_test_session.py
index 24ce567011..737b1f9204 100644
--- a/tests/sandbox/_apply_patch_test_session.py
+++ b/tests/sandbox/_apply_patch_test_session.py
@@ -2,7 +2,8 @@
 
 import io
 import uuid
-from pathlib import Path
+from pathlib import Path, PurePosixPath
+from typing import cast
 
 from agents.sandbox import Manifest
 from agents.sandbox.errors import WorkspaceReadNotFoundError
@@ -94,6 +95,83 @@ async def rm(
         self.files.pop(normalized, None)
 
 
+class PosixHostApplyPatchSession(ApplyPatchSession):
+    """An apply_patch session whose workspace paths compare case-sensitively on every host.
+
+    Linux and macOS hosts compare sandbox paths case-sensitively, while a Windows host folds
+    case in `Path` comparisons. `PurePosixPath` keeps the host comparison case-sensitive
+    everywhere so case-only rename coverage does not depend on the operating system that runs
+    the tests.
+    """
+
+    def normalize_path(self, path: Path | str, *, for_write: bool = False) -> Path:
+        normalized = super().normalize_path(path, for_write=for_write)
+        return cast(Path, PurePosixPath(normalized.as_posix()))
+
+
+class CaseFoldingApplyPatchSession(PosixHostApplyPatchSession):
+    """A case-sensitive host over a sandbox filesystem that folds path case.
+
+    APFS, NTFS, and Docker bind mounts backed by either store `notes.txt` and `Notes.txt` as
+    one file, and they preserve the case of the name that created the file. Lookups here fold
+    case so an existing entry keeps its stored name when it is written again.
+    """
+
+    def _stored_path(self, path: Path | str) -> Path:
+        normalized = self.normalize_path(path)
+        folded = normalized.as_posix().casefold()
+        for stored in self.files:
+            if stored.as_posix().casefold() == folded:
+                return stored
+        return normalized
+
+    async def read(self, path: Path, *, user: str | User | None = None) -> io.BytesIO:
+        return await super().read(self._stored_path(path), user=user)
+
+    async def write(
+        self,
+        path: Path,
+        data: io.IOBase,
+        *,
+        user: str | User | None = None,
+    ) -> None:
+        await super().write(self._stored_path(path), data, user=user)
+
+    async def rm(
+        self,
+        path: Path | str,
+        *,
+        recursive: bool = False,
+        user: str | User | None = None,
+    ) -> None:
+        await super().rm(self._stored_path(path), recursive=recursive, user=user)
+
+
+class WriteFailureApplyPatchSession(CaseFoldingApplyPatchSession):
+    """A case-folding session whose first write fails, as a dropped sandbox connection would.
+
+    A case-only rename removes the source before it writes the destination, so the file exists
+    in neither place while that write is in flight. Failing only the first write leaves the
+    restoring write able to succeed.
+    """
+
+    def __init__(self, manifest: Manifest | None = None) -> None:
+        super().__init__(manifest)
+        self.fail_next_write = True
+
+    async def write(
+        self,
+        path: Path,
+        data: io.IOBase,
+        *,
+        user: str | User | None = None,
+    ) -> None:
+        if self.fail_next_write:
+            self.fail_next_write = False
+            raise ConnectionError("sandbox write failed")
+        await super().write(path, data, user=user)
+
+
 class ProviderNotFoundApplyPatchSession(ApplyPatchSession):
     async def read(self, path: Path, *, user: str | User | None = None) -> io.BytesIO:
         try:
diff --git a/tests/sandbox/test_apply_patch.py b/tests/sandbox/test_apply_patch.py
index c4cd676fec..8106bf5127 100644
--- a/tests/sandbox/test_apply_patch.py
+++ b/tests/sandbox/test_apply_patch.py
@@ -1,6 +1,7 @@
 from __future__ import annotations
 
-from pathlib import Path
+from pathlib import Path, PurePosixPath
+from typing import cast
 
 import pytest
 
@@ -14,7 +15,10 @@
 )
 from tests.sandbox._apply_patch_test_session import (
     ApplyPatchSession,
+    CaseFoldingApplyPatchSession,
+    PosixHostApplyPatchSession,
     ProviderNotFoundApplyPatchSession,
+    WriteFailureApplyPatchSession,
 )
 
 
@@ -247,6 +251,61 @@ async def test_apply_patch_normalizes_backslashes_in_move_to() -> None:
     assert Path("/workspace/source.txt") not in session.files
 
 
+@pytest.mark.asyncio
+async def test_apply_patch_case_only_move_to_keeps_file_on_case_folding_filesystem() -> None:
+    """A case-folding filesystem stores both names as one file, which the removal must keep."""
+    session = CaseFoldingApplyPatchSession()
+    session.files[cast(Path, PurePosixPath("/workspace/notes.txt"))] = b"alpha\nbeta\n"
+
+    await session.apply_patch(
+        ApplyPatchOperation(
+            type="update_file",
+            path="notes.txt",
+            diff="@@\n alpha\n-beta\n+gamma\n",
+            move_to="Notes.txt",
+        )
+    )
+
+    assert session.files == {PurePosixPath("/workspace/Notes.txt"): b"alpha\ngamma\n"}
+
+
+@pytest.mark.asyncio
+async def test_apply_patch_case_only_move_to_moves_file_on_case_sensitive_filesystem() -> None:
+    """A case-sensitive filesystem keeps the names apart, so the source must still be removed."""
+    session = PosixHostApplyPatchSession()
+    session.files[cast(Path, PurePosixPath("/workspace/notes.txt"))] = b"alpha\nbeta\n"
+
+    await session.apply_patch(
+        ApplyPatchOperation(
+            type="update_file",
+            path="notes.txt",
+            diff="@@\n alpha\n-beta\n+gamma\n",
+            move_to="Notes.txt",
+        )
+    )
+
+    assert session.files == {PurePosixPath("/workspace/Notes.txt"): b"alpha\ngamma\n"}
+
+
+@pytest.mark.asyncio
+async def test_apply_patch_case_only_move_to_restores_source_when_write_fails() -> None:
+    """The source is removed before the write, so a failed write must put the file back."""
+    session = WriteFailureApplyPatchSession()
+    session.files[cast(Path, PurePosixPath("/workspace/notes.txt"))] = b"alpha\nbeta\n"
+
+    with pytest.raises(ConnectionError):
+        await session.apply_patch(
+            ApplyPatchOperation(
+                type="update_file",
+                path="notes.txt",
+                diff="@@\n alpha\n-beta\n+gamma\n",
+                move_to="Notes.txt",
+            )
+        )
+
+    assert session.files == {PurePosixPath("/workspace/notes.txt"): b"alpha\nbeta\n"}
+
+
 @pytest.mark.asyncio
 async def test_apply_patch_allows_absolute_path_within_root() -> None:
     session = ApplyPatchSession()

From 5b3650ca64bc926cf8278ec7f6ca313f05011f1b Mon Sep 17 00:00:00 2001
From: wolfgang-aura <169568318+wolfgang-aura@users.noreply.github.com>
Date: Mon, 7 Sep 2026 23:23:56 +0800
Subject: [PATCH 2/4] fix: commit the rename before removing the source

The previous commit removed the source first on a case-only rename and
restored the original text from memory if the write then failed. That trades
one way to lose the file for another: if the write and the restore both fail
the file is gone, and a restore that lands late overwrites whatever another
writer put at the source path in the meantime.

Ask the filesystem instead of comparing strings. `same_file` runs
`[ "$1" -ef "$2" ]`, which compares device and inode, so it answers for a
filesystem that folds case and for one that folds Unicode normalisation, which
`str.casefold` cannot. `mv` renames a path and refuses a directory
destination, because `mv` given a directory moves the source inside it and
exits 0, which for a caller that then removes the source is a way to delete a
file while believing it moved.

The rename now writes a staging file, commits it onto the destination with one
move, and removes the source only when the filesystem says it is a different
file. Nothing is removed before the new content is on disk, and nothing is
written back after a failure.

Three of the new tests run on the macOS runner against a real case-folding
volume, so the behaviour is no longer only modelled.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 docs/testing.md                               |   2 +-
 src/agents/sandbox/apply_patch.py             |  68 ++++-----
 .../sandbox/session/base_sandbox_session.py   |  83 +++++++++++
 src/agents/sandbox/session/sandbox_session.py |  18 +++
 src/agents/testing/sandbox.py                 |  20 +++
 tests/sandbox/_apply_patch_test_session.py    | 137 +++++++++++++++++-
 tests/sandbox/test_apply_patch.py             | 110 +++++++++++++-
 tests/sandbox/test_unix_local.py              |  84 ++++++++++-
 8 files changed, 473 insertions(+), 49 deletions(-)

diff --git a/docs/testing.md b/docs/testing.md
index 57e5064615..d7cab034af 100644
--- a/docs/testing.md
+++ b/docs/testing.md
@@ -286,7 +286,7 @@ Each matching Sandbox call consumes the next step in one global FIFO sequence. A
 | `error` | The method should raise a specific exception |
 | `match` | The call should be rejected before producing its outcome unless the matcher returns a value other than `False` |
 
-The supported scripted method names are `apply_patch`, `exec`, `ls`, `mkdir`, `pty_exec_start`, `pty_write_stdin`, `read`, `rm`, and `write`. Only configured model-facing capabilities are exposed. The two PTY methods are exposed together when either PTY method is configured because they form one interactive-shell capability, but calls still consume the global FIFO script.
+The supported scripted method names are `apply_patch`, `exec`, `ls`, `mkdir`, `mv`, `pty_exec_start`, `pty_write_stdin`, `read`, `rm`, `same_file`, and `write`. Only configured model-facing capabilities are exposed. The two PTY methods are exposed together when either PTY method is configured because they form one interactive-shell capability, but calls still consume the global FIFO script.
 
 `sandbox.calls` contains detached `SandboxCall` snapshots with zero-based `call_index`, `method`, positional `args`, and read-only `kwargs`. Static results are also snapshotted when the script is created. `io.BytesIO` and `io.StringIO` values are supported; use a custom Sandbox session for other live stream objects or lifecycle behavior.
 
diff --git a/src/agents/sandbox/apply_patch.py b/src/agents/sandbox/apply_patch.py
index a5c1c62ef8..a6ade970fb 100644
--- a/src/agents/sandbox/apply_patch.py
+++ b/src/agents/sandbox/apply_patch.py
@@ -4,6 +4,7 @@
 import io
 from pathlib import Path
 from typing import TYPE_CHECKING, Any, Literal, Protocol, cast, runtime_checkable
+from uuid import uuid4
 
 from ..apply_diff import ApplyDiffMode, apply_diff
 from ..editor import ApplyPatchOperation, ApplyPatchOperationType, ApplyPatchResult
@@ -112,23 +113,11 @@ async def apply_operation(
 
             moved_relative_path, moved_display_path = self._resolve_path(operation.move_to)
             moved_destination = self._session.normalize_path(moved_relative_path)
-            # A sandbox filesystem that folds case stores notes.txt and Notes.txt as one file, so
-            # removing the source after the write would delete the text that was just written.
-            # Removing the source first renames the file on a case-folding filesystem and on a
-            # case-sensitive one. Every other rename keeps the write-then-remove order so a failed
-            # write leaves the source intact.
-            if _is_case_only_rename(destination, moved_destination):
-                await self._session.rm(destination, user=self._user)
-                await self._write_moved_text(
-                    moved_destination,
-                    updated_text,
-                    restore_destination=destination,
-                    restore_text=original_text,
-                )
-            else:
-                await self._write_text(moved_destination, updated_text)
-                if moved_destination != destination:
-                    await self._session.rm(destination, user=self._user)
+            await self._move_updated_text(
+                source=destination,
+                moved_destination=moved_destination,
+                text=updated_text,
+            )
             return ApplyPatchResult(
                 output=f"Updated {display_path}\nMoved {display_path} to {moved_display_path}"
             )
@@ -224,27 +213,36 @@ async def _read_text(self, destination: Path, *, op_path: str, decode_path: Path
             path=op_path,
         )
 
-    async def _write_moved_text(
+    async def _move_updated_text(
         self,
-        destination: Path,
-        text: str,
         *,
-        restore_destination: Path,
-        restore_text: str,
+        source: Path,
+        moved_destination: Path,
+        text: str,
     ) -> None:
-        """Write a rename destination whose source was already removed, restoring it on failure.
-
-        The source is gone while this write is in flight, so a sandbox write that fails partway
-        through, for example on a dropped connection to a remote session, would otherwise leave
-        the file in neither place. The original text is still in memory, so put it back before
-        the failure propagates.
+        """Apply an update that renames the file, without a window in which it does not exist.
+
+        Writing the destination and then removing the source destroys the file whenever the two
+        paths are one file on disk, which is what a case-only rename is on a filesystem that
+        folds case. Removing the source first destroys it whenever the replacement write fails.
+
+        So neither path is written or removed until the new content is committed somewhere else:
+        the text goes to a staging file, a single `mv` puts it at the destination, and only then
+        is the source removed, and only if the filesystem says it is a different file. Before
+        that `mv` the original is untouched; after it the new content exists. There is no moment
+        where the only copy is in memory, and nothing is restored after the fact, so a file that
+        another writer creates at the source path while this runs is never overwritten.
         """
+        staging = moved_destination.with_name(f".{moved_destination.name}.{uuid4().hex[:8]}.tmp")
+        await self._write_text(staging, text)
         try:
-            await self._write_text(destination, text)
+            await self._session.mv(staging, moved_destination, user=self._user)
         except BaseException:
             with contextlib.suppress(Exception):
-                await self._write_text(restore_destination, restore_text)
+                await self._session.rm(staging, user=self._user)
             raise
+        if not await self._session.same_file(source, moved_destination, user=self._user):
+            await self._session.rm(source, user=self._user)
 
     async def _write_text(self, destination: Path, text: str) -> None:
         await self._session.mkdir(destination.parent, parents=True, user=self._user)
@@ -255,16 +253,6 @@ async def _write_text(self, destination: Path, text: str) -> None:
         )
 
 
-def _is_case_only_rename(source: Path, destination: Path) -> bool:
-    """Return whether two sandbox paths are distinct and differ only in character case."""
-
-    source_posix = source.as_posix()
-    destination_posix = destination.as_posix()
-    if source_posix == destination_posix:
-        return False
-    return source_posix.casefold() == destination_posix.casefold()
-
-
 def _coerce_operations(
     operations: ApplyPatchOperation
     | dict[str, object]
diff --git a/src/agents/sandbox/session/base_sandbox_session.py b/src/agents/sandbox/session/base_sandbox_session.py
index d377bea9ef..19b16ce79f 100644
--- a/src/agents/sandbox/session/base_sandbox_session.py
+++ b/src/agents/sandbox/session/base_sandbox_session.py
@@ -1146,6 +1146,89 @@ async def rm(
         if not result.ok():
             raise ExecNonZeroError(result, command=cmd)
 
+    async def mv(
+        self,
+        source: Path | str,
+        destination: Path | str,
+        *,
+        user: str | User | None = None,
+    ) -> None:
+        """Rename a path, replacing the destination if it exists.
+
+        This is a rename, not `mv`'s other behavior. Given an existing directory as the
+        destination, `mv` puts the source inside it and reports success, which for a caller
+        that then removes the source is a way to delete a file while believing it moved. The
+        destination is checked in the same shell invocation as the move, which keeps the
+        check and the move in one round trip. It does not make them one syscall.
+
+        `mv -T` would say this directly and is GNU-only, so it is unavailable on the BSD
+        userland this also has to run against.
+
+        :param source: Path to move.
+        :param destination: Path to move it to.
+        :param user: Optional sandbox user to move as.
+        :raises ExecNonZeroError: If the destination is an existing directory, or the move
+                fails.
+        """
+        source = await self._validate_path_access(source, for_write=True)
+        destination = await self._validate_path_access(destination, for_write=True)
+
+        source_arg = sandbox_path_str(source)
+        destination_arg = sandbox_path_str(destination)
+        cmd = (
+            "sh",
+            "-lc",
+            'if [ -d "$2" ]; then exit 3; fi\nmv -f -- "$1" "$2"',
+            "sh",
+            source_arg,
+            destination_arg,
+        )
+        result = await self.exec(*cmd, shell=False, user=user)
+        if not result.ok():
+            raise ExecNonZeroError(
+                result, command=("sh", "-lc", "<mv>", source_arg, destination_arg)
+            )
+
+    async def same_file(
+        self,
+        left: Path | str,
+        right: Path | str,
+        *,
+        user: str | User | None = None,
+    ) -> bool:
+        """Return whether two paths name the same file on the sandbox filesystem.
+
+        This asks the filesystem, through `test -ef`, which compares device and inode. Two
+        paths that differ as strings can be one file: a filesystem that folds case stores
+        `notes.txt` and `Notes.txt` as a single entry, and APFS folds Unicode normalization
+        as well, so the NFC and NFD spellings of one accented name are also a single entry.
+        No string comparison can answer this, and neither can the host that is driving the
+        session, which may not be the kind of system the sandbox is running on.
+
+        :param left: First path to compare.
+        :param right: Second path to compare.
+        :param user: Optional sandbox user to compare as.
+        :returns: True when both paths resolve to the same file.
+        """
+        left = await self._validate_path_access(left)
+        right = await self._validate_path_access(right)
+
+        left_arg = sandbox_path_str(left)
+        right_arg = sandbox_path_str(right)
+        cmd = ("sh", "-lc", '[ "$1" -ef "$2" ]', "sh", left_arg, right_arg)
+        result = await self.exec(*cmd, shell=False, user=user)
+        if result.exit_code == 0:
+            return True
+        # `[` answers "different file" with 1 and reports its own failures with 2, and a
+        # missing shell exits 127. Only 1 is an answer; anything else is the session
+        # failing to tell us, and a caller about to delete a file on the strength of this
+        # must not read that as "different".
+        if result.exit_code == 1:
+            return False
+        raise ExecNonZeroError(
+            result, command=("sh", "-lc", "<same_file_check>", left_arg, right_arg)
+        )
+
     async def mkdir(
         self,
         path: Path | str,
diff --git a/src/agents/sandbox/session/sandbox_session.py b/src/agents/sandbox/session/sandbox_session.py
index 923f025857..5d78fa67cc 100644
--- a/src/agents/sandbox/session/sandbox_session.py
+++ b/src/agents/sandbox/session/sandbox_session.py
@@ -641,6 +641,24 @@ async def rm(
     ) -> None:
         await self._inner.rm(path, recursive=recursive, user=user)
 
+    async def mv(
+        self,
+        source: Path | str,
+        destination: Path | str,
+        *,
+        user: str | User | None = None,
+    ) -> None:
+        await self._inner.mv(source, destination, user=user)
+
+    async def same_file(
+        self,
+        left: Path | str,
+        right: Path | str,
+        *,
+        user: str | User | None = None,
+    ) -> bool:
+        return await self._inner.same_file(left, right, user=user)
+
     async def mkdir(
         self,
         path: Path | str,
diff --git a/src/agents/testing/sandbox.py b/src/agents/testing/sandbox.py
index d086ae7c9d..dfd674181c 100644
--- a/src/agents/testing/sandbox.py
+++ b/src/agents/testing/sandbox.py
@@ -27,10 +27,12 @@
     "exec",
     "ls",
     "mkdir",
+    "mv",
     "pty_exec_start",
     "pty_write_stdin",
     "read",
     "rm",
+    "same_file",
     "write",
 ]
 SandboxStepReason = Literal["invalid_input", "unknown_method", "invalid_matcher", "invalid_outcome"]
@@ -489,6 +491,24 @@ async def mkdir(
     ) -> None:
         await self._invoke("mkdir", (path,), {"parents": parents, "user": user})
 
+    async def mv(
+        self,
+        source: Path | str,
+        destination: Path | str,
+        *,
+        user: str | User | None = None,
+    ) -> None:
+        await self._invoke("mv", (source, destination), {"user": user})
+
+    async def same_file(
+        self,
+        left: Path | str,
+        right: Path | str,
+        *,
+        user: str | User | None = None,
+    ) -> bool:
+        return cast(bool, await self._invoke("same_file", (left, right), {"user": user}))
+
     async def apply_patch(
         self,
         operations: ApplyPatchOperation
diff --git a/tests/sandbox/_apply_patch_test_session.py b/tests/sandbox/_apply_patch_test_session.py
index 737b1f9204..9ddf096f1a 100644
--- a/tests/sandbox/_apply_patch_test_session.py
+++ b/tests/sandbox/_apply_patch_test_session.py
@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 import io
+import unicodedata
 import uuid
 from pathlib import Path, PurePosixPath
 from typing import cast
@@ -22,6 +23,16 @@ def __init__(self, manifest: Manifest | None = None) -> None:
         self.files: dict[Path, bytes] = {}
         self.mkdir_calls: list[tuple[Path, bool]] = []
         self.rm_calls: list[tuple[Path, bool]] = []
+        self.mv_calls: list[tuple[Path, Path]] = []
+        self.directories: set[Path] = set()
+
+    def _stored_path(self, path: Path | str) -> Path:
+        """Return the key this store holds `path` under.
+
+        A store that folds names overrides this. Here every distinct spelling is a distinct
+        file, which is what a case-sensitive filesystem does.
+        """
+        return self.normalize_path(path)
 
     async def start(self) -> None:
         return None
@@ -94,6 +105,39 @@ async def rm(
         self.rm_calls.append((normalized, recursive))
         self.files.pop(normalized, None)
 
+    async def mv(
+        self,
+        source: Path | str,
+        destination: Path | str,
+        *,
+        user: str | User | None = None,
+    ) -> None:
+        _ = user
+        if self.normalize_path(destination) in self.directories:
+            # A real `mv` would move the source inside this directory and report success.
+            raise IsADirectoryError(self.normalize_path(destination))
+        stored_source = self._stored_path(source)
+        if stored_source not in self.files:
+            raise FileNotFoundError(stored_source)
+        payload = self.files.pop(stored_source)
+        # Look the destination up after removing the source, so a case-only rename does not
+        # find the entry it is renaming. A real `mv` replaces whatever is at the destination
+        # and stores the name it was given, which is how a case-only rename changes the case.
+        self.files.pop(self._stored_path(destination), None)
+        normalized_destination = self.normalize_path(destination)
+        self.files[normalized_destination] = payload
+        self.mv_calls.append((stored_source, normalized_destination))
+
+    async def same_file(
+        self,
+        left: Path | str,
+        right: Path | str,
+        *,
+        user: str | User | None = None,
+    ) -> bool:
+        _ = user
+        return self._stored_path(left) == self._stored_path(right)
+
 
 class PosixHostApplyPatchSession(ApplyPatchSession):
     """An apply_patch session whose workspace paths compare case-sensitively on every host.
@@ -147,14 +191,47 @@ async def rm(
         await super().rm(self._stored_path(path), recursive=recursive, user=user)
 
 
-class WriteFailureApplyPatchSession(CaseFoldingApplyPatchSession):
-    """A case-folding session whose first write fails, as a dropped sandbox connection would.
+class NormalizationFoldingApplyPatchSession(PosixHostApplyPatchSession):
+    """A case-sensitive host over a filesystem that folds case and Unicode normalization.
 
-    A case-only rename removes the source before it writes the destination, so the file exists
-    in neither place while that write is in flight. Failing only the first write leaves the
-    restoring write able to succeed.
+    This is APFS. It stores one entry for the decomposed and the composed spelling of the same
+    accented name, as well as for `notes.txt` and `Notes.txt`. `str.casefold` answers the first
+    pair wrong, which is one reason the fix may not ask a string whether two paths are one file.
     """
 
+    def _stored_path(self, path: Path | str) -> Path:
+        normalized = self.normalize_path(path)
+        folded = unicodedata.normalize("NFC", normalized.as_posix()).casefold()
+        for stored in self.files:
+            if unicodedata.normalize("NFC", stored.as_posix()).casefold() == folded:
+                return stored
+        return normalized
+
+    async def read(self, path: Path, *, user: str | User | None = None) -> io.BytesIO:
+        return await ApplyPatchSession.read(self, self._stored_path(path), user=user)
+
+    async def write(
+        self,
+        path: Path,
+        data: io.IOBase,
+        *,
+        user: str | User | None = None,
+    ) -> None:
+        await ApplyPatchSession.write(self, self._stored_path(path), data, user=user)
+
+    async def rm(
+        self,
+        path: Path | str,
+        *,
+        recursive: bool = False,
+        user: str | User | None = None,
+    ) -> None:
+        await ApplyPatchSession.rm(self, self._stored_path(path), recursive=recursive, user=user)
+
+
+class WriteFailureApplyPatchSession(CaseFoldingApplyPatchSession):
+    """A case-folding session whose first write fails, as a dropped sandbox connection would."""
+
     def __init__(self, manifest: Manifest | None = None) -> None:
         super().__init__(manifest)
         self.fail_next_write = True
@@ -172,6 +249,34 @@ async def write(
         await super().write(path, data, user=user)
 
 
+class ConcurrentWriterApplyPatchSession(PosixHostApplyPatchSession):
+    """A case-sensitive session where another writer takes the source path and the move fails.
+
+    The rename is committed by a move. This session lets the staging write land, then has a
+    second writer put its own file at the source path, then fails the move. The operation
+    cannot succeed from here. What it must not do is put the original text back over the file
+    that other writer just created.
+    """
+
+    def __init__(self, manifest: Manifest | None = None) -> None:
+        super().__init__(manifest)
+        self.concurrent_source: Path | None = None
+        self.concurrent_text = "written by someone else\n"
+
+    async def mv(
+        self,
+        source: Path | str,
+        destination: Path | str,
+        *,
+        user: str | User | None = None,
+    ) -> None:
+        if self.concurrent_source is not None:
+            self.files[self.normalize_path(self.concurrent_source)] = self.concurrent_text.encode(
+                "utf-8"
+            )
+        raise ConnectionError("sandbox move failed")
+
+
 class ProviderNotFoundApplyPatchSession(ApplyPatchSession):
     async def read(self, path: Path, *, user: str | User | None = None) -> io.BytesIO:
         try:
@@ -190,6 +295,8 @@ def __init__(self, manifest: Manifest | None = None) -> None:
         self.write_users: list[str | None] = []
         self.mkdir_users: list[str | None] = []
         self.rm_users: list[str | None] = []
+        self.mv_users: list[str | None] = []
+        self.same_file_users: list[str | None] = []
 
     @staticmethod
     def _user_name(user: str | User | None) -> str | None:
@@ -228,3 +335,23 @@ async def rm(
     ) -> None:
         self.rm_users.append(self._user_name(user))
         await super().rm(path, recursive=recursive)
+
+    async def mv(
+        self,
+        source: Path | str,
+        destination: Path | str,
+        *,
+        user: str | User | None = None,
+    ) -> None:
+        self.mv_users.append(self._user_name(user))
+        await super().mv(source, destination)
+
+    async def same_file(
+        self,
+        left: Path | str,
+        right: Path | str,
+        *,
+        user: str | User | None = None,
+    ) -> bool:
+        self.same_file_users.append(self._user_name(user))
+        return await super().same_file(left, right)
diff --git a/tests/sandbox/test_apply_patch.py b/tests/sandbox/test_apply_patch.py
index 8106bf5127..886515163a 100644
--- a/tests/sandbox/test_apply_patch.py
+++ b/tests/sandbox/test_apply_patch.py
@@ -16,6 +16,8 @@
 from tests.sandbox._apply_patch_test_session import (
     ApplyPatchSession,
     CaseFoldingApplyPatchSession,
+    ConcurrentWriterApplyPatchSession,
+    NormalizationFoldingApplyPatchSession,
     PosixHostApplyPatchSession,
     ProviderNotFoundApplyPatchSession,
     WriteFailureApplyPatchSession,
@@ -288,8 +290,8 @@ async def test_apply_patch_case_only_move_to_moves_file_on_case_sensitive_filesy
 
 
 @pytest.mark.asyncio
-async def test_apply_patch_case_only_move_to_restores_source_when_write_fails() -> None:
-    """The source is removed before the write, so a failed write must put the file back."""
+async def test_apply_patch_move_to_leaves_the_source_alone_when_the_write_fails() -> None:
+    """Nothing is removed until the replacement is committed, so a failed write changes nothing."""
     session = WriteFailureApplyPatchSession()
     session.files[cast(Path, PurePosixPath("/workspace/notes.txt"))] = b"alpha\nbeta\n"
 
@@ -304,6 +306,110 @@ async def test_apply_patch_case_only_move_to_restores_source_when_write_fails()
         )
 
     assert session.files == {PurePosixPath("/workspace/notes.txt"): b"alpha\nbeta\n"}
+    # The file surviving is not enough. The refused implementation removed the source and then
+    # wrote it back, which also ends here. Nothing may be removed at all.
+    assert session.rm_calls == []
+
+
+@pytest.mark.asyncio
+async def test_apply_patch_move_to_does_not_overwrite_a_concurrent_writer_after_a_failure() -> None:
+    """A failed move must not restore the original over a file another writer just created.
+
+    The operation cannot finish once the move fails. The question is what it leaves behind. An
+    implementation that kept the original text in memory and wrote it back at the source path
+    would destroy whatever arrived there in the meantime.
+    """
+    session = ConcurrentWriterApplyPatchSession()
+    source = cast(Path, PurePosixPath("/workspace/notes.txt"))
+    session.files[source] = b"alpha\nbeta\n"
+    session.concurrent_source = source
+
+    with pytest.raises(ConnectionError):
+        await session.apply_patch(
+            ApplyPatchOperation(
+                type="update_file",
+                path="notes.txt",
+                diff="@@\n alpha\n-beta\n+gamma\n",
+                move_to="Notes.txt",
+            )
+        )
+
+    assert session.files[source] == b"written by someone else\n"
+    assert PurePosixPath("/workspace/Notes.txt") not in session.files
+    assert not [path for path in session.files if path.name.endswith(".tmp")]
+
+
+@pytest.mark.asyncio
+async def test_apply_patch_move_to_keeps_the_file_when_only_unicode_normalization_changes() -> None:
+    """APFS folds NFC against NFD, so the two spellings of one accented name are one file.
+
+    `str.casefold` does not normalize, so any fix that compares folded strings sends this pair
+    down the path that destroys it. Asking the filesystem covers it without naming the case.
+    """
+    session = NormalizationFoldingApplyPatchSession()
+    decomposed = "/workspace/cafe\u0301.txt"
+    composed = "/workspace/caf\u00e9.txt"
+    session.files[cast(Path, PurePosixPath(decomposed))] = b"alpha\nbeta\n"
+
+    await session.apply_patch(
+        ApplyPatchOperation(
+            type="update_file",
+            path=decomposed,
+            diff="@@\n alpha\n-beta\n+gamma\n",
+            move_to=composed,
+        )
+    )
+
+    assert session.files == {PurePosixPath(composed): b"alpha\ngamma\n"}
+
+
+@pytest.mark.asyncio
+async def test_apply_patch_move_to_an_existing_directory_keeps_the_source() -> None:
+    """`mv` moves a file into a directory destination and calls that success.
+
+    The source would then be removed on the strength of that success, and the operation would
+    report a move that did not happen. `move_to` comes from the model, so this is reachable.
+    """
+    session = PosixHostApplyPatchSession()
+    source = cast(Path, PurePosixPath("/workspace/notes.txt"))
+    session.files[source] = b"alpha\nbeta\n"
+    session.directories.add(cast(Path, PurePosixPath("/workspace/docs")))
+
+    with pytest.raises(IsADirectoryError):
+        await session.apply_patch(
+            ApplyPatchOperation(
+                type="update_file",
+                path="notes.txt",
+                diff="@@\n alpha\n-beta\n+gamma\n",
+                move_to="docs",
+            )
+        )
+
+    assert session.files[source] == b"alpha\nbeta\n"
+    assert not [path for path in session.files if path.name.endswith(".tmp")]
+
+
+@pytest.mark.asyncio
+async def test_apply_patch_move_to_commits_the_destination_before_removing_the_source() -> None:
+    """The order is the fix. Assert it directly, so a future reordering fails here."""
+    session = PosixHostApplyPatchSession()
+    session.files[cast(Path, PurePosixPath("/workspace/notes.txt"))] = b"alpha\nbeta\n"
+
+    await session.apply_patch(
+        ApplyPatchOperation(
+            type="update_file",
+            path="notes.txt",
+            diff="@@\n alpha\n-beta\n+gamma\n",
+            move_to="Notes.txt",
+        )
+    )
+
+    assert len(session.mv_calls) == 1
+    staging, moved_to = session.mv_calls[0]
+    assert staging.parent == PurePosixPath("/workspace")
+    assert staging.name.endswith(".tmp")
+    assert moved_to == PurePosixPath("/workspace/Notes.txt")
+    assert session.rm_calls == [(cast(Path, PurePosixPath("/workspace/notes.txt")), False)]
 
 
 @pytest.mark.asyncio
diff --git a/tests/sandbox/test_unix_local.py b/tests/sandbox/test_unix_local.py
index fb13be3c51..ee5f356fe0 100644
--- a/tests/sandbox/test_unix_local.py
+++ b/tests/sandbox/test_unix_local.py
@@ -12,8 +12,9 @@
 
 import pytest
 
+from agents.editor import ApplyPatchOperation
 from agents.sandbox import SandboxPathGrant
-from agents.sandbox.errors import PtySessionNotFoundError
+from agents.sandbox.errors import ExecNonZeroError, PtySessionNotFoundError
 from agents.sandbox.manifest import Environment, Manifest
 from agents.sandbox.sandboxes import unix_local as unix_local_module
 from agents.sandbox.sandboxes.unix_local import (
@@ -513,3 +514,84 @@ def _slow_extract(tar: object, **kwargs: object) -> None:
     # the workspace root are only released once nothing is still writing to them.
     assert events == ["extract-start", "extract-end"]
     assert not buf.closed
+
+
+class TestUnixLocalApplyPatchRename:
+    """apply_patch renames against a real filesystem, not a model of one.
+
+    Every other test of this behaviour drives a session double. A double can only be wrong in
+    the same direction as the code it was written beside. The default macOS volume folds case,
+    so on the macOS runner these exercise the case that loses the file.
+    """
+
+    @pytest.mark.asyncio
+    @pytest.mark.requires_native_macos_sandbox
+    async def test_case_only_move_to_keeps_the_file(self, tmp_path: Path) -> None:
+        workspace = tmp_path / "workspace"
+        client = UnixLocalSandboxClient()
+        manifest = Manifest(root=str(workspace))
+
+        async with await client.create(manifest=manifest, snapshot=None, options=None) as session:
+            await session.write(Path("notes.txt"), io.BytesIO(b"alpha\nbeta\n"))
+
+            source = workspace / "notes.txt"
+            destination = workspace / "Notes.txt"
+            if not await session.same_file(source, destination):
+                pytest.skip("this volume does not fold case, so it cannot exercise the bug")
+
+            await session.apply_patch(
+                ApplyPatchOperation(
+                    type="update_file",
+                    path="notes.txt",
+                    diff="@@\n alpha\n-beta\n+gamma\n",
+                    move_to="Notes.txt",
+                )
+            )
+
+            names = sorted(entry.name for entry in workspace.iterdir())
+            assert names == ["Notes.txt"]
+            assert destination.read_bytes() == b"alpha\ngamma\n"
+
+    @pytest.mark.asyncio
+    @pytest.mark.requires_native_macos_sandbox
+    async def test_move_to_an_existing_directory_keeps_the_source(self, tmp_path: Path) -> None:
+        """The guard against a directory destination is a shell test, so run a real shell.
+
+        `mv` given an existing directory moves the source inside it and exits 0. A caller that
+        removes the source on that exit code deletes the file. The unit tests model this; this
+        one runs it.
+        """
+        workspace = tmp_path / "workspace"
+        client = UnixLocalSandboxClient()
+        manifest = Manifest(root=str(workspace))
+
+        async with await client.create(manifest=manifest, snapshot=None, options=None) as session:
+            await session.write(Path("notes.txt"), io.BytesIO(b"alpha\nbeta\n"))
+            await session.mkdir(Path("docs"))
+
+            with pytest.raises(ExecNonZeroError):
+                await session.apply_patch(
+                    ApplyPatchOperation(
+                        type="update_file",
+                        path="notes.txt",
+                        diff="@@\n alpha\n-beta\n+gamma\n",
+                        move_to="docs",
+                    )
+                )
+
+            assert (workspace / "notes.txt").read_bytes() == b"alpha\nbeta\n"
+            assert sorted(entry.name for entry in (workspace / "docs").iterdir()) == []
+
+    @pytest.mark.asyncio
+    @pytest.mark.requires_native_macos_sandbox
+    async def test_same_file_answers_for_real_paths(self, tmp_path: Path) -> None:
+        workspace = tmp_path / "workspace"
+        client = UnixLocalSandboxClient()
+        manifest = Manifest(root=str(workspace))
+
+        async with await client.create(manifest=manifest, snapshot=None, options=None) as session:
+            await session.write(Path("one.txt"), io.BytesIO(b"one\n"))
+            await session.write(Path("two.txt"), io.BytesIO(b"two\n"))
+
+            assert await session.same_file(workspace / "one.txt", workspace / "one.txt") is True
+            assert await session.same_file(workspace / "one.txt", workspace / "two.txt") is False

From bcd1d9f6c73f0a1f2801fe8277d18d0c4ca0ab22 Mon Sep 17 00:00:00 2001
From: wolfgang-aura <169568318+wolfgang-aura@users.noreply.github.com>
Date: Tue, 8 Sep 2026 02:51:31 +0800
Subject: [PATCH 3/4] fix: answer identity without following symlinks, and
 stage safely

Acts on five of the six findings the Codex reviewer raised on 5b3650ca.

`test -ef` follows symlinks, so a source symlink pointing at the destination
answered "same file" and the removal was skipped, leaving the old name pointing
at the new one. `same_file` takes `follow_symlinks` now, and the editor asks
with it off, because the answer decides whether removing one path destroys the
other.

The staging write moves inside the `try`, so a write that fails after creating
the file no longer orphans it. The staging basename is a fixed length, because
decorating a destination basename near the 255-byte limit overflowed it. A
`move_to` naming the path the file already has short-circuits to an in-place
write, which is what an update without `move_to` does and what this code did
before the rename was staged.

`docs/testing.md` is reverted: AGENTS.md keeps documentation for unreleased
behaviour out of the pull request that introduces it.

The directory check is still not atomic with the rename. Closing that needs a
per-backend rename primitive, which is a question for the maintainer.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 docs/testing.md                               |   2 +-
 src/agents/sandbox/apply_patch.py             |  27 ++++-
 .../sandbox/session/base_sandbox_session.py   |  13 ++-
 src/agents/sandbox/session/sandbox_session.py |   3 +-
 src/agents/testing/sandbox.py                 |  10 +-
 tests/sandbox/_apply_patch_test_session.py    |  22 +++-
 tests/sandbox/test_apply_patch.py             | 102 +++++++++++++++++-
 tests/sandbox/test_unix_local.py              |  25 +++++
 8 files changed, 192 insertions(+), 12 deletions(-)

diff --git a/docs/testing.md b/docs/testing.md
index d7cab034af..57e5064615 100644
--- a/docs/testing.md
+++ b/docs/testing.md
@@ -286,7 +286,7 @@ Each matching Sandbox call consumes the next step in one global FIFO sequence. A
 | `error` | The method should raise a specific exception |
 | `match` | The call should be rejected before producing its outcome unless the matcher returns a value other than `False` |
 
-The supported scripted method names are `apply_patch`, `exec`, `ls`, `mkdir`, `mv`, `pty_exec_start`, `pty_write_stdin`, `read`, `rm`, `same_file`, and `write`. Only configured model-facing capabilities are exposed. The two PTY methods are exposed together when either PTY method is configured because they form one interactive-shell capability, but calls still consume the global FIFO script.
+The supported scripted method names are `apply_patch`, `exec`, `ls`, `mkdir`, `pty_exec_start`, `pty_write_stdin`, `read`, `rm`, and `write`. Only configured model-facing capabilities are exposed. The two PTY methods are exposed together when either PTY method is configured because they form one interactive-shell capability, but calls still consume the global FIFO script.
 
 `sandbox.calls` contains detached `SandboxCall` snapshots with zero-based `call_index`, `method`, positional `args`, and read-only `kwargs`. Static results are also snapshotted when the script is created. `io.BytesIO` and `io.StringIO` values are supported; use a custom Sandbox session for other live stream objects or lifecycle behavior.
 
diff --git a/src/agents/sandbox/apply_patch.py b/src/agents/sandbox/apply_patch.py
index a6ade970fb..0d8c5d2253 100644
--- a/src/agents/sandbox/apply_patch.py
+++ b/src/agents/sandbox/apply_patch.py
@@ -228,20 +228,39 @@ async def _move_updated_text(
 
         So neither path is written or removed until the new content is committed somewhere else:
         the text goes to a staging file, a single `mv` puts it at the destination, and only then
-        is the source removed, and only if the filesystem says it is a different file. Before
+        is the source removed, and only if the filesystem says it is a different entry. Before
         that `mv` the original is untouched; after it the new content exists. There is no moment
         where the only copy is in memory, and nothing is restored after the fact, so a file that
         another writer creates at the source path while this runs is never overwritten.
+
+        The staging file is a new inode, so a rename the filesystem folds onto the source path
+        replaces the original's mode and extended attributes. Carrying those across would mean
+        reading and reapplying them per backend; committing the content in a single `mv` is
+        worth more than the mode bits.
+
+        The staging name is a fixed length rather than a decoration of the destination name,
+        because a destination basename near the filesystem's 255-byte limit would make the
+        decorated name exceed it and the write would fail with ENAMETOOLONG.
         """
-        staging = moved_destination.with_name(f".{moved_destination.name}.{uuid4().hex[:8]}.tmp")
-        await self._write_text(staging, text)
+        if source == moved_destination:
+            # Not a rename, so nothing needs committing elsewhere. Writing in place is what an
+            # update without `move_to` does, and it keeps the inode, the mode and the xattrs.
+            await self._write_text(source, text)
+            return
+
+        staging = moved_destination.with_name(f".apply_patch-{uuid4().hex}.tmp")
         try:
+            await self._write_text(staging, text)
             await self._session.mv(staging, moved_destination, user=self._user)
         except BaseException:
             with contextlib.suppress(Exception):
                 await self._session.rm(staging, user=self._user)
             raise
-        if not await self._session.same_file(source, moved_destination, user=self._user):
+        # A symlink is its own directory entry: removing it leaves the file it points at, so
+        # the source still has to go. `-ef` follows symlinks, so ask without following.
+        if not await self._session.same_file(
+            source, moved_destination, follow_symlinks=False, user=self._user
+        ):
             await self._session.rm(source, user=self._user)
 
     async def _write_text(self, destination: Path, text: str) -> None:
diff --git a/src/agents/sandbox/session/base_sandbox_session.py b/src/agents/sandbox/session/base_sandbox_session.py
index 19b16ce79f..d0aa5e63c9 100644
--- a/src/agents/sandbox/session/base_sandbox_session.py
+++ b/src/agents/sandbox/session/base_sandbox_session.py
@@ -1194,6 +1194,7 @@ async def same_file(
         left: Path | str,
         right: Path | str,
         *,
+        follow_symlinks: bool = True,
         user: str | User | None = None,
     ) -> bool:
         """Return whether two paths name the same file on the sandbox filesystem.
@@ -1205,8 +1206,15 @@ async def same_file(
         No string comparison can answer this, and neither can the host that is driving the
         session, which may not be the kind of system the sandbox is running on.
 
+        `test -ef` resolves symlinks, so a symlink and the file it points at are the same
+        file by this test while being two directory entries: removing the symlink leaves the
+        file alone. Pass ``follow_symlinks=False`` when the answer is going to decide whether
+        removing one path destroys the other, which makes a symlink on either side answer no.
+
         :param left: First path to compare.
         :param right: Second path to compare.
+        :param follow_symlinks: If false, a symlink on either side is not the same file as
+                its target.
         :param user: Optional sandbox user to compare as.
         :returns: True when both paths resolve to the same file.
         """
@@ -1215,7 +1223,10 @@ async def same_file(
 
         left_arg = sandbox_path_str(left)
         right_arg = sandbox_path_str(right)
-        cmd = ("sh", "-lc", '[ "$1" -ef "$2" ]', "sh", left_arg, right_arg)
+        test = '[ "$1" -ef "$2" ]'
+        if not follow_symlinks:
+            test = '[ ! -L "$1" ] && [ ! -L "$2" ] && ' + test
+        cmd = ("sh", "-lc", test, "sh", left_arg, right_arg)
         result = await self.exec(*cmd, shell=False, user=user)
         if result.exit_code == 0:
             return True
diff --git a/src/agents/sandbox/session/sandbox_session.py b/src/agents/sandbox/session/sandbox_session.py
index 5d78fa67cc..c6d5a1df6b 100644
--- a/src/agents/sandbox/session/sandbox_session.py
+++ b/src/agents/sandbox/session/sandbox_session.py
@@ -655,9 +655,10 @@ async def same_file(
         left: Path | str,
         right: Path | str,
         *,
+        follow_symlinks: bool = True,
         user: str | User | None = None,
     ) -> bool:
-        return await self._inner.same_file(left, right, user=user)
+        return await self._inner.same_file(left, right, follow_symlinks=follow_symlinks, user=user)
 
     async def mkdir(
         self,
diff --git a/src/agents/testing/sandbox.py b/src/agents/testing/sandbox.py
index dfd674181c..919f0c88ce 100644
--- a/src/agents/testing/sandbox.py
+++ b/src/agents/testing/sandbox.py
@@ -505,9 +505,17 @@ async def same_file(
         left: Path | str,
         right: Path | str,
         *,
+        follow_symlinks: bool = True,
         user: str | User | None = None,
     ) -> bool:
-        return cast(bool, await self._invoke("same_file", (left, right), {"user": user}))
+        return cast(
+            bool,
+            await self._invoke(
+                "same_file",
+                (left, right),
+                {"follow_symlinks": follow_symlinks, "user": user},
+            ),
+        )
 
     async def apply_patch(
         self,
diff --git a/tests/sandbox/_apply_patch_test_session.py b/tests/sandbox/_apply_patch_test_session.py
index 9ddf096f1a..6e660b716f 100644
--- a/tests/sandbox/_apply_patch_test_session.py
+++ b/tests/sandbox/_apply_patch_test_session.py
@@ -24,6 +24,8 @@ def __init__(self, manifest: Manifest | None = None) -> None:
         self.mkdir_calls: list[tuple[Path, bool]] = []
         self.rm_calls: list[tuple[Path, bool]] = []
         self.mv_calls: list[tuple[Path, Path]] = []
+        # Link path -> target path, for the paths a test declares to be symlinks.
+        self.symlinks: dict[Path, Path] = {}
         self.directories: set[Path] = set()
 
     def _stored_path(self, path: Path | str) -> Path:
@@ -133,10 +135,25 @@ async def same_file(
         left: Path | str,
         right: Path | str,
         *,
+        follow_symlinks: bool = True,
         user: str | User | None = None,
     ) -> bool:
         _ = user
-        return self._stored_path(left) == self._stored_path(right)
+        if not follow_symlinks and (
+            self._stored_path(left) in self.symlinks or self._stored_path(right) in self.symlinks
+        ):
+            return False
+        return self._resolved_path(left) == self._resolved_path(right)
+
+    def _resolved_path(self, path: Path | str) -> Path:
+        # `-ef` resolves the whole chain, so a fake that follows one hop would answer False
+        # where a real filesystem answers True. The seen set stops a cycle.
+        stored = self._stored_path(path)
+        seen: set[Path] = set()
+        while stored in self.symlinks and stored not in seen:
+            seen.add(stored)
+            stored = self._stored_path(self.symlinks[stored])
+        return stored
 
 
 class PosixHostApplyPatchSession(ApplyPatchSession):
@@ -351,7 +368,8 @@ async def same_file(
         left: Path | str,
         right: Path | str,
         *,
+        follow_symlinks: bool = True,
         user: str | User | None = None,
     ) -> bool:
         self.same_file_users.append(self._user_name(user))
-        return await super().same_file(left, right)
+        return await super().same_file(left, right, follow_symlinks=follow_symlinks)
diff --git a/tests/sandbox/test_apply_patch.py b/tests/sandbox/test_apply_patch.py
index 886515163a..c589a6baa4 100644
--- a/tests/sandbox/test_apply_patch.py
+++ b/tests/sandbox/test_apply_patch.py
@@ -2,6 +2,7 @@
 
 from pathlib import Path, PurePosixPath
 from typing import cast
+from unittest.mock import AsyncMock, MagicMock
 
 import pytest
 
@@ -13,6 +14,7 @@
     ApplyPatchFileNotFoundError,
     ApplyPatchPathError,
 )
+from agents.sandbox.session.sandbox_session import SandboxSession
 from tests.sandbox._apply_patch_test_session import (
     ApplyPatchSession,
     CaseFoldingApplyPatchSession,
@@ -307,8 +309,10 @@ async def test_apply_patch_move_to_leaves_the_source_alone_when_the_write_fails(
 
     assert session.files == {PurePosixPath("/workspace/notes.txt"): b"alpha\nbeta\n"}
     # The file surviving is not enough. The refused implementation removed the source and then
-    # wrote it back, which also ends here. Nothing may be removed at all.
-    assert session.rm_calls == []
+    # wrote it back, which also ends here. The source may not be removed at all, and the only
+    # path this is allowed to remove is the staging file it was in the middle of writing.
+    assert PurePosixPath("/workspace/notes.txt") not in [path for path, _ in session.rm_calls]
+    assert all(path.name.startswith(".apply_patch-") for path, _ in session.rm_calls)
 
 
 @pytest.mark.asyncio
@@ -412,6 +416,100 @@ async def test_apply_patch_move_to_commits_the_destination_before_removing_the_s
     assert session.rm_calls == [(cast(Path, PurePosixPath("/workspace/notes.txt")), False)]
 
 
+@pytest.mark.asyncio
+async def test_apply_patch_move_to_removes_a_source_symlink_pointing_at_the_destination() -> None:
+    """`test -ef` follows symlinks, and the removal decision must not.
+
+    A symlink and its target are one file by device and inode, and two directory entries.
+    Removing the symlink leaves the target alone, so a rename that reads them as the same file
+    leaves the old name behind pointing at the new one.
+    """
+    session = PosixHostApplyPatchSession()
+    link = cast(Path, PurePosixPath("/workspace/notes.txt"))
+    target = cast(Path, PurePosixPath("/workspace/Notes.txt"))
+    session.files[link] = b"alpha\nbeta\n"
+    session.files[target] = b"alpha\nbeta\n"
+    session.symlinks[link] = target
+
+    await session.apply_patch(
+        ApplyPatchOperation(
+            type="update_file",
+            path="notes.txt",
+            diff="@@\n alpha\n-beta\n+gamma\n",
+            move_to="Notes.txt",
+        )
+    )
+
+    assert session.files == {target: b"alpha\ngamma\n"}
+
+
+@pytest.mark.asyncio
+async def test_sandbox_session_forwards_follow_symlinks_to_the_inner_session() -> None:
+    """The editor always talks to the instrumented wrapper, never to the session underneath.
+
+    `BaseSandboxSession.apply_patch` builds the editor around `self`, and every client hands
+    out a `SandboxSession`. A wrapper that accepts `follow_symlinks` and drops it leaves the
+    inner session running the plain `-ef` test, and every test above uses a session double that
+    never crosses the wrapper, so nothing else here would notice.
+    """
+    inner = MagicMock()
+    inner.same_file = AsyncMock(return_value=True)
+    session = SandboxSession(inner)
+
+    await session.same_file("/workspace/link.txt", "/workspace/target.txt", follow_symlinks=False)
+
+    # .get, not [], so a wrapper that drops the argument fails on the value rather than
+    # raising KeyError from the assertion itself.
+    assert inner.same_file.await_args.kwargs.get("follow_symlinks") is False
+
+
+@pytest.mark.asyncio
+async def test_apply_patch_move_to_the_same_path_writes_in_place() -> None:
+    """A `move_to` that names the path it already has is an update, not a rename.
+
+    Committing it through a staging file would replace the inode, and with it the mode and the
+    extended attributes, for an operation that moves nothing.
+    """
+    session = PosixHostApplyPatchSession()
+    source = cast(Path, PurePosixPath("/workspace/notes.txt"))
+    session.files[source] = b"alpha\nbeta\n"
+
+    await session.apply_patch(
+        ApplyPatchOperation(
+            type="update_file",
+            path="notes.txt",
+            diff="@@\n alpha\n-beta\n+gamma\n",
+            move_to="notes.txt",
+        )
+    )
+
+    assert session.files == {source: b"alpha\ngamma\n"}
+    assert session.mv_calls == []
+    assert session.rm_calls == []
+
+
+@pytest.mark.asyncio
+async def test_apply_patch_move_to_a_long_name_keeps_the_staging_name_within_the_limit() -> None:
+    """A staging name built from the destination name overflows the 255-byte basename limit."""
+    session = PosixHostApplyPatchSession()
+    session.files[cast(Path, PurePosixPath("/workspace/notes.txt"))] = b"alpha\nbeta\n"
+    long_name = "n" * 250 + ".txt"
+
+    await session.apply_patch(
+        ApplyPatchOperation(
+            type="update_file",
+            path="notes.txt",
+            diff="@@\n alpha\n-beta\n+gamma\n",
+            move_to=long_name,
+        )
+    )
+
+    assert len(session.mv_calls) == 1
+    staging, _ = session.mv_calls[0]
+    assert len(staging.name.encode("utf-8")) <= 255
+    assert session.files == {PurePosixPath(f"/workspace/{long_name}"): b"alpha\ngamma\n"}
+
+
 @pytest.mark.asyncio
 async def test_apply_patch_allows_absolute_path_within_root() -> None:
     session = ApplyPatchSession()
diff --git a/tests/sandbox/test_unix_local.py b/tests/sandbox/test_unix_local.py
index ee5f356fe0..9451777395 100644
--- a/tests/sandbox/test_unix_local.py
+++ b/tests/sandbox/test_unix_local.py
@@ -595,3 +595,28 @@ async def test_same_file_answers_for_real_paths(self, tmp_path: Path) -> None:
 
             assert await session.same_file(workspace / "one.txt", workspace / "one.txt") is True
             assert await session.same_file(workspace / "one.txt", workspace / "two.txt") is False
+
+    @pytest.mark.asyncio
+    @pytest.mark.requires_native_macos_sandbox
+    async def test_same_file_does_not_follow_symlinks_when_asked_not_to(
+        self, tmp_path: Path
+    ) -> None:
+        """A symlink and its target are one file and two directory entries.
+
+        `test -ef` follows the link, so it calls them the same file. A caller deciding whether
+        removing one destroys the other needs the other answer, and this is shell code, so run
+        a real shell against a real symlink.
+        """
+        workspace = tmp_path / "workspace"
+        client = UnixLocalSandboxClient()
+        manifest = Manifest(root=str(workspace))
+
+        async with await client.create(manifest=manifest, snapshot=None, options=None) as session:
+            await session.write(Path("target.txt"), io.BytesIO(b"alpha\n"))
+            (workspace / "link.txt").symlink_to(workspace / "target.txt")
+
+            link = workspace / "link.txt"
+            target = workspace / "target.txt"
+            assert await session.same_file(link, target) is True
+            assert await session.same_file(link, target, follow_symlinks=False) is False
+            assert await session.same_file(target, target, follow_symlinks=False) is True

From 0b7e082cc71bb2cb21e9095a131840cb94b1be85 Mon Sep 17 00:00:00 2001
From: wolfgang-aura <169568318+wolfgang-aura@users.noreply.github.com>
Date: Tue, 8 Sep 2026 03:48:51 +0800
Subject: [PATCH 4/4] docs: say what the removal does not guarantee

The docstring said a file another writer creates at the source path is never
overwritten. That is true and was standing in for a guarantee it does not make:
the identity answer is read before the removal, and the removal names a path
rather than the entry that answer was about, so such a file can still be
removed.

No behaviour change. Closing the window needs a removal that can be told which
entry it may remove, which is the per-backend question already open with the
maintainer.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 src/agents/sandbox/apply_patch.py | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/agents/sandbox/apply_patch.py b/src/agents/sandbox/apply_patch.py
index 0d8c5d2253..3d2ddc72bb 100644
--- a/src/agents/sandbox/apply_patch.py
+++ b/src/agents/sandbox/apply_patch.py
@@ -233,6 +233,11 @@ async def _move_updated_text(
         where the only copy is in memory, and nothing is restored after the fact, so a file that
         another writer creates at the source path while this runs is never overwritten.
 
+        It can still be removed. The identity answer is read before the removal, and the removal
+        names a path rather than the entry that answer was about, so a writer that replaces the
+        source between the two loses the file it just wrote. Closing that needs a removal that
+        can be told which entry it is allowed to remove, which no backend here offers.
+
         The staging file is a new inode, so a rename the filesystem folds onto the source path
         replaces the original's mode and extended attributes. Carrying those across would mean
         reading and reapplying them per backend; committing the content in a single `mv` is