Skip to content

Commit 09d2ca6

Browse files
jpheinclaude
andauthored
fix(hooks): route mining through daemon when PALACE_DAEMON_URL is set (#2)
* fix(hooks): route mining through daemon when PALACE_DAEMON_URL is set Daemon-strict mode (introduced 2026-04-24 in commits 8c90c0f + 0e97b19 to fix the HNSW drift incident) skipped all three local mining paths when PALACE_DAEMON_URL was set, on the assumption a daemon-side writer would do the work instead. The diary checkpoint half got that writer via /silent-save, but the transcript-ingest half did not — so for ~11 days every Claude Code session left a checkpoint summary in the recovery collection and zero verbatim transcript drawers in mempalace_drawers. mempalace_search lost visibility into recent sessions even though MCP, daemon, and HNSW were all healthy. Replace the three skip-and-bail branches with POSTs to the daemon's existing /mine endpoint via a new _post_daemon_mine() helper: * _maybe_auto_ingest (background project mine on Stop) * _mine_sync (synchronous project mine on PreCompact) * _ingest_transcript (transcript convo mine on Stop / PreCompact) The daemon already has /mine; this PR just wires the hook to call it. Path translation (so the daemon can find client-side paths in its own filesystem) is handled daemon-side via PALACE_DAEMON_PATH_MAP — see the companion palace-daemon PR. Behavior change: transcript ingest now routes to the project wing derived via _wing_from_transcript_path(), matching the diary checkpoint behavior at hook_stop:740. Previously hardcoded "sessions"; now produces e.g. "wing_memorypalace" / "wing_realmwatch" per transcript. The "sessions" wing remains as the fallback for paths that don't match the Claude Code project layout (now spelled "wing_sessions" by the existing helper). Tests: * 5 existing tests now use clear=True on patch.dict so PALACE_DAEMON_URL in the developer env stops leaking into local-spawn assertions. * 6 new tests cover _post_daemon_mine (URL/body/api-key/error-paths) and the daemon-routed branches in all three mining functions. * Full suite green: 1552 passed, 1 skipped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(hooks): address Copilot review on #2 Three findings from Copilot's review pass plus one CI lint fix: 1. **Wing derivation parity** (Copilot L330, L354). _maybe_auto_ingest and _mine_sync hardcoded wing="general" in the daemon-routed branch, but the local-spawn fallback omits --wing so convo_miner derives from Path(mine_dir).name. The two paths produced different wings for identical input. Add _wing_from_mine_dir() that mirrors normalize_wing_name(Path(mine_dir).name) and use it in both daemon-routed branches. 2. **Reuse transcript validator** (Copilot L597). _ingest_transcript bypassed _validate_transcript_path, accepting any harness-supplied .jsonl path including ".." traversal sequences. _count_human_messages already validates; mirror that here so the same traversal/extension guards apply on the ingest path. 3. **Timeout adequacy**. Daemon /mine awaits proc.communicate() (blocks until subprocess finishes). 10s was too short for typical mines; bump to 30s. The hook-side timeout is cosmetic — even on timeout the daemon-side mine completes since the subprocess is independent of the HTTP connection — but a misleading log entry still reads like a failure. 4. **Ruff format**. CI's `ruff format --check .` flagged blank-line gaps inside the new test class _FakeResp. Auto-formatted. New tests: - test_ingest_transcript_rejects_traversal — closes the security finding - test_ingest_transcript_rejects_wrong_extension — same guard family - test_wing_from_mine_dir_normalizes — covers the new helper - test_maybe_auto_ingest_routes_through_daemon updated to assert wing derivation (project dirname "my-project" → "my_project") - test_mine_sync_routes_through_daemon updated likewise 82 passed, 1 skipped. Ruff format + lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5b8e867 commit 09d2ca6

2 files changed

Lines changed: 260 additions & 20 deletions

File tree

mempalace/hooks_cli.py

Lines changed: 80 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,57 @@ def _daemon_strict() -> bool:
278278
)
279279

280280

281+
def _post_daemon_mine(directory: str, wing: str, mode: str = "convos") -> bool:
282+
"""POST a /mine request to palace-daemon. Returns True on accepted job, False on error.
283+
284+
The hook sends client-side absolute paths (e.g. ``/home/<user>/.claude/projects/...``);
285+
the daemon translates them to its own filesystem layout via its
286+
``PALACE_DAEMON_PATH_MAP`` env var. Failures are logged and swallowed —
287+
a missed mine is not worth crashing a hook over. Note: the daemon's
288+
/mine endpoint currently blocks until the mine subprocess finishes,
289+
so the timeout is sized for typical workloads rather than network
290+
round-trip; on a real mine that exceeds it, the hook gets a stale
291+
timeout log but the daemon-side work still completes.
292+
"""
293+
daemon_url = os.environ.get("PALACE_DAEMON_URL", "").strip().rstrip("/")
294+
if not daemon_url:
295+
return False
296+
try:
297+
import urllib.request
298+
299+
req = urllib.request.Request(
300+
f"{daemon_url}/mine",
301+
data=json.dumps({"dir": directory, "wing": wing, "mode": mode}).encode("utf-8"),
302+
headers={"content-type": "application/json"},
303+
method="POST",
304+
)
305+
api_key = os.environ.get("PALACE_API_KEY", "").strip()
306+
if api_key:
307+
req.add_header("x-api-key", api_key)
308+
with urllib.request.urlopen(req, timeout=30) as resp:
309+
body = resp.read().decode("utf-8", errors="replace")
310+
_log(f"Daemon mine accepted: dir={directory} wing={wing} mode={mode} resp={body[:200]}")
311+
return True
312+
except Exception as e:
313+
_log(f"Daemon mine failed (dir={directory} wing={wing}): {e}")
314+
return False
315+
316+
317+
def _wing_from_mine_dir(mine_dir: str) -> str:
318+
"""Derive a wing name from a mine target directory, matching local-spawn semantics.
319+
320+
The local ``mempalace mine <dir> --mode projects`` invocation does not
321+
pass ``--wing``, so ``convo_miner`` / ``miner`` derive the wing from
322+
the directory's basename via ``normalize_wing_name``. Mirror that
323+
here so daemon-routed and local-spawn paths produce the same wing
324+
for the same input — Copilot review on jphein/mempalace#2 caught
325+
a hardcoded ``"general"`` here that diverged from local behavior.
326+
"""
327+
from .config import normalize_wing_name
328+
329+
return normalize_wing_name(Path(mine_dir).name)
330+
331+
281332
def _maybe_auto_ingest():
282333
"""Background-mine MEMPAL_DIR (project files) if set.
283334
@@ -286,12 +337,13 @@ def _maybe_auto_ingest():
286337
asymmetric interpreter handling and PID-file overwrite when both
287338
targets fire from a single hook call (#1231 review).
288339
"""
289-
if _daemon_strict():
290-
_log("Skipping auto-ingest: PALACE_DAEMON_URL set, daemon owns writes")
291-
return
292340
targets = _get_mine_targets()
293341
if not targets:
294342
return
343+
if _daemon_strict():
344+
for mine_dir, mode in targets:
345+
_post_daemon_mine(mine_dir, wing=_wing_from_mine_dir(mine_dir), mode=mode)
346+
return
295347
if _mine_already_running():
296348
_log("Skipping auto-ingest: mine already running")
297349
return
@@ -309,12 +361,13 @@ def _mine_sync():
309361
in ``hook_precompact`` — keeping them out of this function avoids
310362
timeout stacking against the harness 30s ceiling (#1231 review).
311363
"""
312-
if _daemon_strict():
313-
_log("Skipping sync mine: PALACE_DAEMON_URL set, daemon owns writes")
314-
return
315364
targets = _get_mine_targets()
316365
if not targets:
317366
return
367+
if _daemon_strict():
368+
for mine_dir, mode in targets:
369+
_post_daemon_mine(mine_dir, wing=_wing_from_mine_dir(mine_dir), mode=mode)
370+
return
318371
STATE_DIR.mkdir(parents=True, exist_ok=True)
319372
log_path = STATE_DIR / "hook.log"
320373
for mine_dir, mode in targets:
@@ -543,14 +596,30 @@ def _save_diary_direct(
543596

544597

545598
def _ingest_transcript(transcript_path: str):
546-
"""Mine a Claude Code session transcript into the palace as a conversation."""
547-
if _daemon_strict():
548-
_log("Skipping transcript ingest: PALACE_DAEMON_URL set, daemon owns writes")
599+
"""Mine a Claude Code session transcript into the palace as a conversation.
600+
601+
When ``PALACE_DAEMON_URL`` is set, route the mine through the daemon's
602+
``/mine`` endpoint (so the daemon stays the single writer). Otherwise
603+
fall back to spawning ``mempalace mine`` locally.
604+
605+
``transcript_path`` arrives from harness-supplied JSON, so reuse the
606+
same traversal/extension guards ``_count_human_messages`` already
607+
applies via ``_validate_transcript_path``.
608+
"""
609+
path = _validate_transcript_path(transcript_path)
610+
if path is None:
611+
if transcript_path:
612+
_log(f"WARNING: transcript ingest rejected by validator: {transcript_path!r}")
549613
return
550-
path = Path(transcript_path).expanduser()
551614
if not path.is_file() or path.stat().st_size < 100:
552615
return
553616

617+
project_wing = _wing_from_transcript_path(transcript_path)
618+
619+
if _daemon_strict():
620+
_post_daemon_mine(str(path.parent), wing=project_wing, mode="convos")
621+
return
622+
554623
from .config import MempalaceConfig
555624

556625
try:
@@ -572,7 +641,7 @@ def _ingest_transcript(transcript_path: str):
572641
"--mode",
573642
"convos",
574643
"--wing",
575-
"sessions",
644+
project_wing,
576645
],
577646
stdout=log_f,
578647
stderr=log_f,

0 commit comments

Comments
 (0)