diff --git a/src/agent_tools/filesystem_tools.py b/src/agent_tools/filesystem_tools.py index eda371420..92dc42687 100644 --- a/src/agent_tools/filesystem_tools.py +++ b/src/agent_tools/filesystem_tools.py @@ -3,6 +3,7 @@ import json import os import re import difflib +import secrets import shutil import time from typing import Optional, Dict, Any, Tuple, List @@ -289,6 +290,37 @@ class ReadFileTool: data = data[:MAX_READ_CHARS] + f"\n... [truncated at {MAX_READ_CHARS} chars]" return {"output": data, "exit_code": 0} + +def _write_new_file_without_overwrite(path: str, body: str) -> None: + """Publish a new file without exposing a writable placeholder at its path. + + Stage beside the destination, then hard-link it into place. The link is + atomic and fails if another writer created the destination first. + """ + directory = os.path.dirname(path) or "." + temporary_path = os.path.join( + directory, f".odysseus-write-{secrets.token_hex(16)}.tmp" + ) + fd = os.open( + temporary_path, + os.O_WRONLY | os.O_CREAT | os.O_EXCL, + 0o666, + ) + + try: + with os.fdopen(fd, "w", encoding="utf-8") as temporary_file: + fd = None + temporary_file.write(body) + os.link(temporary_path, path) + finally: + if fd is not None: + os.close(fd) + try: + os.unlink(temporary_path) + except FileNotFoundError: + pass + + class _EmptyBodyWouldTruncate(Exception): """Raised inside the write thread when an undeclared empty body is about to replace a file that holds bytes. Carries the size at risk so the caller can be @@ -358,11 +390,17 @@ class WriteFileTool: raise _EmptyBodyWouldTruncate(path, existing_bytes) return old, 0 - # Create a missing target exclusively. If another writer wins - # the race, inspect what appeared rather than truncating it. try: - with open(path, "x", encoding="utf-8") as f: - f.write(body) + if body: + # Publish whitespace content atomically. Writing it + # after exclusive creation could overwrite bytes from + # a writer that filled the new placeholder meanwhile. + _write_new_file_without_overwrite(path, body) + else: + # An exact empty body needs no staged data, so create + # the file exclusively and never write through it. + with open(path, "x", encoding="utf-8"): + pass except FileExistsError: if os.path.isfile(path): existing_bytes = os.path.getsize(path) diff --git a/tests/test_write_file_empty_body.py b/tests/test_write_file_empty_body.py index 55dc35163..9b735f15d 100644 --- a/tests/test_write_file_empty_body.py +++ b/tests/test_write_file_empty_body.py @@ -7,6 +7,7 @@ existing file is opened in "w" mode, and the tool answers exit_code=0 with path are still there afterwards; each "still works" test guards the write path this change must not narrow. """ +import builtins import json import os import re @@ -248,6 +249,36 @@ async def test_empty_body_does_not_truncate_a_file_created_after_the_absence_che assert _read(target) == "concurrent update" +@pytest.mark.asyncio +async def test_whitespace_body_does_not_clobber_a_concurrent_creation( + target, monkeypatch +): + """Whitespace on a missing path must not overwrite a competing writer.""" + real_open = builtins.open + real_link = os.link + + def write_concurrent_content(path): + with real_open(path, "w", encoding="utf-8") as concurrent: + concurrent.write("concurrent update") + + def interleaved_open(path, mode="r", *args, **kwargs): + handle = real_open(path, mode, *args, **kwargs) + if path == target and mode == "x": + write_concurrent_content(path) + return handle + + def interleaved_link(source, destination, *args, **kwargs): + if destination == target: + write_concurrent_content(destination) + return real_link(source, destination, *args, **kwargs) + + monkeypatch.setattr(builtins, "open", interleaved_open) + monkeypatch.setattr(os, "link", interleaved_link) + res = await WriteFileTool().execute(_text_call(target, " \n\t"), {}) + assert res["exit_code"] == 1, res + assert _read(target) == "concurrent update" + + @pytest.mark.asyncio async def test_a_real_body_still_writes_and_reports_a_diff(target): _seed(target)