fix(file-ops): a failed byte-exact read is not an absent file

Three defects in the byte-exact read this PR introduced.

base64 is not on every backend (busybox, distroless). The sample path
already degrades when it is missing; this one returned the read as a
failure, and read_file_raw is also _apply_add's existence check, which
treated any error as "the path is free". A backend with cat but no base64
turned `*** Add File` over an existing file into a silent overwrite that
reported success:

    main:     refused, "file already exists — use Update File"
    PR head:  b'KEEP ME -- months of work\n' -> b'clobbered by Add'

So: fall back to od (POSIX, in busybox) when base64 exits 127, and when
neither exists report a transport error. ReadResult grows not_found, set
only where the path is genuinely absent, and _apply_add refuses unless it
sees that flag — a read that FAILED can no longer pass as an absent path.
getattr keeps a producer without the field failing closed rather than
raising. The doubles in test_patch_parser that meant "absent" now say so.

The native fast path stat'd the path and then opened it, two lookups on a
name. A swap to a FIFO in between blocks the thread, and nothing times out
that. One O_RDONLY|O_NONBLOCK open, fstat on THAT descriptor, then read, so
a non-regular file is handed to the shell path and its timeout instead.

Tests: the od fallback round-trips byte-exactly and patches; an Add over a
file the backend cannot read is refused with the file intact; the native
read hands a FIFO to the shell rather than opening it. The first two go red
if their fix is removed. The third pins the property, not the race — with
one descriptor there is no window left to swap into, so reverting to
stat-then-open does not flip it.

Not addressed here: inserted text is still encoded UTF-8 regardless of the
file's declared encoding, so an edit adding non-ASCII to a latin-1 file
writes mixed bytes. That is the write half, it predates this PR, and it
needs source-encoding detection.

tests/tools/test_file_write_safety.py

@@ -438,6 +438,78 @@ class TestBomHandling:
         ops.patch_replace(str(target), "VERSION=1", "VERSION=2")
         assert target.read_bytes() == original.replace(b"VERSION=1", b"VERSION=2")
 
