openai / openai/openai-agents-python

[Sandbox] apply_patch update_file with a case-only move_to deletes the file on a case-insensitive filesystem

Open
#4,889 4 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

feature:sandboxes
Dominant language
Python
Stars
29.6k
Forks
4.8k
Avg merge
1d 20h
Merged PRs (30d)
123

Description

Please read this first
  • Have you read the docs? Yes.
  • Have you searched for related issues? Yes. #4099 is the same two lines and a different defect, and is closed. Nothing open or closed covers a rename that changes only capitalisation.
Describe the bug

WorkspaceEditor.apply_operation handles update_file with a move_to by writing the updated text to the destination and then removing the source:

https://github.com/openai/openai-agents-python/blob/main/src/agents/sandbox/apply_patch.py#L112-L116

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:
    await self._session.rm(destination, user=self._user)

The guard compares two paths. It cannot tell whether they name the same file. When the sandbox filesystem folds case, notes.txt and Notes.txt are one file, the comparison still reports them as different, and the rm deletes what the write just produced. The operation reports Updated notes.txt and Moved notes.txt to Notes.txt, and the file is gone with the edit inside it.

Two conditions have to hold together, and both are ordinary:

  1. The host running the SDK compares paths case-sensitively. normalize_path returns a host-native Path, so this is every Linux and macOS host.
  2. The sandbox filesystem folds case. APFS on macOS is case-insensitive by default, as is NTFS, as is a Docker bind mount backed by either.

A macOS laptop running UnixLocalSandboxSession is both at once, and unix_local.py treats Darwin as a first-class platform.

Renaming a file to fix its capitalisation is a normal thing to ask an agent to do, which is what makes this worth reporting rather than a curiosity. There is no error, no warning, and nothing in the tool output that says the file was destroyed.

Debug information
  • Agents SDK version: main at 1d471a4, and v0.22.0
  • Related library versions: not relevant, the path never reaches a provider
  • Python version: 3.13.15
  • Operating system: reproduced on Windows using the script below, which models the two conditions above. Not run on macOS. See the note under the repro steps.
  • Model and model provider: none, the defect is below the model
  • Does the issue reproduce with the latest Agents SDK release? Yes, the code is identical in v0.22.0.
  • Does the issue occur consistently or intermittently? Consistently, whenever both conditions hold.
Traceback (most recent call last):
  File "repro_case_rename.py", line 61, in main
    assert session.files, "the edit and the file were both destroyed by the rename"
AssertionError: the edit and the file were both destroyed by the rename
Repro steps

Save this at the root of a repository checkout and run it. It needs no sandbox provider and no network.

"""apply_patch update_file with a case-only move_to deletes the file."""

import asyncio
import io
from pathlib import PurePosixPath

from agents.editor import ApplyPatchOperation
from tests.sandbox._apply_patch_test_session import ApplyPatchSession


class CaseFoldingSession(ApplyPatchSession):
    """A host that compares paths case-sensitively, over a filesystem that folds case."""

    def normalize_path(self, path, **kwargs):
        return PurePosixPath(str(super().normalize_path(path, **kwargs)).replace("\\", "/"))

    def _key(self, path):
        return str(self.normalize_path(path)).lower()

    async def read(self, path, *, user=None):
        key = self._key(path)
        if key not in self.files:
            raise FileNotFoundError(key)
        return io.BytesIO(self.files[key])

    async def write(self, path, data, *, user=None):
        payload = data.read()
        self.files[self._key(path)] = (
            payload.encode("utf-8") if isinstance(payload, str) else bytes(payload)
        )

    async def rm(self, path, *, recursive=False, user=None):
        self.files.pop(self._key(path), None)

    async def mkdir(self, path, *, parents=False, user=None):
        return None


async def main() -> None:
    session = CaseFoldingSession()
    session.files = {"/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",
        )
    )

    print("files after the rename:", session.files)
    assert session.files, "the edit and the file were both destroyed by the rename"


asyncio.run(main())

The two overrides are the whole of the model. normalize_path returns a PurePosixPath because the machine I have is Windows, where Path comparison folds case and the guard holds by accident. The _key lowercasing is the case-insensitive filesystem. On macOS both come for free and the plain ApplyPatchSession would show it, but I cannot run that here and would rather say so than imply I did.

The same shape reaches the real sandbox through the apply_patch tool, from an operation of the form {"type": "update_file", "path": "notes.txt", "move_to": "Notes.txt", "diff": "..."}.

Expected behavior

The file survives the rename and holds the updated text.

Whichever way you want it fixed, the source removal needs to know it is not pointing at the file that was just written. Asking the session whether the two paths resolve to the same file is the honest version, since case folding is a property of the sandbox filesystem and not of the machine running the SDK. Comparing the case-folded strings is cheaper and would also refuse a legitimate case-only rename on a case-sensitive filesystem, so it trades one wrong answer for another.

There is a related question you may want to settle at the same time. A move_to that names an existing different file overwrites it with no check, on any filesystem. I have not filed that separately because the fix probably lives in the same few lines.

I am happy to open a pull request with the fix and a regression test that pins the case-folding session, if you would like it.

Reported by Claude Opus 5 running under my supervision. I read this before posting it. The reproduction was executed and its output is quoted above; the macOS behaviour is reasoned from the code and the platform, not observed.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/agents/sandbox/apply_patch.py around WorkspaceEditor.apply_operation and inspect the update_file move_to path handling. Use tests/sandbox/_apply_patch_test_session.py and the CaseFoldingSession reproduction as the starting test setup. Done means a case-only rename preserves the file and its updated text, with a regression test covering that behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
operating-systems, testing-qa, tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.