From 1998aede66053a9d54421f0c5c533ee89a617e89 Mon Sep 17 00:00:00 2001 From: Felipe Truman Date: Fri, 17 Apr 2026 16:26:11 -0300 Subject: [PATCH] fix: Windows CI compat for palace lock tests and path normalization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the two actionable Copilot comments from the 2nd review pass. tests/test_palace_locks.py (#7, #8) multiprocessing.get_context("fork") is unavailable on Windows, so the cross-process tests would crash the Windows CI runner. Added `_get_mp_context()` that picks "spawn" on Windows and "fork" elsewhere. Spawn re-imports the module in the child; it inherits os.environ (including the monkeypatched HOME), which is all these tests need. mempalace/palace.py (#10) The per-palace lock key was computed from os.path.abspath(palace_path). On Windows the filesystem is case-insensitive, so `C:\\Palace` and `c:\\palace` would hash to different keys and two concurrent mines could touch the same on-disk palace. Switched to `os.path.normcase(os.path.realpath(...))` so: * realpath resolves symlinks and `..` segments * normcase folds case on Windows (no-op on POSIX) Testing pytest tests/test_palace_locks.py tests/test_hooks_cli.py tests/test_backends.py tests/test_cli.py → 98 passed, 0 failed. --- mempalace/palace.py | 11 +++++++++-- tests/test_palace_locks.py | 16 ++++++++++++++-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/mempalace/palace.py b/mempalace/palace.py index 917c76d..07efb6a 100644 --- a/mempalace/palace.py +++ b/mempalace/palace.py @@ -329,14 +329,21 @@ def mine_palace_lock(palace_path: str): *different* palaces can still run in parallel — we only serialize writes into the same palace, which is the correctness boundary. + The key is derived from a fully normalized form of the path: + `realpath` resolves symlinks and `..` segments, and `normcase` folds + case on Windows (which has a case-insensitive filesystem). Without + normcase, `C:\\Palace` and `c:\\palace` would hash to different keys + on Windows and let two concurrent mines touch the same on-disk palace. + Non-blocking: if another `mine` is already writing to this palace, raise MineAlreadyRunning so the caller can exit cleanly instead of piling up as a waiting worker. """ lock_dir = os.path.join(os.path.expanduser("~"), ".mempalace", "locks") os.makedirs(lock_dir, exist_ok=True) - resolved = os.path.abspath(os.path.expanduser(palace_path)) - palace_key = hashlib.sha256(resolved.encode()).hexdigest()[:16] + resolved = os.path.realpath(os.path.expanduser(palace_path)) + lock_key_source = os.path.normcase(resolved) + palace_key = hashlib.sha256(lock_key_source.encode()).hexdigest()[:16] lock_path = os.path.join(lock_dir, f"mine_palace_{palace_key}.lock") lf = open(lock_path, "w") diff --git a/tests/test_palace_locks.py b/tests/test_palace_locks.py index a7596b9..601c894 100644 --- a/tests/test_palace_locks.py +++ b/tests/test_palace_locks.py @@ -22,6 +22,18 @@ from mempalace.palace import ( ) +def _get_mp_context(): + """Pick a start method that works on every CI runner. + + `fork` is cheaper (no re-import) but is unavailable on Windows, so we fall + back to `spawn` there. `spawn` inherits ``os.environ`` (including the + monkeypatched ``HOME``) and re-imports the ``mempalace`` package in the + child, which is sufficient for the lock-file semantics exercised here. + """ + start_method = "spawn" if os.name == "nt" else "fork" + return multiprocessing.get_context(start_method) + + # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- @@ -75,7 +87,7 @@ def test_same_palace_serializes_across_processes(tmp_path, monkeypatch): ready = str(tmp_path / "ready") release = str(tmp_path / "release") - ctx = multiprocessing.get_context("fork") + ctx = _get_mp_context() holder = ctx.Process(target=_hold_lock, args=(palace, ready, release)) holder.start() try: @@ -104,7 +116,7 @@ def test_different_palaces_dont_conflict(tmp_path, monkeypatch): ready = str(tmp_path / "ready_a") release = str(tmp_path / "release_a") - ctx = multiprocessing.get_context("fork") + ctx = _get_mp_context() holder = ctx.Process(target=_hold_lock, args=(palace_a, ready, release)) holder.start() try: