Merge pull request #1230 from MemPalace/fix/hooks-convos-mode-1232

fix(hooks): pass --mode convos when mining Claude Code transcript dirs

mempalace/hooks_cli.py

@@ -197,16 +197,27 @@ def _output(data: dict):
     sys.stdout.buffer.flush()
 
 
-def _get_mine_dir(transcript_path: str = "") -> str:
-    """Determine directory to mine from MEMPAL_DIR or transcript path."""
+def _get_mine_dir(transcript_path: str = "") -> tuple[str, str]:
+    """Determine directory to mine and the miner mode to use.
+
+    Returns ``(dir, mode)`` where ``mode`` is ``"projects"`` or ``"convos"``.
+    Empty ``dir`` means no ingest should run.
+
+    MEMPAL_DIR is treated as a project directory ("projects" mode). The
+    transcript-path fallback resolves to the parent of a Claude Code
+    session JSONL, which must be mined with the conversation miner —
+    running the projects miner there ingests JSONL as if it were source
+    code.
+    """
     mempal_dir = os.environ.get("MEMPAL_DIR", "")
-    if mempal_dir and os.path.isdir(mempal_dir):
-        return mempal_dir
-    if transcript_path:
-        path = Path(transcript_path).expanduser()
-        if path.is_file():
-            return str(path.parent)
-    return ""
+    if mempal_dir:
+        resolved = Path(mempal_dir).expanduser().resolve()
+        if resolved.is_dir():
+            return str(resolved), "projects"
+    path = _validate_transcript_path(transcript_path)
+    if path is not None and path.is_file():
+        return str(path.parent), "convos"
+    return "", "projects"
 
 
 _MINE_PID_FILE = STATE_DIR / "mine.pid"
@@ -265,21 +276,21 @@ def _spawn_mine(cmd: list) -> None:
 
 def _maybe_auto_ingest(transcript_path: str = ""):
     """Run mempalace mine in background if a mine directory is available."""
-    mine_dir = _get_mine_dir(transcript_path)
+    mine_dir, mode = _get_mine_dir(transcript_path)
     if not mine_dir:
         return
     if _mine_already_running():
         _log("Skipping auto-ingest: mine already running")
         return
     try:
-        _spawn_mine([sys.executable, "-m", "mempalace", "mine", mine_dir])
+        _spawn_mine([sys.executable, "-m", "mempalace", "mine", mine_dir, "--mode", mode])
     except OSError:
         pass
 
 
 def _mine_sync(transcript_path: str = ""):
     """Run mempalace mine synchronously (for precompact -- data must land first)."""
-    mine_dir = _get_mine_dir(transcript_path)
+    mine_dir, mode = _get_mine_dir(transcript_path)
     if not mine_dir:
         return
     try:
