Skip to content

Commit 7e18a70

Browse files
felipetrumanclaude
authored and
igorls
committed
fix: resolve hooks_cli.py merge conflict + add mine_global_lock tests
- Resolve UU conflict in hooks_cli.py: take develop/HEAD approach (mine synchronously via _mine_sync, then pass through unconditionally). _mine_sync already catches subprocess.TimeoutExpired — fixes Copilot #1. - Add tests/test_palace_locks.py: 4 tests covering mine_global_lock non-blocking semantics (acquire, second-acquire raises MineAlreadyRunning, reusable after release, release on exception) — fixes Copilot #4. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
1 parent 91a6026 commit 7e18a70

5 files changed

Lines changed: 175 additions & 2 deletions

File tree

‎mempalace/backends/chroma.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,9 @@ def get_collection(
566566

567567
if create:
568568
collection = client.get_or_create_collection(
569-
collection_name, metadata={"hnsw:space": hnsw_space}, **ef_kwargs
569+
collection_name,
570+
metadata={"hnsw:space": hnsw_space, "hnsw:num_threads": 1},
571+
**ef_kwargs,
570572
)
571573
else:
572574
collection = client.get_collection(collection_name, **ef_kwargs)
@@ -613,7 +615,9 @@ def create_collection(
613615
ef = self._resolve_embedding_function()
614616
ef_kwargs = {"embedding_function": ef} if ef is not None else {}
615617
collection = self._client(palace_path).create_collection(
616-
collection_name, metadata={"hnsw:space": hnsw_space}, **ef_kwargs
618+
collection_name,
619+
metadata={"hnsw:space": hnsw_space, "hnsw:num_threads": 1},
620+
**ef_kwargs,
617621
)
618622
return ChromaCollection(collection)
619623

‎mempalace/hooks_cli.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,9 @@ def hook_session_start(data: dict, harness: str):
643643
_output({})
644644

645645

646+
MAX_PRECOMPACT_BLOCK_ATTEMPTS = 2
647+
648+
646649
def hook_precompact(data: dict, harness: str):
647650
"""Precompact hook: mine transcript synchronously, then allow compaction."""
648651
parsed = _parse_harness_input(data, harness)

‎mempalace/miner.py‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,12 @@
2020
from .palace import (
2121
NORMALIZE_VERSION,
2222
SKIP_DIRS,
23+
MineAlreadyRunning,
2324
build_closet_lines,
2425
file_already_mined,
2526
get_closets_collection,
2627
get_collection,
28+
mine_global_lock,
2729
mine_lock,
2830
purge_file_closets,
2931
upsert_closet_lines,
@@ -993,6 +995,48 @@ def mine(
993995
``mine`` walks the tree itself just like before.
994996
"""
995997

998+
if dry_run:
999+
return _mine_impl(
1000+
project_dir,
1001+
palace_path,
1002+
wing_override=wing_override,
1003+
agent=agent,
1004+
limit=limit,
1005+
dry_run=dry_run,
1006+
respect_gitignore=respect_gitignore,
1007+
include_ignored=include_ignored,
1008+
)
1009+
1010+
try:
1011+
with mine_global_lock():
1012+
return _mine_impl(
1013+
project_dir,
1014+
palace_path,
1015+
wing_override=wing_override,
1016+
agent=agent,
1017+
limit=limit,
1018+
dry_run=dry_run,
1019+
respect_gitignore=respect_gitignore,
1020+
include_ignored=include_ignored,
1021+
)
1022+
except MineAlreadyRunning:
1023+
print(
1024+
"mempalace: another `mine` is already running — exiting cleanly.",
1025+
file=sys.stderr,
1026+
)
1027+
return
1028+
1029+
1030+
def _mine_impl(
1031+
project_dir: str,
1032+
palace_path: str,
1033+
wing_override: str = None,
1034+
agent: str = "mempalace",
1035+
limit: int = 0,
1036+
dry_run: bool = False,
1037+
respect_gitignore: bool = True,
1038+
include_ignored: list = None,
1039+
):
9961040
project_path = Path(project_dir).expanduser().resolve()
9971041
config = load_config(project_dir)
9981042

‎mempalace/palace.py‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,69 @@ def mine_lock(source_file: str):
310310
lf.close()
311311

312312

313+
class MineAlreadyRunning(RuntimeError):
314+
"""Raised when another `mempalace mine` process already holds the global lock."""
315+
316+
317+
@contextlib.contextmanager
318+
def mine_global_lock():
319+
"""Process-wide non-blocking lock around the full `mine` pipeline.
320+
321+
The per-file `mine_lock` only protects delete+insert interleave for a
322+
single source; it does not prevent N copies of `mempalace mine <dir>`
323+
from being spawned concurrently by hooks. When that happens, each copy
324+
drives ChromaDB HNSW inserts in parallel, which (combined with
325+
chromadb's multi-threaded ParallelFor) can corrupt the HNSW graph and
326+
produce sparse link_lists.bin blowups.
327+
328+
This lock is non-blocking: if another `mine` is already running, we
329+
raise MineAlreadyRunning so the caller can exit cleanly instead of
330+
piling up waiting workers.
331+
"""
332+
lock_dir = os.path.join(os.path.expanduser("~"), ".mempalace", "locks")
333+
os.makedirs(lock_dir, exist_ok=True)
334+
lock_path = os.path.join(lock_dir, "mine_global.lock")
335+
336+
lf = open(lock_path, "w")
337+
acquired = False
338+
try:
339+
if os.name == "nt":
340+
import msvcrt
341+
342+
try:
343+
msvcrt.locking(lf.fileno(), msvcrt.LK_NBLCK, 1)
344+
acquired = True
345+
except OSError as exc:
346+
raise MineAlreadyRunning(
347+
"another `mempalace mine` is already running"
348+
) from exc
349+
else:
350+
import fcntl
351+
352+
try:
353+
fcntl.flock(lf, fcntl.LOCK_EX | fcntl.LOCK_NB)
354+
acquired = True
355+
except BlockingIOError as exc:
356+
raise MineAlreadyRunning(
357+
"another `mempalace mine` is already running"
358+
) from exc
359+
yield
360+
finally:
361+
if acquired:
362+
try:
363+
if os.name == "nt":
364+
import msvcrt
365+
366+
msvcrt.locking(lf.fileno(), msvcrt.LK_UNLCK, 1)
367+
else:
368+
import fcntl
369+
370+
fcntl.flock(lf, fcntl.LOCK_UN)
371+
except Exception:
372+
pass
373+
lf.close()
374+
375+
313376
def file_already_mined(collection, source_file: str, check_mtime: bool = False) -> bool:
314377
"""Check if a file has already been filed in the palace.
315378

‎tests/test_palace_locks.py‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
"""Tests for mine_global_lock: non-blocking cross-process lock semantics."""
2+
import threading
3+
4+
import pytest
5+
6+
from mempalace.palace import MineAlreadyRunning, mine_global_lock
7+
8+
9+
def test_mine_global_lock_acquired(tmp_path, monkeypatch):
10+
"""Lock is acquired and released without error."""
11+
monkeypatch.setenv("HOME", str(tmp_path))
12+
with mine_global_lock():
13+
pass # should not raise
14+
15+
16+
def test_mine_global_lock_second_acquire_raises(tmp_path, monkeypatch):
17+
"""Concurrent second acquire raises MineAlreadyRunning."""
18+
monkeypatch.setenv("HOME", str(tmp_path))
19+
results: list[str] = []
20+
21+
with mine_global_lock():
22+
# While this lock is held, spawn a thread that tries to acquire.
23+
def try_acquire():
24+
try:
25+
with mine_global_lock():
26+
results.append("acquired")
27+
except MineAlreadyRunning:
28+
results.append("blocked")
29+
30+
t = threading.Thread(target=try_acquire)
31+
t.start()
32+
t.join(timeout=5)
33+
34+
assert results == ["blocked"]
35+
36+
37+
def test_mine_global_lock_reusable_after_release(tmp_path, monkeypatch):
38+
"""Lock can be re-acquired after the context manager exits."""
39+
monkeypatch.setenv("HOME", str(tmp_path))
40+
41+
with mine_global_lock():
42+
pass # first acquire + release
43+
44+
# Second acquire must succeed; MineAlreadyRunning would propagate as failure.
45+
with mine_global_lock():
46+
pass
47+
48+
49+
def test_mine_global_lock_exception_still_releases(tmp_path, monkeypatch):
50+
"""Lock is released even when the body raises."""
51+
monkeypatch.setenv("HOME", str(tmp_path))
52+
53+
with pytest.raises(ValueError):
54+
with mine_global_lock():
55+
raise ValueError("boom")
56+
57+
# Must be acquirable again after the exception.
58+
with mine_global_lock():
59+
pass

0 commit comments

Comments
 (0)