+    @staticmethod
+    def _env_without(*missing: str):
+        """A real shell where only the named BINARIES are absent (busybox, distroless)."""
+        import re as _re
+        from tools.environments.local import LocalEnvironment
+        stub = "( echo 'sh: not found' >&2; exit 127 )"  # a SUBSHELL: `exit` must not kill the shell
+
+        class Env(LocalEnvironment):
+            def execute(self, command, *args, **kwargs):
+                for name in missing:
+                    command = _re.sub(rf"\b{name} <", f"{stub} <", command)
+                    command = _re.sub(rf"\b{name}\b(?! <)", stub, command)
+                return super().execute(command, *args, **kwargs)
+        return Env
+
+    def test_byte_exact_read_falls_back_to_hex_without_base64(self, tmp_path: Path, monkeypatch):
+        # base64 is not on every backend. The sample path already degrades when it is missing
+        # (_detect_binary), so the byte-exact read must too, and byte-exactly.
+        from tools.file_operations import ShellFileOperations
+        monkeypatch.setenv("HERMES_NATIVE_FILE_READ", "0")
+        target = tmp_path / "conf.txt"
+        original = b"HEADER\nVERSION=1\n"
+        target.write_bytes(original)
+        ops = ShellFileOperations(self._env_without("base64")(cwd=str(tmp_path)), cwd=str(tmp_path))
+
+        assert ops._read_exact_bytes(str(target)) == (original, None)
+        assert ops.patch_replace(str(target), "VERSION=1", "VERSION=2").success
+        assert target.read_bytes() == original.replace(b"VERSION=1", b"VERSION=2")
+
+    def test_add_file_refuses_when_the_read_failed_rather_than_the_path_being_free(
+            self, tmp_path: Path, monkeypatch):
+        # `Add File` uses read_file_raw's error as its existence check. A backend with no byte
+        # transport at all makes that read FAIL, which must not read as "the path is free" —
+        # that writes the Add payload over the file the check exists to protect.
+        from tools.file_operations import ShellFileOperations
+        monkeypatch.setenv("HERMES_NATIVE_FILE_READ", "0")
+        target = tmp_path / "KEEP.txt"
+        precious = b"KEEP ME\n"
+        target.write_bytes(precious)
+        ops = ShellFileOperations(self._env_without("base64", "od")(cwd=str(tmp_path)), cwd=str(tmp_path))
+
+        read = ops.read_file_raw(str(target))
+        assert read.error and not read.not_found  # a failed read, NOT an absent path
+        res = ops.patch_v4a(f"*** Begin Patch\n*** Add File: {target}\n+clobbered\n*** End Patch")
+        assert not res.success
+        assert target.read_bytes() == precious
+
+    def test_native_byte_exact_read_never_opens_a_non_regular_file(self, tmp_path: Path, monkeypatch):
+        # The native fast path bypasses the backend timeout, so a blocking open there hangs the
+        # thread with nothing to interrupt it. The shell path below has a timeout and is allowed
+        # to take a FIFO; the native path must hand it over instead of opening it. Stubbing the
+        # shell read keeps this about the native branch: if it opens the FIFO the test hangs.
+        import signal
+        from tools.file_operations import ExecuteResult, ShellFileOperations
+        from tools.environments.local import LocalEnvironment
+        fifo = tmp_path / "pipe"
+        os.mkfifo(fifo)  # no writer: a blocking open never returns
+        ops = ShellFileOperations(LocalEnvironment(cwd=str(tmp_path)), cwd=str(tmp_path))
+        monkeypatch.setattr(ops, "_exec",
+                            lambda *a, **k: ExecuteResult(stdout="handed to the shell", exit_code=1))
+
+        def _bail(*_args):
+            raise TimeoutError("the native read opened a FIFO and blocked")
+        previous = signal.signal(signal.SIGALRM, _bail)
+        signal.alarm(5)
+        try:
+            data, failed = ops._read_exact_bytes(str(fifo))
+        finally:
+            signal.alarm(0)
+            signal.signal(signal.SIGALRM, previous)
+        assert data is None and failed is not None and "handed to the shell" in failed.stdout
+
 
 class TestProtectedInstructionFiles:
     """Writes to agent-instruction files ALWAYS require approval.

tests/tools/test_patch_parser.py

@@ -400,7 +400,7 @@ class TestValidationPhase:
                 }
                 content = files.get(path)
                 if content is None:
-                    return SimpleNamespace(content=None, error=f"File not found: {path}")
+                    return SimpleNamespace(content=None, error=f"File not found: {path}", not_found=True)
                 return SimpleNamespace(content=content, error=None)
 
             def write_file(self, path, content, pre_content=None):
@@ -454,7 +454,7 @@ class TestValidationPhase:
             def read_file_raw(self, path):
                 if path == "exists.py":
                     return SimpleNamespace(content=original, error=None)
-                return SimpleNamespace(content=None, error=f"File not found: {path}")
+                return SimpleNamespace(content=None, error=f"File not found: {path}", not_found=True)
 
             def write_file(self, path, content):
                 written[path] = content
@@ -485,7 +485,7 @@ class TestValidationPhase:
             def read_file_raw(self, path):
                 if path in state:
                     return SimpleNamespace(content=state[path], error=None)
-                return SimpleNamespace(content=None, error=f"File not found: {path}")
+                return SimpleNamespace(content=None, error=f"File not found: {path}", not_found=True)
 
             def delete_file(self, path):
                 state.pop(path, None)
@@ -634,7 +634,7 @@ class TestV4ALspDiagnosticsPropagation:
 
         class FakeFileOps:
             def read_file_raw(self, path):
-                return SimpleNamespace(content=None, error=f"File not found: {path}")
+                return SimpleNamespace(content=None, error=f"File not found: {path}", not_found=True)
 
             def write_file(self, path, content, pre_content=None):
                 return SimpleNamespace(error=None, lsp_diagnostics=diag_block)
@@ -689,7 +689,7 @@ class TestV4ALspDiagnosticsPropagation:
 
         class FakeFileOps:
             def read_file_raw(self, path):
-                return SimpleNamespace(content=None, error=f"File not found: {path}")
+                return SimpleNamespace(content=None, error=f"File not found: {path}", not_found=True)
 
             def write_file(self, path, content, pre_content=None):
                 # lsp_diagnostics omitted entirely (older WriteResult shape).
@@ -725,7 +725,7 @@ class TestV4ALspDiagnosticsPropagation:
 
         class FakeFileOps:
             def read_file_raw(self, path):
-                return SimpleNamespace(content=None, error=f"File not found: {path}")
+                return SimpleNamespace(content=None, error=f"File not found: {path}", not_found=True)
 
             def write_file(self, path, content, pre_content=None):
                 return SimpleNamespace(error=None, lsp_diagnostics=per_file[path])
@@ -750,7 +750,7 @@ class _DictFileOps:
     def read_file_raw(self, path):
         if path in self.files:
             return SimpleNamespace(content=self.files[path], error=None)
-        return SimpleNamespace(content="", error="file not found")
+        return SimpleNamespace(content="", error="file not found", not_found=True)
 
     def write_file(self, path, content, pre_content=None):
         self.files[path] = content

tools/file_operations.py

@@ -277,11 +277,19 @@ class ShellFileOperations(LintMixin, SearchMixin, FileOperations):
             full = path if os.path.isabs(path) else os.path.join(
                 getattr(self.env, "cwd", None) or self.cwd, path)
             try:
-                if _stat.S_ISREG(os.stat(full).st_mode):
-                    with open(full, "rb") as fh:
-                        return fh.read(), None
+                # One lookup, not two: a stat-then-open pair can have the path swapped for a FIFO in
+                # between, and that open blocks this thread forever (no backend timeout covers it).
+                # O_NONBLOCK returns a descriptor for a FIFO instead of waiting, and fstat judges THAT
+                # descriptor, so a non-regular file is rejected rather than read.
+                fd = os.open(full, os.O_RDONLY | getattr(os, "O_NONBLOCK", 0))
+                try:
+                    if _stat.S_ISREG(os.fstat(fd).st_mode):
+                        with open(fd, "rb", closefd=False) as fh:
+                            return fh.read(), None
+                finally:
+                    os.close(fd)
             except OSError:
-                pass  # missing/unreadable: the shell read below reports it the usual way
+                pass  # missing/unreadable/would-block: the shell read below reports it the usual way
         # Fenced like the compound read probe, and for the same reason: a backend whose merged
         # stdout carries login-shell noise (a remote shell announcing TERM, a banner) would
         # otherwise have it whitespace-joined onto the payload and decoded INTO the file's bytes,
@@ -301,6 +309,8 @@ class ShellFileOperations(LintMixin, SearchMixin, FileOperations):
             read_rc = int(status[0])
         except (IndexError, ValueError):
             return None, garbled
+        if read_rc == 127:  # no base64 on this backend (busybox, distroless): try the hex transport
+            return self._read_exact_bytes_hex(path)
         if read_rc != 0:
             # stderr is merged into the fenced segment, so that segment holds base64's own diagnostic
             # ("No such file or directory", "Permission denied"): keep it for the caller's message.
@@ -312,6 +322,41 @@ class ShellFileOperations(LintMixin, SearchMixin, FileOperations):
             return None, garbled
         return data, None
 
+    def _read_exact_bytes_hex(self, path: str) -> "tuple[Optional[bytes], Optional[ExecuteResult]]":
+        """``od`` fallback for a backend without ``base64``, fenced the same way.
+
+        ``read_file_raw`` is the edit paths' source read AND, through ``_apply_add``, their
+        existence check, so a transport that simply is not installed must not read as "no such
+        file" — that clobbers the file the Add was refusing to overwrite. ``od`` is POSIX and
+        present in busybox; when it is missing too the caller gets a transport error, never a
+        not-found."""
+        sentinel = _new_sentinel(_BYTES_SENTINEL_PREFIX)
+        mark = f"echo {sentinel}"
+        result = self._exec(
+            f"{mark}; od -An -v -tx1 < {self._escape_shell_arg(path)}; __hb=$?; {mark}; echo $__hb")
+        segments = _split_segments(result.stdout or "", sentinel)
+        unavailable = ExecuteResult(
+            stdout=f"{path}: this backend has neither base64 nor od, so a byte-exact read is unavailable",
+            exit_code=1)
+        if len(segments) != 3:
+            return None, result if result.exit_code != 0 else unavailable
+        status = _strip_terminal_fence_leaks(segments[2]).split()
+        try:
+            read_rc = int(status[0])
+        except (IndexError, ValueError):
+            return None, unavailable
+        if read_rc == 127:
+            return None, unavailable
+        if read_rc != 0:
+            return None, ExecuteResult(
+                stdout=_strip_terminal_fence_leaks(segments[1]).strip() or f"{path}: exit {read_rc}",
+                exit_code=read_rc)
+        try:
+            return bytes.fromhex("".join(_strip_terminal_fence_leaks(segments[1]).split())), None
+        except ValueError:
+            return None, ExecuteResult(stdout=f"{path}: the backend returned a garbled byte-exact read",
+                                       exit_code=1)
+
     @staticmethod
     def _decode_base64_sample(text: str) -> Optional[bytes]:
         """Decode one ``base64`` transport reply (a ``head -c N`` sample or a whole file). Whitespace-joins
