Merge upstream/main into bench/scale-test-suite to resolve conflicts

Merged both the PR's benchmark suite additions (psutil dep, pytest
markers, --ignore=tests/benchmarks) and upstream's coverage changes
(pytest-cov, --cov-fail-under=30, coverage config) so both coexist.

Co-authored-by: factory-droid[bot] <138933559+factory-droid[bot]@users.noreply.github.com>

tests/conftest.py

@@ -34,6 +34,24 @@ from mempalace.config import MempalaceConfig  # noqa: E402
 from mempalace.knowledge_graph import KnowledgeGraph  # noqa: E402
 
 
+@pytest.fixture(autouse=True)
+def _reset_mcp_cache():
+    """Reset the MCP server's cached ChromaDB client/collection between tests."""
+
+    def _clear_cache():
+        try:
+            from mempalace import mcp_server
+
+            mcp_server._client_cache = None
+            mcp_server._collection_cache = None
+        except (ImportError, AttributeError):
+            pass
+
+    _clear_cache()
+    yield
+    _clear_cache()
+
+
 @pytest.fixture(scope="session", autouse=True)
 def _isolate_home():
     """Ensure HOME points to a temp dir for the entire test session.
@@ -84,7 +102,9 @@ def collection(palace_path):
     """A ChromaDB collection pre-seeded in the temp palace."""
     client = chromadb.PersistentClient(path=palace_path)
     col = client.get_or_create_collection("mempalace_drawers")
-    return col
+    yield col
+    client.delete_collection("mempalace_drawers")
+    del client
 
 
 @pytest.fixture

tests/test_hooks_cli.py

@@ -0,0 +1,207 @@
+import contextlib
+import json
+from pathlib import Path
+from unittest.mock import patch
+
+from mempalace.hooks_cli import (
+    SAVE_INTERVAL,
+    STOP_BLOCK_REASON,
+    PRECOMPACT_BLOCK_REASON,
+    _count_human_messages,
+    _sanitize_session_id,
+    hook_stop,
+    hook_session_start,
+    hook_precompact,
+)
+
+
+# --- _sanitize_session_id ---
+
+
+def test_sanitize_normal_id():
+    assert _sanitize_session_id("abc-123_XYZ") == "abc-123_XYZ"
+
+
+def test_sanitize_strips_dangerous_chars():
+    assert _sanitize_session_id("../../etc/passwd") == "etcpasswd"
+
+
+def test_sanitize_empty_returns_unknown():
+    assert _sanitize_session_id("") == "unknown"
+    assert _sanitize_session_id("!!!") == "unknown"
+
+
+# --- _count_human_messages ---
+
+
+def _write_transcript(path: Path, entries: list[dict]):
+    with open(path, "w", encoding="utf-8") as f:
+        for entry in entries:
+            f.write(json.dumps(entry) + "\n")
+
+
+def test_count_human_messages_basic(tmp_path):
+    transcript = tmp_path / "t.jsonl"
+    _write_transcript(
+        transcript,
+        [
+            {"message": {"role": "user", "content": "hello"}},
+            {"message": {"role": "assistant", "content": "hi"}},
+            {"message": {"role": "user", "content": "bye"}},
+        ],
+    )
+    assert _count_human_messages(str(transcript)) == 2
+
+
+def test_count_skips_command_messages(tmp_path):
+    transcript = tmp_path / "t.jsonl"
+    _write_transcript(
+        transcript,
+        [
+            {"message": {"role": "user", "content": "<command-message>status</command-message>"}},
+            {"message": {"role": "user", "content": "real question"}},
+        ],
+    )
+    assert _count_human_messages(str(transcript)) == 1
+
+
+def test_count_handles_list_content(tmp_path):
+    transcript = tmp_path / "t.jsonl"
+    _write_transcript(
+        transcript,
+        [
+            {"message": {"role": "user", "content": [{"type": "text", "text": "hello"}]}},
+            {
+                "message": {
+                    "role": "user",
+                    "content": [{"type": "text", "text": "<command-message>x</command-message>"}],
+                }
+            },
+        ],
+    )
+    assert _count_human_messages(str(transcript)) == 1
+
+
+def test_count_missing_file():
+    assert _count_human_messages("/nonexistent/path.jsonl") == 0
+
+
+def test_count_empty_file(tmp_path):
+    transcript = tmp_path / "t.jsonl"
+    transcript.write_text("")
+    assert _count_human_messages(str(transcript)) == 0
+
+
+def test_count_malformed_json_lines(tmp_path):
+    transcript = tmp_path / "t.jsonl"
+    transcript.write_text('not json\n{"message": {"role": "user", "content": "ok"}}\n')
+    assert _count_human_messages(str(transcript)) == 1
+
+
+# --- hook_stop ---
+
+
+def _capture_hook_output(hook_fn, data, harness="claude-code", state_dir=None):
+    """Run a hook and capture its JSON stdout output."""
+    import io
+
+    buf = io.StringIO()
+    patches = [patch("mempalace.hooks_cli._output", side_effect=lambda d: buf.write(json.dumps(d)))]
+    if state_dir:
+        patches.append(patch("mempalace.hooks_cli.STATE_DIR", state_dir))
+    with contextlib.ExitStack() as stack:
+        for p in patches:
+            stack.enter_context(p)
+        hook_fn(data, harness)
+    return json.loads(buf.getvalue())
+
+
+def test_stop_hook_passthrough_when_active(tmp_path):
+    with patch("mempalace.hooks_cli.STATE_DIR", tmp_path):
+        result = _capture_hook_output(
+            hook_stop,
+            {"session_id": "test", "stop_hook_active": True, "transcript_path": ""},
+            state_dir=tmp_path,
+        )
+    assert result == {}
+
+
+def test_stop_hook_passthrough_when_active_string(tmp_path):
+    with patch("mempalace.hooks_cli.STATE_DIR", tmp_path):
+        result = _capture_hook_output(
+            hook_stop,
+            {"session_id": "test", "stop_hook_active": "true", "transcript_path": ""},
+            state_dir=tmp_path,
+        )
+    assert result == {}
+
+
+def test_stop_hook_passthrough_below_interval(tmp_path):
+    transcript = tmp_path / "t.jsonl"
+    _write_transcript(
+        transcript,
+        [{"message": {"role": "user", "content": f"msg {i}"}} for i in range(SAVE_INTERVAL - 1)],
+    )
+    result = _capture_hook_output(
+        hook_stop,
+        {"session_id": "test", "stop_hook_active": False, "transcript_path": str(transcript)},
+        state_dir=tmp_path,
+    )
+    assert result == {}
+
+
+def test_stop_hook_blocks_at_interval(tmp_path):
+    transcript = tmp_path / "t.jsonl"
+    _write_transcript(
+        transcript,
+        [{"message": {"role": "user", "content": f"msg {i}"}} for i in range(SAVE_INTERVAL)],
+    )
+    result = _capture_hook_output(
+        hook_stop,
+        {"session_id": "test", "stop_hook_active": False, "transcript_path": str(transcript)},
+        state_dir=tmp_path,
+    )
+    assert result["decision"] == "block"
+    assert result["reason"] == STOP_BLOCK_REASON
+
+
+def test_stop_hook_tracks_save_point(tmp_path):
+    transcript = tmp_path / "t.jsonl"
+    _write_transcript(
+        transcript,
+        [{"message": {"role": "user", "content": f"msg {i}"}} for i in range(SAVE_INTERVAL)],
+    )
+    data = {"session_id": "test", "stop_hook_active": False, "transcript_path": str(transcript)}
+
+    # First call blocks
+    result = _capture_hook_output(hook_stop, data, state_dir=tmp_path)
+    assert result["decision"] == "block"
+
+    # Second call with same count passes through (already saved)
+    result = _capture_hook_output(hook_stop, data, state_dir=tmp_path)
+    assert result == {}
+
+
+# --- hook_session_start ---
+
+
+def test_session_start_passes_through(tmp_path):
+    result = _capture_hook_output(
+        hook_session_start,
+        {"session_id": "test"},
+        state_dir=tmp_path,
+    )
+    assert result == {}
+
+
+# --- hook_precompact ---
+
+
+def test_precompact_always_blocks(tmp_path):
+    result = _capture_hook_output(
+        hook_precompact,
+        {"session_id": "test"},
+        state_dir=tmp_path,
+    )
+    assert result["decision"] == "block"
+    assert result["reason"] == PRECOMPACT_BLOCK_REASON

tests/test_mcp_server.py

@@ -9,25 +9,26 @@ via monkeypatch to avoid touching real data.
 import json
 
 
-def _patch_mcp_server(monkeypatch, config, palace_path, kg):
+def _patch_mcp_server(monkeypatch, config, kg):
     """Patch the mcp_server module globals to use test fixtures."""
     from mempalace import mcp_server
 
-    assert getattr(config, "palace_path", None) == palace_path, (
-        f"config.palace_path ({getattr(config, 'palace_path', None)!r}) does not match palace_path fixture ({palace_path!r})"
-    )
     monkeypatch.setattr(mcp_server, "_config", config)
     monkeypatch.setattr(mcp_server, "_kg", kg)
 
 
 def _get_collection(palace_path, create=False):
-    """Helper to get collection from test palace."""
+    """Helper to get collection from test palace.
+
+    Returns (client, collection) so callers can clean up the client
+    when they are done.
+    """
     import chromadb
 
     client = chromadb.PersistentClient(path=palace_path)
     if create:
-        return client.get_or_create_collection("mempalace_drawers")
-    return client.get_collection("mempalace_drawers")
+        return client, client.get_or_create_collection("mempalace_drawers")
+    return client, client.get_collection("mempalace_drawers")
 
 
 # ── Protocol Layer ──────────────────────────────────────────────────────
@@ -77,11 +78,12 @@ class TestHandleRequest:
         assert resp["error"]["code"] == -32601
 
     def test_tools_call_dispatches(self, monkeypatch, config, palace_path, seeded_kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, seeded_kg)
+        _patch_mcp_server(monkeypatch, config, seeded_kg)
         from mempalace.mcp_server import handle_request
 
         # Create a collection so status works
-        _get_collection(palace_path, create=True)
+        _client, _col = _get_collection(palace_path, create=True)
+        del _client
 
         resp = handle_request(
             {
@@ -100,8 +102,9 @@ class TestHandleRequest:
 
 class TestReadTools:
     def test_status_empty_palace(self, monkeypatch, config, palace_path, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
-        _get_collection(palace_path, create=True)
+        _patch_mcp_server(monkeypatch, config, kg)
+        _client, _col = _get_collection(palace_path, create=True)
+        del _client
         from mempalace.mcp_server import tool_status
 
         result = tool_status()
@@ -109,7 +112,7 @@ class TestReadTools:
         assert result["wings"] == {}
 
     def test_status_with_data(self, monkeypatch, config, palace_path, seeded_collection, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
+        _patch_mcp_server(monkeypatch, config, kg)
         from mempalace.mcp_server import tool_status
 
         result = tool_status()
@@ -118,7 +121,7 @@ class TestReadTools:
         assert "notes" in result["wings"]
 
     def test_list_wings(self, monkeypatch, config, palace_path, seeded_collection, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
+        _patch_mcp_server(monkeypatch, config, kg)
         from mempalace.mcp_server import tool_list_wings
 
         result = tool_list_wings()
@@ -126,7 +129,7 @@ class TestReadTools:
         assert result["wings"]["notes"] == 1
 
     def test_list_rooms_all(self, monkeypatch, config, palace_path, seeded_collection, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
+        _patch_mcp_server(monkeypatch, config, kg)
         from mempalace.mcp_server import tool_list_rooms
 
         result = tool_list_rooms()
@@ -135,7 +138,7 @@ class TestReadTools:
         assert "planning" in result["rooms"]
 
     def test_list_rooms_filtered(self, monkeypatch, config, palace_path, seeded_collection, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
+        _patch_mcp_server(monkeypatch, config, kg)
         from mempalace.mcp_server import tool_list_rooms
 
         result = tool_list_rooms(wing="project")
@@ -143,7 +146,7 @@ class TestReadTools:
         assert "planning" not in result["rooms"]
 
     def test_get_taxonomy(self, monkeypatch, config, palace_path, seeded_collection, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
+        _patch_mcp_server(monkeypatch, config, kg)
         from mempalace.mcp_server import tool_get_taxonomy
 
         result = tool_get_taxonomy()
@@ -152,8 +155,7 @@ class TestReadTools:
         assert result["taxonomy"]["notes"]["planning"] == 1
 
     def test_no_palace_returns_error(self, monkeypatch, config, kg):
-        config._file_config["palace_path"] = "/nonexistent/path"
-        _patch_mcp_server(monkeypatch, config, "/nonexistent/path", kg)
+        _patch_mcp_server(monkeypatch, config, kg)
         from mempalace.mcp_server import tool_status
 
         result = tool_status()
@@ -165,7 +167,7 @@ class TestReadTools:
 
 class TestSearchTool:
     def test_search_basic(self, monkeypatch, config, palace_path, seeded_collection, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
+        _patch_mcp_server(monkeypatch, config, kg)
         from mempalace.mcp_server import tool_search
 
         result = tool_search(query="JWT authentication tokens")
@@ -176,14 +178,14 @@ class TestSearchTool:
         assert "JWT" in top["text"] or "authentication" in top["text"].lower()
 
     def test_search_with_wing_filter(self, monkeypatch, config, palace_path, seeded_collection, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
+        _patch_mcp_server(monkeypatch, config, kg)
         from mempalace.mcp_server import tool_search
 
         result = tool_search(query="planning", wing="notes")
         assert all(r["wing"] == "notes" for r in result["results"])
 
     def test_search_with_room_filter(self, monkeypatch, config, palace_path, seeded_collection, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
+        _patch_mcp_server(monkeypatch, config, kg)
         from mempalace.mcp_server import tool_search
 
         result = tool_search(query="database", room="backend")
@@ -195,8 +197,9 @@ class TestSearchTool:
 
 class TestWriteTools:
     def test_add_drawer(self, monkeypatch, config, palace_path, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
-        _get_collection(palace_path, create=True)
+        _patch_mcp_server(monkeypatch, config, kg)
+        _client, _col = _get_collection(palace_path, create=True)
+        del _client
         from mempalace.mcp_server import tool_add_drawer
 
         result = tool_add_drawer(
@@ -210,8 +213,9 @@ class TestWriteTools:
         assert result["drawer_id"].startswith("drawer_test_wing_test_room_")
 
     def test_add_drawer_duplicate_detection(self, monkeypatch, config, palace_path, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
-        _get_collection(palace_path, create=True)
+        _patch_mcp_server(monkeypatch, config, kg)
+        _client, _col = _get_collection(palace_path, create=True)
+        del _client
         from mempalace.mcp_server import tool_add_drawer
 
         content = "This is a unique test memory about Rust ownership and borrowing."
@@ -219,11 +223,11 @@ class TestWriteTools:
         assert result1["success"] is True
 
         result2 = tool_add_drawer(wing="w", room="r", content=content)
-        assert result2["success"] is False
-        assert result2["reason"] == "duplicate"
+        assert result2["success"] is True
+        assert result2["reason"] == "already_exists"
 
     def test_delete_drawer(self, monkeypatch, config, palace_path, seeded_collection, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
+        _patch_mcp_server(monkeypatch, config, kg)
         from mempalace.mcp_server import tool_delete_drawer
 
         result = tool_delete_drawer("drawer_proj_backend_aaa")
@@ -231,14 +235,14 @@ class TestWriteTools:
         assert seeded_collection.count() == 3
 
     def test_delete_drawer_not_found(self, monkeypatch, config, palace_path, seeded_collection, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
+        _patch_mcp_server(monkeypatch, config, kg)
         from mempalace.mcp_server import tool_delete_drawer
 
         result = tool_delete_drawer("nonexistent_drawer")
         assert result["success"] is False
 
     def test_check_duplicate(self, monkeypatch, config, palace_path, seeded_collection, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
+        _patch_mcp_server(monkeypatch, config, kg)
         from mempalace.mcp_server import tool_check_duplicate
 
         # Exact match text from seeded_collection should be flagged
@@ -262,7 +266,7 @@ class TestWriteTools:
 
 class TestKGTools:
     def test_kg_add(self, monkeypatch, config, palace_path, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
+        _patch_mcp_server(monkeypatch, config, kg)
         from mempalace.mcp_server import tool_kg_add
 
         result = tool_kg_add(
@@ -274,14 +278,14 @@ class TestKGTools:
         assert result["success"] is True
 
     def test_kg_query(self, monkeypatch, config, palace_path, seeded_kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, seeded_kg)
+        _patch_mcp_server(monkeypatch, config, seeded_kg)
         from mempalace.mcp_server import tool_kg_query
 
         result = tool_kg_query(entity="Max")
         assert result["count"] > 0
 
     def test_kg_invalidate(self, monkeypatch, config, palace_path, seeded_kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, seeded_kg)
+        _patch_mcp_server(monkeypatch, config, seeded_kg)
         from mempalace.mcp_server import tool_kg_invalidate
 
         result = tool_kg_invalidate(
@@ -293,14 +297,14 @@ class TestKGTools:
         assert result["success"] is True
 
     def test_kg_timeline(self, monkeypatch, config, palace_path, seeded_kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, seeded_kg)
+        _patch_mcp_server(monkeypatch, config, seeded_kg)
         from mempalace.mcp_server import tool_kg_timeline
 
         result = tool_kg_timeline(entity="Alice")
         assert result["count"] > 0
 
     def test_kg_stats(self, monkeypatch, config, palace_path, seeded_kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, seeded_kg)
+        _patch_mcp_server(monkeypatch, config, seeded_kg)
         from mempalace.mcp_server import tool_kg_stats
 
         result = tool_kg_stats()
@@ -312,8 +316,9 @@ class TestKGTools:
 
 class TestDiaryTools:
     def test_diary_write_and_read(self, monkeypatch, config, palace_path, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
-        _get_collection(palace_path, create=True)
+        _patch_mcp_server(monkeypatch, config, kg)
+        _client, _col = _get_collection(palace_path, create=True)
+        del _client
         from mempalace.mcp_server import tool_diary_write, tool_diary_read
 
         w = tool_diary_write(
@@ -330,8 +335,9 @@ class TestDiaryTools:
         assert "authentication" in r["entries"][0]["content"]
 
     def test_diary_read_empty(self, monkeypatch, config, palace_path, kg):
-        _patch_mcp_server(monkeypatch, config, palace_path, kg)
-        _get_collection(palace_path, create=True)
+        _patch_mcp_server(monkeypatch, config, kg)
+        _client, _col = _get_collection(palace_path, create=True)
+        del _client
         from mempalace.mcp_server import tool_diary_read
 
         r = tool_diary_read(agent_name="Nobody")

tests/test_searcher.py

@@ -30,8 +30,8 @@ class TestSearchMemories:
         result = search_memories("code", palace_path, n_results=2)
         assert len(result["results"]) <= 2
 
-    def test_no_palace_returns_error(self):
-        result = search_memories("anything", "/nonexistent/path")
+    def test_no_palace_returns_error(self, tmp_path):
+        result = search_memories("anything", str(tmp_path / "missing"))
         assert "error" in result
 
     def test_result_fields(self, palace_path, seeded_collection):