Fix brittle imports and cycles after moving atomic_write_text to filesystem.py

- callers (Edit/Insert/Write/diff applier) import atomic_write_text
  lazily from .filesystem to avoid the filesystem<->tool-module cycle
- drop now-unused imports from base.py, add stat/uuid/contextlib to
  filesystem.py where the helper now lives
- update the helper docstring and filesystem module doc to match the
  new home; re-export via __all__
- tests import atomic_write_text from tools.filesystem

Author: beacoder

python_agent_harness/tools/base.py

@@ -2,13 +2,8 @@
 
 from __future__ import annotations
 
-import contextlib
-import os
-import stat
 import threading
-import uuid
 from abc import ABC, abstractmethod
-from pathlib import Path
 from typing import Any, Protocol, runtime_checkable
 
 from ..core.models import ToolSpec

python_agent_harness/tools/diffapply.py

@@ -26,8 +26,6 @@
 import os
 import re
 
-from .base import atomic_write_text
-
 _HUNK_HEADER_RE = re.compile(r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@")
 _FILE_OLD_RE = re.compile(r"^---[ \t]")
 _FILE_NEW_RE = re.compile(r"^\+\+\+[ \t]")
@@ -257,6 +255,8 @@ def _apply_section(section: _Section, cwd: str, fallback_path: str | None) -> tu
     ending = _file_line_ending(file_lines)
     for hunk, pos in reversed(plan):  # bottom-up: earlier positions stay valid
         _apply_hunk(hunk, pos, new_lines, ending)
+    from .filesystem import atomic_write_text
+
     try:
         atomic_write_text(target, "".join(new_lines))
     except OSError as e:

python_agent_harness/tools/edit.py

@@ -16,7 +16,7 @@
 import subprocess
 
 from ..io.diffrender import unified_diff
-from .base import Tool, ToolContext, atomic_write_text
+from .base import Tool, ToolContext
 
 # Any line ending, as a single compiled pattern (CRLF first so a CRLF is
 # never rewritten as two endings).
@@ -168,6 +168,8 @@ def _string_replace(self, path: str, old: str | None, new_str: str, ctx: ToolCon
                 "for the replacement, or a unified diff"
             )
         new = content.replace(old, new_str, 1)
+        from .filesystem import atomic_write_text
+
         try:
             atomic_write_text(path, new)
         except OSError as e:

python_agent_harness/tools/filesystem.py

@@ -1,12 +1,13 @@
 """Filesystem tool helpers and compatibility re-exports.
 
 This module hosts the SHARED helper machinery for the filesystem
-tools — spooling oversized tool results to temp files
-(`gptel-agent--truncate-buffer` parity), git-root detection, and the
-``natnump`` predicate — and re-exports the tool classes that live in
-per-tool modules (`read.py`, `glob.py`, `grep.py`, `edit.py`,
-`write.py`, `insert.py`, `mkdir.py`), so existing imports such as
-``from .tools.filesystem import Read`` keep working.
+tools — atomic file replacement (``atomic_write_text``), spooling
+oversized tool results to temp files (`gptel-agent--truncate-buffer`
+parity), git-root detection, and the ``natnump`` predicate — and
+re-exports the tool classes that live in per-tool modules (`read.py`,
+`glob.py`, `grep.py`, `edit.py`, `write.py`, `insert.py`, `mkdir.py`),
+so existing imports such as ``from .tools.filesystem import Read``
+keep working.
 
 The helpers must stay defined HERE (not in a separate ``_common``
 module): tests monkey-patch ``filesystem._spool_dir`` and read
@@ -31,13 +32,16 @@
 
 from __future__ import annotations
 
+import contextlib
 import os
 import re
 import shutil  # noqa: F401  (mock target for tests)
+import stat
 import subprocess  # noqa: F401  (mock target for tests)
 import tempfile
 import threading
 import time
+import uuid
 from collections.abc import Callable, Iterator
 from pathlib import Path
 from typing import TypeGuard
@@ -232,13 +236,15 @@ def atomic_write_text(path: str, content: str) -> None:
     """Replace PATH's contents with CONTENT atomically.
 
     THE single write path for every tool that rewrites a file (``Edit``,
-    ``Insert``, ``Write``, and the pure-Python diff applier).  It lives
-    here, in the module every tool already imports, so the four callers
-    cannot drift apart -- and because ``base`` imports nothing from
-    ``tools``, putting it here is also the only placement that avoids a
-    circular import: ``filesystem.py`` re-imports ``edit``/``write``/
-    ``insert`` at its bottom, so a helper defined *there* would make
-    ``edit`` import a half-initialised ``filesystem``.
+    ``Insert``, ``Write``, and the pure-Python diff applier), so the four
+    callers cannot drift apart.  It lives in this module with the other
+    shared helpers, and the four callers import it LAZILY (inside the
+    method that writes) rather than at module level, because this module
+    re-imports ``edit``/``write``/``insert``/``diffapply`` at its bottom
+    for the compatibility re-exports: a module-level import here would
+    close the cycle on a half-initialised module (``tools/__init__``
+    loads ``edit`` first, whose import of this module would then ask the
+    half-initialised ``edit`` for ``Edit``).
 
     A plain ``open(path, "w")`` TRUNCATES the file before writing, so a
     write that fails partway through -- ENOSPC, a quota, an I/O error,
@@ -345,6 +351,7 @@ def atomic_write_text(path: str, content: str) -> None:
     "MAX_OUTPUT",
     "READ_SIZE_LIMIT",
     "SPOOL_LINES",
+    "atomic_write_text",
     "cleanup_spooled_files",
     "_fix_patch_headers",
     "_git_glob_results",

python_agent_harness/tools/insert.py

@@ -9,7 +9,7 @@
 import os
 
 from ..io.diffrender import unified_diff
-from .base import Tool, ToolContext, atomic_write_text
+from .base import Tool, ToolContext
 from .edit import _to_crlf, _uses_crlf
 
 
@@ -77,6 +77,8 @@ def run(self, args: dict, ctx: ToolContext) -> str:
         else:
             lines.insert(ln, new_str)
         new_content = "".join(lines)
+        from .filesystem import atomic_write_text
+
         try:
             atomic_write_text(path, new_content)
         except OSError as e:

python_agent_harness/tools/write.py

@@ -5,7 +5,7 @@
 import os
 
 from ..io.diffrender import unified_diff
-from .base import Tool, ToolContext, atomic_write_text
+from .base import Tool, ToolContext
 
 
 class Write(Tool):
@@ -82,6 +82,8 @@ def run(self, args: dict, ctx: ToolContext) -> str:
             # mirrors the read side; both together keep Write's behavior
             # identical on every platform.  Both are handled by
             # atomic_write_text, which also makes the overwrite crash-safe.
+            from .filesystem import atomic_write_text
+
             atomic_write_text(path, content)
         except OSError as e:
             return f"Error: {e}"

tests/tools/test_filesystem.py

@@ -14,7 +14,7 @@
 from pathlib import Path
 from unittest import mock
 
-from python_agent_harness.tools.base import ToolContext, ToolRuntime, atomic_write_text
+from python_agent_harness.tools.base import ToolContext, ToolRuntime
 from python_agent_harness.tools.diffapply import apply_unified_diff, diff_targets
 from python_agent_harness.tools.edit_mac import EditMac
 from python_agent_harness.tools.edit_win import EditWindows
@@ -28,6 +28,7 @@
     Write,
     _fix_patch_headers,
     _strip_diff_fence,
+    atomic_write_text,
 )
 from python_agent_harness.tools.glob_mac import GlobMac
 from python_agent_harness.tools.glob_win import GlobWindows