@@ -1112,7 +1157,8 @@ class ShellFileOperations(LintMixin, SearchMixin, FileOperations):
                 if score > 0:
                     scored.append((score, os.path.join(dir_path, f)))
         scored.sort(key=lambda x: -x[0])
-        return ReadResult(error=f"File not found: {path}", similar_files=[fp for _, fp in scored[:5]])
+        return ReadResult(error=f"File not found: {path}", not_found=True,
+                          similar_files=[fp for _, fp in scored[:5]])
 
     def read_file_raw(self, path: str) -> ReadResult:
         """Whole file as a plain string (no pagination/line numbers/clamping)."""
@@ -1145,7 +1191,7 @@ class ShellFileOperations(LintMixin, SearchMixin, FileOperations):
         path = self._expand_path(path)
         file_size, status = self._probe_regular_file(path)
         if status == "missing":
-            return ReadResult(error=f"File not found: {path}")
+            return ReadResult(error=f"File not found: {path}", not_found=True)
         if status == "not_regular":
             return self._not_regular_error(path)
         if status not in ("ok", "bad_size"):

tools/file_operations_common.py

@@ -25,6 +25,10 @@ class ReadResult:
     mime_type: Optional[str] = None
     dimensions: Optional[str] = None  # For images: "WIDTHxHEIGHT"
     error: Optional[str] = None
+    #: True only when the path is genuinely absent. An error with this False is a read that
+    #: FAILED (transport down, no byte transport installed); callers deciding whether a path
+    #: is free must not read that as "absent". See patch_parser._apply_add.
+    not_found: bool = False
     similar_files: List[str] = field(default_factory=list)
     _snapshot: Optional[tuple] = None
 

tools/patch_parser.py

@@ -331,6 +331,10 @@ def _apply_add(op: PatchOperation, file_ops: Any) -> ApplyResult:
     read_back = file_ops.read_file_raw(op.file_path)
     if not read_back.error:
         return _fail(f"{op.file_path}: file already exists — use Update File, not Add File")
+    if not getattr(read_back, "not_found", False):
+        # The read FAILED; it did not report an absent path. Treating that as "the path is free"
+        # writes the Add payload over whatever is actually there.
+        return _fail(f"{op.file_path}: could not confirm the path is free — {read_back.error}")
     content_lines = [line.content for hunk in op.hunks for line in hunk.lines if line.prefix == '+']
     result = file_ops.write_file(op.file_path, '\n'.join(content_lines))
     diff = f"--- /dev/null\n+++ b/{op.file_path}\n" + '\n'.join(f"+{line}" for line in content_lines)