@@ -287,7 +298,7 @@ def _mine_sync(transcript_path: str = ""):
         log_path = STATE_DIR / "hook.log"
         with open(log_path, "a") as log_f:
             subprocess.run(
-                [sys.executable, "-m", "mempalace", "mine", mine_dir],
+                [sys.executable, "-m", "mempalace", "mine", mine_dir, "--mode", mode],
                 stdout=log_f,
                 stderr=log_f,
                 timeout=60,

tests/test_hooks_cli.py

@@ -17,6 +17,7 @@ from mempalace.hooks_cli import (
     _maybe_auto_ingest,
     _mempalace_python,
     _mine_already_running,
+    _mine_sync,
     _parse_harness_input,
     _sanitize_session_id,
     _validate_transcript_path,
@@ -434,7 +435,7 @@ def test_maybe_auto_ingest_no_env(tmp_path):
 
 
 def test_maybe_auto_ingest_with_env(tmp_path):
-    """With MEMPAL_DIR set to a valid directory, spawns subprocess."""
+    """With MEMPAL_DIR set, spawns mine in projects mode against that dir."""
     mempal_dir = tmp_path / "project"
     mempal_dir.mkdir()
     with patch.dict("os.environ", {"MEMPAL_DIR": str(mempal_dir)}):
@@ -443,10 +444,14 @@ def test_maybe_auto_ingest_with_env(tmp_path):
                 with patch("mempalace.hooks_cli.subprocess.Popen") as mock_popen:
                     _maybe_auto_ingest()
                     mock_popen.assert_called_once()
+                    cmd = mock_popen.call_args[0][0]
+                    assert "mine" in cmd
+                    assert str(mempal_dir.resolve()) in cmd
+                    assert cmd[cmd.index("--mode") + 1] == "projects"
 
 
 def test_maybe_auto_ingest_with_transcript(tmp_path):
-    """Falls back to transcript directory when MEMPAL_DIR is not set."""
+    """Transcript fallback spawns mine in convos mode against the JSONL parent."""
     transcript = tmp_path / "t.jsonl"
     transcript.write_text("")
     with patch.dict("os.environ", {}, clear=True):
@@ -455,6 +460,38 @@ def test_maybe_auto_ingest_with_transcript(tmp_path):
                 with patch("mempalace.hooks_cli.subprocess.Popen") as mock_popen:
                     _maybe_auto_ingest(str(transcript))
                     mock_popen.assert_called_once()
+                    cmd = mock_popen.call_args[0][0]
+                    assert "mine" in cmd
+                    assert str(tmp_path.resolve()) in cmd
+                    assert cmd[cmd.index("--mode") + 1] == "convos"
+
+
+def test_mine_sync_with_transcript_uses_convos_mode(tmp_path):
+    """Precompact sync path also picks convos mode for JSONL transcripts."""
+    transcript = tmp_path / "t.jsonl"
+    transcript.write_text("")
+    with patch.dict("os.environ", {}, clear=True):
+        with patch("mempalace.hooks_cli.STATE_DIR", tmp_path):
+            with patch("mempalace.hooks_cli.subprocess.run") as mock_run:
+                _mine_sync(str(transcript))
+                mock_run.assert_called_once()
+                cmd = mock_run.call_args[0][0]
+                assert "mine" in cmd
+                assert str(tmp_path.resolve()) in cmd
+                assert cmd[cmd.index("--mode") + 1] == "convos"
+
+
+def test_mine_sync_with_env_uses_projects_mode(tmp_path):
+    """Precompact sync path uses projects mode when MEMPAL_DIR is set."""
+    mempal_dir = tmp_path / "project"
+    mempal_dir.mkdir()
+    with patch.dict("os.environ", {"MEMPAL_DIR": str(mempal_dir)}):
+        with patch("mempalace.hooks_cli.STATE_DIR", tmp_path):
+            with patch("mempalace.hooks_cli.subprocess.run") as mock_run:
+                _mine_sync()
+                mock_run.assert_called_once()
+                cmd = mock_run.call_args[0][0]
+                assert cmd[cmd.index("--mode") + 1] == "projects"
 
 
 def test_maybe_auto_ingest_oserror(tmp_path):
@@ -517,27 +554,66 @@ def test_mine_already_running_corrupt_file(tmp_path):
 
 
 def test_get_mine_dir_mempal_dir(tmp_path):
-    """MEMPAL_DIR takes priority over transcript_path."""
+    """MEMPAL_DIR takes priority, is expanded/resolved, and is treated as projects mode."""
     mempal_dir = tmp_path / "project"
     mempal_dir.mkdir()
     transcript = tmp_path / "t.jsonl"
     transcript.write_text("")
     with patch.dict("os.environ", {"MEMPAL_DIR": str(mempal_dir)}):
-        assert _get_mine_dir(str(transcript)) == str(mempal_dir)
+        result_dir, result_mode = _get_mine_dir(str(transcript))
+        assert Path(result_dir).resolve() == mempal_dir.resolve()
+        assert result_mode == "projects"
+
+
+def test_get_mine_dir_mempal_dir_tilde(tmp_path):
+    """MEMPAL_DIR with a tilde prefix is expanded correctly."""
+    mempal_dir = tmp_path / "project"
+    mempal_dir.mkdir()
+    home = Path.home()
+    # Build a ~-relative path only if tmp_path is inside home
+    try:
+        rel = mempal_dir.relative_to(home)
+    except ValueError:
+        pytest.skip("tmp_path is not under home, cannot build ~-relative path")
+    tilde_path = "~/" + str(rel)
+    with patch.dict("os.environ", {"MEMPAL_DIR": tilde_path}):
+        result_dir, result_mode = _get_mine_dir("")
+        assert Path(result_dir).resolve() == mempal_dir.resolve()
+        assert result_mode == "projects"
 
 
 def test_get_mine_dir_transcript_fallback(tmp_path):
-    """Falls back to transcript parent dir when MEMPAL_DIR is not set."""
+    """Transcript fallback resolves to its parent dir in convos mode."""
     transcript = tmp_path / "t.jsonl"
     transcript.write_text("")
     with patch.dict("os.environ", {}, clear=True):
-        assert _get_mine_dir(str(transcript)) == str(tmp_path)
+        result_dir, result_mode = _get_mine_dir(str(transcript))
+        assert Path(result_dir).resolve() == tmp_path.resolve()
+        assert result_mode == "convos"
+
+
+def test_get_mine_dir_transcript_path_traversal_rejected(tmp_path):
+    """Transcript paths with '..' components are rejected and return no dir."""
+    with patch.dict("os.environ", {}, clear=True):
+        result_dir, result_mode = _get_mine_dir("../../etc/passwd")
+        assert result_dir == ""
+        assert result_mode == "projects"
+
+
+def test_get_mine_dir_transcript_non_jsonl_rejected(tmp_path):
+    """Transcript paths without .jsonl/.json extension are rejected."""
+    bad = tmp_path / "notes.txt"
+    bad.write_text("content")
+    with patch.dict("os.environ", {}, clear=True):
+        result_dir, result_mode = _get_mine_dir(str(bad))
+        assert result_dir == ""
+        assert result_mode == "projects"
 
 
 def test_get_mine_dir_empty():
-    """Returns empty string when nothing is available."""
+    """Returns empty dir when nothing is available."""
     with patch.dict("os.environ", {}, clear=True):
-        assert _get_mine_dir("") == ""
+        assert _get_mine_dir("") == ("", "projects")
 
 
 # --- _parse_harness_input ---
@@ -669,9 +745,10 @@ def test_precompact_mines_transcript_dir(tmp_path, monkeypatch):
         )
     assert result == {}
     mock_run.assert_called_once()
-    # Verify mine dir is the transcript's parent
+    # Mine dir is the transcript's parent (resolved) and mode is convos for JSONL.
     call_args = mock_run.call_args[0][0]
-    assert str(tmp_path) in call_args[-1]
+    assert str(tmp_path.resolve()) in call_args
+    assert call_args[call_args.index("--mode") + 1] == "convos"
 
 
 # --- run_hook ---