fix(memory): support Windows memory file locking

tests/tools/test_memory_tool.py

@@ -4,6 +4,7 @@
 import pytest
 from pathlib import Path
 
+import tools.memory_tool as memory_tool_module
 from tools.memory_tool import (
     MemoryStore,
     memory_tool,
@@ -201,12 +202,107 @@ def test_deduplication_on_load(self, tmp_path, monkeypatch):
         monkeypatch.setattr("tools.memory_tool.get_memory_dir", lambda: tmp_path)
         # Write file with duplicates
         mem_file = tmp_path / "MEMORY.md"
-        mem_file.write_text("duplicate entry\n§\nduplicate entry\n§\nunique entry")
+        mem_file.write_text("duplicate entry\n§\nduplicate entry\n§\nunique entry", encoding="utf-8")
 
         store = MemoryStore()
         store.load_from_disk()
         assert len(store.memory_entries) == 2
 
+    def test_windows_lock_fallback_uses_msvcrt(self, store, monkeypatch):
+        calls = []
+
+        class FakeMSVCRT:
+            LK_LOCK = 1
+            LK_UNLCK = 2
+
+            @staticmethod
+            def locking(fileno, mode, size):
+                calls.append((mode, size))
+
+        monkeypatch.setattr("tools.memory_tool._HAS_FCNTL", False)
+        monkeypatch.setattr("tools.memory_tool._HAS_MSVCRT", True)
+        monkeypatch.setattr("tools.memory_tool._msvcrt", FakeMSVCRT)
+
+        result = store.add("memory", "Windows lock fallback")
+
+        assert result["success"] is True
+        assert calls == [(FakeMSVCRT.LK_LOCK, 1), (FakeMSVCRT.LK_UNLCK, 1)]
+
+    def test_windows_lock_retries_with_integer_fd(self, tmp_path, monkeypatch):
+        calls = []
+        sleeps = []
+        attempts = {"count": 0}
+
+        class FakeMSVCRT:
+            LK_LOCK = 1
+            LK_UNLCK = 2
+
+            @staticmethod
+            def locking(fileno, mode, size):
+                calls.append((fileno, mode, size))
+                if mode == FakeMSVCRT.LK_LOCK:
+                    attempts["count"] += 1
+                    if attempts["count"] < 3:
+                        raise OSError("lock busy")
+
+        monkeypatch.setattr("tools.memory_tool._HAS_FCNTL", False)
+        monkeypatch.setattr("tools.memory_tool._HAS_MSVCRT", True)
+        monkeypatch.setattr("tools.memory_tool._msvcrt", FakeMSVCRT)
+        monkeypatch.setattr("tools.memory_tool.time.sleep", sleeps.append)
+
+        with MemoryStore._file_lock(tmp_path / "MEMORY.md"):
+            pass
+
+        assert sleeps == [
+            memory_tool_module._WINDOWS_LOCK_RETRY_DELAY_SECONDS,
+            memory_tool_module._WINDOWS_LOCK_RETRY_DELAY_SECONDS,
+        ]
+        assert all(isinstance(fileno, int) for fileno, _, _ in calls)
+        assert [mode for _, mode, _ in calls] == [1, 1, 1, 2]
+
+    def test_windows_lock_timeout_raises(self, tmp_path, monkeypatch):
+        sleeps = []
+
+        class FakeMSVCRT:
+            LK_LOCK = 1
+            LK_UNLCK = 2
+
+            @staticmethod
+            def locking(fileno, mode, size):
+                if mode == FakeMSVCRT.LK_LOCK:
+                    raise OSError("still busy")
+
+        monkeypatch.setattr("tools.memory_tool._HAS_FCNTL", False)
+        monkeypatch.setattr("tools.memory_tool._HAS_MSVCRT", True)
+        monkeypatch.setattr("tools.memory_tool._msvcrt", FakeMSVCRT)
+        monkeypatch.setattr("tools.memory_tool.time.sleep", sleeps.append)
+
+        with pytest.raises(TimeoutError):
+            with MemoryStore._file_lock(tmp_path / "MEMORY.md"):
+                pass
+
+        assert len(sleeps) == memory_tool_module._WINDOWS_LOCK_RETRY_ATTEMPTS
+
+    def test_unix_lock_path_uses_fcntl(self, tmp_path, monkeypatch):
+        calls = []
+
+        class FakeFCNTL:
+            LOCK_EX = 1
+            LOCK_UN = 2
+
+            @staticmethod
+            def flock(fileobj, mode):
+                calls.append((fileobj, mode))
+
+        monkeypatch.setattr("tools.memory_tool._HAS_FCNTL", True)
+        monkeypatch.setattr("tools.memory_tool._HAS_MSVCRT", False)
+        monkeypatch.setattr("tools.memory_tool._fcntl", FakeFCNTL)
+
+        with MemoryStore._file_lock(tmp_path / "MEMORY.md"):
+            pass
+
+        assert [mode for _, mode in calls] == [FakeFCNTL.LOCK_EX, FakeFCNTL.LOCK_UN]
+
 
 class TestMemoryStoreSnapshot:
     def test_snapshot_frozen_at_load(self, store):

tools/memory_tool.py

@@ -28,21 +28,26 @@
 import os
 import re
 import tempfile
+import time
 from contextlib import contextmanager
 from pathlib import Path
 from hermes_constants import get_hermes_home
 from typing import Dict, Any, List, Optional
 
-# fcntl is Unix-only; on Windows use msvcrt for file locking
-msvcrt = None
 try:
-    import fcntl
+    import fcntl as _fcntl
 except ImportError:
-    fcntl = None
-    try:
-        import msvcrt
-    except ImportError:
-        pass
+    _fcntl = None
+
+try:
+    import msvcrt as _msvcrt
+except ImportError:
+    _msvcrt = None
+
+_HAS_FCNTL = _fcntl is not None
+_HAS_MSVCRT = _msvcrt is not None
+_WINDOWS_LOCK_RETRY_ATTEMPTS = 100
+_WINDOWS_LOCK_RETRY_DELAY_SECONDS = 0.1
 
 logger = logging.getLogger(__name__)
 
@@ -145,36 +150,58 @@ def _file_lock(path: Path):
         """Acquire an exclusive file lock for read-modify-write safety.
 
         Uses a separate .lock file so the memory file itself can still be
-        atomically replaced via os.replace().
+        atomically replaced via os.replace(). On Unix we use flock(); on
+        Windows we use msvcrt.locking() with a short retry loop.
         """
         lock_path = path.with_suffix(path.suffix + ".lock")
         lock_path.parent.mkdir(parents=True, exist_ok=True)
 
-        if fcntl is None and msvcrt is None:
+        if not _HAS_FCNTL and not _HAS_MSVCRT:
             yield
             return
 
-        if msvcrt and (not lock_path.exists() or lock_path.stat().st_size == 0):
-            lock_path.write_text(" ", encoding="utf-8")
-
-        fd = open(lock_path, "r+" if msvcrt else "a+")
+        lock_file = open(lock_path, "a+b")
         try:
-            if fcntl:
-                fcntl.flock(fd, fcntl.LOCK_EX)
+            if _HAS_FCNTL:
+                _fcntl.flock(lock_file, _fcntl.LOCK_EX)
             else:
-                fd.seek(0)
-                msvcrt.locking(fd.fileno(), msvcrt.LK_LOCK, 1)
+                MemoryStore._acquire_windows_lock(lock_file, lock_path)
             yield
         finally:
-            if fcntl:
-                fcntl.flock(fd, fcntl.LOCK_UN)
-            elif msvcrt:
+            if _HAS_FCNTL:
+                _fcntl.flock(lock_file, _fcntl.LOCK_UN)
+            elif _HAS_MSVCRT:
                 try:
-                    fd.seek(0)
-                    msvcrt.locking(fd.fileno(), msvcrt.LK_UNLCK, 1)
-                except (OSError, IOError):
+                    MemoryStore._release_windows_lock(lock_file)
+                except OSError:
                     pass
-            fd.close()
+            lock_file.close()
+
+    @staticmethod
+    def _acquire_windows_lock(lock_file, lock_path: Path):
+        """Acquire a Windows file lock, retrying briefly for concurrent writers."""
+        lock_file.seek(0)
+        lock_file.write(b"0")
+        lock_file.flush()
+        lock_file.seek(0)
+
+        last_error = None
+        for _ in range(_WINDOWS_LOCK_RETRY_ATTEMPTS):
+            try:
+                _msvcrt.locking(lock_file.fileno(), _msvcrt.LK_LOCK, 1)
+                return
+            except OSError as exc:
+                last_error = exc
+                time.sleep(_WINDOWS_LOCK_RETRY_DELAY_SECONDS)
+                lock_file.seek(0)
+
+        raise TimeoutError(f"Timed out acquiring memory lock for {lock_path}") from last_error
+
+    @staticmethod
+    def _release_windows_lock(lock_file):
+        """Release the Windows file lock if it was successfully acquired."""
+        lock_file.seek(0)
+        _msvcrt.locking(lock_file.fileno(), _msvcrt.LK_UNLCK, 1)
 
     @staticmethod
     def _path_for(target: str) -> Path: