Merge pull request #2030 from fatkobra/feat/1963-hook-write-routing
feat(hooks): apply shared daemon write-routing policy
This commit is contained in:
Igor Lins e Silva
GitHub
commit
f852ef98fa
|
|
@ -0,0 +1,112 @@
|
|||
# Hook write routing
|
||||
|
||||
Hook-triggered writes use the shared write-routing policy introduced for the
|
||||
Tier 3 rollout tracked in #1963.
|
||||
|
||||
## Scope
|
||||
|
||||
This applies to every routine write initiated by the Python hook layer:
|
||||
|
||||
- Stop-hook diary checkpoints;
|
||||
- transcript/conversation ingest;
|
||||
- project auto-ingest;
|
||||
- SessionEnd final flushes;
|
||||
- PreCompact transcript ingest;
|
||||
- PreCompact synchronous project mining.
|
||||
|
||||
It does not change CLI write routing. CLI adoption is a separate follow-up.
|
||||
|
||||
## Policy behavior
|
||||
|
||||
### `direct`
|
||||
|
||||
Hooks use the existing direct in-process or subprocess paths.
|
||||
|
||||
The daemon is not probed.
|
||||
|
||||
### `prefer`
|
||||
|
||||
Hooks use the daemon when it is already healthy.
|
||||
|
||||
If the daemon is unavailable, hooks retain the historical direct fallback.
|
||||
|
||||
### `require`
|
||||
|
||||
Hooks use the daemon when it is already healthy.
|
||||
|
||||
If the daemon is unavailable:
|
||||
|
||||
- no in-process ChromaDB write runs;
|
||||
- no direct `mempalace mine` subprocess is started;
|
||||
- no daemon is cold-started from the hook;
|
||||
- the hook log records the skipped operation;
|
||||
- the hook returns a visible `systemMessage`;
|
||||
- the Stop save marker is not advanced, allowing a later retry.
|
||||
|
||||
## Why hooks do not start the daemon
|
||||
|
||||
Hooks operate under strict latency budgets. Starting a Python daemon and its
|
||||
storage dependencies from a Stop or SessionEnd hook can exceed that budget.
|
||||
|
||||
A supervised installation using `require` must start the daemon earlier, for
|
||||
example at login, plugin initialization, or session setup:
|
||||
|
||||
mempalace daemon start
|
||||
|
||||
SessionStart performs a fast health probe in `require` mode and warns early if
|
||||
the required daemon is unavailable.
|
||||
|
||||
## One decision per hook event
|
||||
|
||||
A Stop or SessionEnd event may perform several writes:
|
||||
|
||||
1. diary checkpoint;
|
||||
2. transcript ingest;
|
||||
3. project auto-ingest.
|
||||
|
||||
The route is resolved once and stored in a context-local value for the whole
|
||||
write burst. This avoids repeated health probes and prevents different writes
|
||||
from selecting inconsistent routes during the same event.
|
||||
|
||||
## Submission ambiguity
|
||||
|
||||
Once a daemon submission is attempted, an error never triggers direct
|
||||
fallback. The daemon may have accepted the job before the client observed the
|
||||
failure; retrying directly could duplicate content.
|
||||
|
||||
## Invalid configuration
|
||||
|
||||
An explicitly invalid routing policy fails closed: hook writes are blocked and
|
||||
no direct ChromaDB fallback is attempted.
|
||||
|
||||
An unrelated configuration read/runtime failure preserves the historical
|
||||
direct-save behavior so a final checkpoint is not lost because of an
|
||||
independent configuration failure.
|
||||
|
||||
## Backward compatibility
|
||||
|
||||
The default policy remains `direct`.
|
||||
|
||||
Legacy settings remain supported through the shared policy resolver:
|
||||
|
||||
- `MEMPALACE_HOOKS_DAEMON=true` maps to `prefer`;
|
||||
- `hooks.daemon: true` maps to `prefer`;
|
||||
- false values map to `direct`.
|
||||
|
||||
## Configuration examples
|
||||
|
||||
Prefer the daemon but permit direct fallback:
|
||||
|
||||
MEMPALACE_HOOK_WRITE_ROUTING=prefer
|
||||
|
||||
Require the daemon and prohibit direct writers:
|
||||
|
||||
MEMPALACE_HOOK_WRITE_ROUTING=require
|
||||
|
||||
Configuration file:
|
||||
|
||||
{
|
||||
"write_routing": {
|
||||
"hooks": "require"
|
||||
}
|
||||
}
|
||||
|
|
@ -111,9 +111,10 @@ direct write would violate the safety purpose of the policy.
|
|||
|
||||
## Follow-up PRs
|
||||
|
||||
PR 2 will apply the policy to hook-triggered writes.
|
||||
Hook-triggered writes now consume this policy; see
|
||||
`docs/hook-write-routing.md`.
|
||||
|
||||
PR 3 will apply the policy to routine CLI writes.
|
||||
The remaining rollout PR will apply the policy to routine CLI writes.
|
||||
|
||||
Maintenance operations such as repair, migration, and index rebuild are not
|
||||
ordinary routed writes. They require a separate exclusive-maintenance policy.
|
||||
|
|
|
|||
|
|
@ -7,6 +7,9 @@ Supported harnesses: claude-code, codex (extensible to cursor, gemini, etc.)
|
|||
"""
|
||||
|
||||
import hashlib
|
||||
from contextlib import contextmanager
|
||||
from contextvars import ContextVar
|
||||
from dataclasses import dataclass
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
|
|
@ -18,6 +21,13 @@ from pathlib import Path
|
|||
from typing import Optional
|
||||
|
||||
from mempalace.config import MempalaceConfig
|
||||
from mempalace.write_routing import (
|
||||
ResolvedWriteRoutingPolicy,
|
||||
WriteRoutingDecision,
|
||||
WriteRoutingError,
|
||||
WriteRoutingPolicy,
|
||||
choose_write_route,
|
||||
)
|
||||
|
||||
SAVE_INTERVAL = 15
|
||||
STATE_DIR = Path.home() / ".mempalace" / "hook_state"
|
||||
|
|
@ -510,6 +520,11 @@ def _spawn_mine(cmd: list) -> None:
|
|||
|
||||
|
||||
def _hooks_daemon_enabled() -> bool:
|
||||
"""Legacy compatibility helper for the pre-policy hook setting.
|
||||
|
||||
New hook write paths use ``resolve_write_routing("hooks")``. This
|
||||
helper remains for callers/tests that still inspect ``hooks.daemon``.
|
||||
"""
|
||||
try:
|
||||
return MempalaceConfig().hook_use_daemon is True
|
||||
except Exception:
|
||||
|
|
@ -527,11 +542,9 @@ def _daemon_mine_dedupe_key(source: str, mode: str) -> str:
|
|||
def _daemon_available() -> bool:
|
||||
"""True iff a daemon is already running for the configured palace.
|
||||
|
||||
This is a fast localhost health check, not a spawn: the 500ms hook budget
|
||||
forbids auto-starting a python subprocess from a hook (cold start is
|
||||
~15s). Daemon mode for hooks requires the user to have started the daemon
|
||||
explicitly via `mempalace daemon start`; when it isn't up, hooks fall back
|
||||
to the existing direct (in-process / spawn) path instead of blocking.
|
||||
This is a fast localhost health check, not a spawn: the hook time budget
|
||||
forbids cold-starting a long-lived daemon. ``prefer`` may fall back to the
|
||||
direct path when this returns false; ``require`` must block the write.
|
||||
"""
|
||||
from .daemon import HOOK_PROBE_TIMEOUT, get_client_if_running
|
||||
|
||||
|
|
@ -544,6 +557,143 @@ def _daemon_available() -> bool:
|
|||
return False
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class HookWriteRouting:
|
||||
"""One hook invocation's resolved routing state."""
|
||||
|
||||
decision: Optional[WriteRoutingDecision]
|
||||
source: str
|
||||
error: Optional[str] = None
|
||||
|
||||
@property
|
||||
def use_daemon(self) -> bool:
|
||||
return self.decision is not None and self.decision.use_daemon
|
||||
|
||||
@property
|
||||
def blocked(self) -> bool:
|
||||
return self.error is not None or (self.decision is not None and self.decision.blocked)
|
||||
|
||||
@property
|
||||
def notice(self) -> str:
|
||||
if self.error is not None:
|
||||
return (
|
||||
"MemPalace hook writes were skipped because write-routing "
|
||||
f"configuration is invalid: {self.error}. No direct ChromaDB "
|
||||
"fallback was attempted."
|
||||
)
|
||||
if self.blocked:
|
||||
return (
|
||||
"MemPalace hook writes were skipped because routing is set to "
|
||||
"'require' but the local daemon is unavailable. Start it with "
|
||||
"`mempalace daemon start`; no direct ChromaDB fallback was attempted."
|
||||
)
|
||||
return ""
|
||||
|
||||
|
||||
_HOOK_WRITE_ROUTING_CONTEXT = ContextVar(
|
||||
"mempalace_hook_write_routing",
|
||||
default=None,
|
||||
)
|
||||
|
||||
|
||||
def _resolve_configured_hook_policy() -> ResolvedWriteRoutingPolicy:
|
||||
"""Resolve the new policy, with legacy-object compatibility."""
|
||||
|
||||
config = MempalaceConfig()
|
||||
resolver = getattr(config, "resolve_write_routing", None)
|
||||
if callable(resolver):
|
||||
resolved = resolver("hooks")
|
||||
if isinstance(resolved, ResolvedWriteRoutingPolicy):
|
||||
return resolved
|
||||
|
||||
# Compatibility for older/custom config objects and existing tests that
|
||||
# expose only the pre-policy ``hook_use_daemon`` property.
|
||||
policy = (
|
||||
WriteRoutingPolicy.PREFER
|
||||
if getattr(config, "hook_use_daemon", False) is True
|
||||
else WriteRoutingPolicy.DIRECT
|
||||
)
|
||||
return ResolvedWriteRoutingPolicy(
|
||||
policy=policy,
|
||||
source="legacy hook_use_daemon",
|
||||
)
|
||||
|
||||
|
||||
def _compute_hook_write_routing() -> HookWriteRouting:
|
||||
"""Resolve hook policy and probe daemon liveness at most once."""
|
||||
|
||||
try:
|
||||
resolved = _resolve_configured_hook_policy()
|
||||
except WriteRoutingError as exc:
|
||||
routing = HookWriteRouting(
|
||||
decision=None,
|
||||
source="configuration-error",
|
||||
error=str(exc),
|
||||
)
|
||||
_log(routing.notice)
|
||||
return routing
|
||||
except Exception as exc:
|
||||
# Preserve the historical save-on-config-read-failure behavior. An
|
||||
# explicitly invalid routing value raises WriteRoutingError above and
|
||||
# fails closed; an unrelated config I/O/runtime failure falls back to
|
||||
# direct so a final checkpoint is not silently lost.
|
||||
_log(f"WARNING: could not resolve hook write routing: {exc}; defaulting to direct")
|
||||
resolved = ResolvedWriteRoutingPolicy(
|
||||
policy=WriteRoutingPolicy.DIRECT,
|
||||
source="config-unavailable fallback",
|
||||
)
|
||||
|
||||
daemon_available = False
|
||||
if resolved.policy is not WriteRoutingPolicy.DIRECT:
|
||||
daemon_available = _daemon_available()
|
||||
|
||||
decision = choose_write_route(
|
||||
resolved.policy,
|
||||
daemon_available=daemon_available,
|
||||
daemon_can_start=False,
|
||||
)
|
||||
routing = HookWriteRouting(
|
||||
decision=decision,
|
||||
source=resolved.source,
|
||||
)
|
||||
|
||||
if decision.policy is not WriteRoutingPolicy.DIRECT:
|
||||
_log(
|
||||
"Hook write routing: "
|
||||
f"policy={decision.policy.value} source={resolved.source} "
|
||||
f"target={decision.target.value} reason={decision.reason}"
|
||||
)
|
||||
|
||||
return routing
|
||||
|
||||
|
||||
def _current_hook_write_routing() -> HookWriteRouting:
|
||||
routing = _HOOK_WRITE_ROUTING_CONTEXT.get()
|
||||
if routing is not None:
|
||||
return routing
|
||||
return _compute_hook_write_routing()
|
||||
|
||||
|
||||
@contextmanager
|
||||
def _hook_write_routing_context():
|
||||
"""Share one policy resolution and one daemon probe across a hook fire."""
|
||||
|
||||
routing = _compute_hook_write_routing()
|
||||
token = _HOOK_WRITE_ROUTING_CONTEXT.set(routing)
|
||||
try:
|
||||
yield routing
|
||||
finally:
|
||||
_HOOK_WRITE_ROUTING_CONTEXT.reset(token)
|
||||
|
||||
|
||||
def _log_hook_write_blocked(routing: HookWriteRouting, operation: str) -> None:
|
||||
_log(f"{routing.notice} Operation skipped: {operation}.")
|
||||
|
||||
|
||||
def _blocked_hook_output(routing: HookWriteRouting) -> dict:
|
||||
return {"systemMessage": routing.notice}
|
||||
|
||||
|
||||
def _submit_daemon_job(
|
||||
kind: str,
|
||||
payload: dict,
|
||||
|
|
@ -592,9 +742,15 @@ def _maybe_auto_ingest():
|
|||
targets = _get_mine_targets()
|
||||
if not targets:
|
||||
return
|
||||
|
||||
routing = _current_hook_write_routing()
|
||||
if routing.blocked:
|
||||
_log_hook_write_blocked(routing, "project auto-ingest")
|
||||
return
|
||||
|
||||
for mine_dir, mode in targets:
|
||||
try:
|
||||
if _hooks_daemon_enabled() and _daemon_available():
|
||||
if routing.use_daemon:
|
||||
try:
|
||||
_submit_daemon_job(
|
||||
"mine",
|
||||
|
|
@ -626,11 +782,17 @@ def _mine_sync():
|
|||
targets = _get_mine_targets()
|
||||
if not targets:
|
||||
return
|
||||
|
||||
routing = _current_hook_write_routing()
|
||||
if routing.blocked:
|
||||
_log_hook_write_blocked(routing, "synchronous project mine")
|
||||
return
|
||||
|
||||
STATE_DIR.mkdir(parents=True, exist_ok=True)
|
||||
log_path = STATE_DIR / "hook.log"
|
||||
for mine_dir, mode in targets:
|
||||
try:
|
||||
if _hooks_daemon_enabled() and _daemon_available():
|
||||
if routing.use_daemon:
|
||||
try:
|
||||
job = _submit_daemon_job(
|
||||
"mine",
|
||||
|
|
@ -779,6 +941,15 @@ def _save_diary_direct(
|
|||
_log("No recent messages to save")
|
||||
return {"count": 0}
|
||||
|
||||
routing = _current_hook_write_routing()
|
||||
if routing.blocked:
|
||||
_log_hook_write_blocked(routing, "diary checkpoint")
|
||||
return {
|
||||
"count": 0,
|
||||
"routing_blocked": True,
|
||||
"routing_message": routing.notice,
|
||||
}
|
||||
|
||||
themes = _extract_themes(messages)
|
||||
|
||||
# Build a compressed diary entry from recent conversation
|
||||
|
|
@ -790,7 +961,7 @@ def _save_diary_direct(
|
|||
)
|
||||
|
||||
try:
|
||||
if _hooks_daemon_enabled() and _daemon_available():
|
||||
if routing.use_daemon:
|
||||
try:
|
||||
job = _submit_daemon_job(
|
||||
"diary_write",
|
||||
|
|
@ -870,8 +1041,13 @@ def _ingest_transcript(transcript_path: str):
|
|||
except Exception:
|
||||
return
|
||||
|
||||
routing = _current_hook_write_routing()
|
||||
if routing.blocked:
|
||||
_log_hook_write_blocked(routing, "transcript ingest")
|
||||
return
|
||||
|
||||
try:
|
||||
if _hooks_daemon_enabled() and _daemon_available():
|
||||
if routing.use_daemon:
|
||||
try:
|
||||
_submit_daemon_job(
|
||||
"mine",
|
||||
|
|
@ -1132,64 +1308,70 @@ def hook_stop(data: dict, harness: str):
|
|||
_log(f"Session {session_id}: {exchange_count} exchanges, {since_last} since last save")
|
||||
|
||||
if since_last >= SAVE_INTERVAL and exchange_count > 0:
|
||||
_log(f"TRIGGERING SAVE at exchange {exchange_count}")
|
||||
with _hook_write_routing_context() as routing:
|
||||
if routing.blocked:
|
||||
_log_hook_write_blocked(routing, "stop-hook checkpoint")
|
||||
_output(_blocked_hook_output(routing))
|
||||
return
|
||||
|
||||
# Read hook settings from config
|
||||
try:
|
||||
config = MempalaceConfig()
|
||||
silent = config.hook_silent_save
|
||||
toast = config.hook_desktop_toast
|
||||
except Exception:
|
||||
silent = True
|
||||
toast = False
|
||||
_log(f"TRIGGERING SAVE at exchange {exchange_count}")
|
||||
|
||||
project_wing = _wing_from_transcript_path(transcript_path)
|
||||
# Read hook settings from config
|
||||
try:
|
||||
config = MempalaceConfig()
|
||||
silent = config.hook_silent_save
|
||||
toast = config.hook_desktop_toast
|
||||
except Exception:
|
||||
silent = True
|
||||
toast = False
|
||||
|
||||
if silent:
|
||||
# Save directly via Python API — systemMessage renders in terminal
|
||||
result = {"count": 0}
|
||||
if transcript_path:
|
||||
result = _save_diary_direct(
|
||||
transcript_path,
|
||||
session_id,
|
||||
wing=project_wing,
|
||||
toast=toast,
|
||||
agent_name=_diary_agent_for_harness(harness),
|
||||
)
|
||||
_ingest_transcript(transcript_path)
|
||||
_maybe_auto_ingest()
|
||||
# Only advance save marker after successful save
|
||||
count = result.get("count", 0)
|
||||
if count > 0:
|
||||
project_wing = _wing_from_transcript_path(transcript_path)
|
||||
|
||||
if silent:
|
||||
# Save directly via Python API — systemMessage renders in terminal
|
||||
result = {"count": 0}
|
||||
if transcript_path:
|
||||
result = _save_diary_direct(
|
||||
transcript_path,
|
||||
session_id,
|
||||
wing=project_wing,
|
||||
toast=toast,
|
||||
agent_name=_diary_agent_for_harness(harness),
|
||||
)
|
||||
_ingest_transcript(transcript_path)
|
||||
_maybe_auto_ingest()
|
||||
# Only advance save marker after successful save
|
||||
count = result.get("count", 0)
|
||||
if count > 0:
|
||||
try:
|
||||
last_save_file.write_text(str(exchange_count), encoding="utf-8")
|
||||
except OSError:
|
||||
pass
|
||||
themes = result.get("themes", [])
|
||||
if themes:
|
||||
tag = " \u2014 " + ", ".join(themes)
|
||||
else:
|
||||
tag = ""
|
||||
_output(
|
||||
{
|
||||
"systemMessage": f"\u2726 {count} memories woven into the palace{tag}",
|
||||
}
|
||||
)
|
||||
else:
|
||||
_output({})
|
||||
else:
|
||||
# Legacy: block and ask Claude to save via MCP tools.
|
||||
# Marker advances before confirmed save — best-effort; if Claude
|
||||
# fails to save, the checkpoint is lost but won't retry endlessly.
|
||||
try:
|
||||
last_save_file.write_text(str(exchange_count), encoding="utf-8")
|
||||
except OSError:
|
||||
pass
|
||||
themes = result.get("themes", [])
|
||||
if themes:
|
||||
tag = " \u2014 " + ", ".join(themes)
|
||||
else:
|
||||
tag = ""
|
||||
_output(
|
||||
{
|
||||
"systemMessage": f"\u2726 {count} memories woven into the palace{tag}",
|
||||
}
|
||||
)
|
||||
else:
|
||||
_output({})
|
||||
else:
|
||||
# Legacy: block and ask Claude to save via MCP tools.
|
||||
# Marker advances before confirmed save — best-effort; if Claude
|
||||
# fails to save, the checkpoint is lost but won't retry endlessly.
|
||||
try:
|
||||
last_save_file.write_text(str(exchange_count), encoding="utf-8")
|
||||
except OSError:
|
||||
pass
|
||||
if transcript_path:
|
||||
_ingest_transcript(transcript_path)
|
||||
_maybe_auto_ingest()
|
||||
reason = STOP_BLOCK_REASON + f" Write diary entry to wing={project_wing}."
|
||||
_output({"decision": "block", "reason": reason})
|
||||
if transcript_path:
|
||||
_ingest_transcript(transcript_path)
|
||||
_maybe_auto_ingest()
|
||||
reason = STOP_BLOCK_REASON + f" Write diary entry to wing={project_wing}."
|
||||
_output({"decision": "block", "reason": reason})
|
||||
else:
|
||||
_output({})
|
||||
|
||||
|
|
@ -1207,6 +1389,14 @@ def hook_session_start(data: dict, harness: str):
|
|||
# Initialize session state directory
|
||||
STATE_DIR.mkdir(parents=True, exist_ok=True)
|
||||
|
||||
# Surface a required-daemon problem at session start instead of waiting
|
||||
# until the first save is due. Hooks still never cold-start the daemon.
|
||||
with _hook_write_routing_context() as routing:
|
||||
if routing.blocked:
|
||||
_log_hook_write_blocked(routing, "session-start readiness check")
|
||||
_output(_blocked_hook_output(routing))
|
||||
return
|
||||
|
||||
# Pass through — no blocking on session start
|
||||
_output({})
|
||||
|
||||
|
|
@ -1305,16 +1495,22 @@ def hook_session_end(data: dict, harness: str):
|
|||
# short-circuit + upsert). ``reason`` is intentionally not branched on:
|
||||
# every clean-exit reason (incl. ``/clear`` / ``resume``) warrants the
|
||||
# flush. Order matches ``hook_stop``.
|
||||
if valid_transcript:
|
||||
_save_diary_direct(
|
||||
valid_transcript,
|
||||
session_id,
|
||||
wing=_wing_from_transcript_path(valid_transcript),
|
||||
toast=toast,
|
||||
agent_name=_diary_agent_for_harness(harness),
|
||||
)
|
||||
_ingest_transcript(valid_transcript)
|
||||
_maybe_auto_ingest()
|
||||
with _hook_write_routing_context() as routing:
|
||||
if routing.blocked:
|
||||
_log_hook_write_blocked(routing, "session-end flush")
|
||||
_output(_blocked_hook_output(routing))
|
||||
return
|
||||
|
||||
if valid_transcript:
|
||||
_save_diary_direct(
|
||||
valid_transcript,
|
||||
session_id,
|
||||
wing=_wing_from_transcript_path(valid_transcript),
|
||||
toast=toast,
|
||||
agent_name=_diary_agent_for_harness(harness),
|
||||
)
|
||||
_ingest_transcript(valid_transcript)
|
||||
_maybe_auto_ingest()
|
||||
|
||||
_output({})
|
||||
finally:
|
||||
|
|
@ -1341,14 +1537,20 @@ def hook_precompact(data: dict, harness: str):
|
|||
|
||||
_log(f"PRE-COMPACT triggered for session {session_id}")
|
||||
|
||||
# Capture tool output via our normalize path before compaction loses it
|
||||
if transcript_path:
|
||||
_ingest_transcript(transcript_path)
|
||||
with _hook_write_routing_context() as routing:
|
||||
if routing.blocked:
|
||||
_log_hook_write_blocked(routing, "precompact flush")
|
||||
_output(_blocked_hook_output(routing))
|
||||
return
|
||||
|
||||
# Mine MEMPAL_DIR synchronously so project data lands before
|
||||
# compaction proceeds. Transcript convos were already kicked off
|
||||
# above via _ingest_transcript.
|
||||
_mine_sync()
|
||||
# Capture tool output via our normalize path before compaction loses it
|
||||
if transcript_path:
|
||||
_ingest_transcript(transcript_path)
|
||||
|
||||
# Mine MEMPAL_DIR synchronously so project data lands before
|
||||
# compaction proceeds. Transcript convos were already kicked off
|
||||
# above via _ingest_transcript.
|
||||
_mine_sync()
|
||||
|
||||
_output({})
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,608 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import sys
|
||||
import types
|
||||
from pathlib import Path
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from mempalace import hooks_cli
|
||||
from mempalace.write_routing import (
|
||||
ResolvedWriteRoutingPolicy,
|
||||
WriteRoutingError,
|
||||
WriteRoutingPolicy,
|
||||
WriteRoutingTarget,
|
||||
)
|
||||
|
||||
|
||||
class _HookConfig:
|
||||
def __init__(
|
||||
self,
|
||||
policy: WriteRoutingPolicy = WriteRoutingPolicy.DIRECT,
|
||||
*,
|
||||
source: str = "test",
|
||||
routing_error: Exception | None = None,
|
||||
palace_path: str = "/tmp/palace",
|
||||
):
|
||||
self._policy = policy
|
||||
self._source = source
|
||||
self._routing_error = routing_error
|
||||
self.palace_path = palace_path
|
||||
self.hooks_auto_save = True
|
||||
self.hook_silent_save = True
|
||||
self.hook_desktop_toast = False
|
||||
|
||||
def resolve_write_routing(self, scope: str) -> ResolvedWriteRoutingPolicy:
|
||||
assert scope == "hooks"
|
||||
if self._routing_error is not None:
|
||||
raise self._routing_error
|
||||
return ResolvedWriteRoutingPolicy(
|
||||
policy=self._policy,
|
||||
source=self._source,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clear_hook_routing_context(monkeypatch):
|
||||
token = hooks_cli._HOOK_WRITE_ROUTING_CONTEXT.set(None)
|
||||
|
||||
for key in (
|
||||
"MEMPALACE_WRITE_ROUTING",
|
||||
"MEMPALACE_HOOK_WRITE_ROUTING",
|
||||
"MEMPALACE_HOOKS_DAEMON",
|
||||
):
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
hooks_cli._HOOK_WRITE_ROUTING_CONTEXT.reset(token)
|
||||
|
||||
|
||||
def _write_transcript(path: Path, count: int = 3) -> None:
|
||||
path.write_text(
|
||||
"".join(
|
||||
json.dumps(
|
||||
{
|
||||
"message": {
|
||||
"role": "user",
|
||||
"content": f"message {index}",
|
||||
}
|
||||
}
|
||||
)
|
||||
+ "\n"
|
||||
for index in range(count)
|
||||
),
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
|
||||
def _capture_output(callable_):
|
||||
captured = []
|
||||
|
||||
with patch(
|
||||
"mempalace.hooks_cli._output",
|
||||
side_effect=captured.append,
|
||||
):
|
||||
callable_()
|
||||
|
||||
assert captured
|
||||
return captured[-1]
|
||||
|
||||
|
||||
def test_direct_policy_does_not_probe_daemon():
|
||||
config = _HookConfig(WriteRoutingPolicy.DIRECT)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"mempalace.hooks_cli.MempalaceConfig",
|
||||
return_value=config,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._daemon_available",
|
||||
side_effect=AssertionError("direct must not probe daemon"),
|
||||
),
|
||||
):
|
||||
routing = hooks_cli._compute_hook_write_routing()
|
||||
|
||||
assert routing.decision is not None
|
||||
assert routing.decision.target is WriteRoutingTarget.DIRECT
|
||||
assert routing.blocked is False
|
||||
assert routing.use_daemon is False
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("policy", "available", "target"),
|
||||
[
|
||||
(
|
||||
WriteRoutingPolicy.PREFER,
|
||||
True,
|
||||
WriteRoutingTarget.DAEMON,
|
||||
),
|
||||
(
|
||||
WriteRoutingPolicy.PREFER,
|
||||
False,
|
||||
WriteRoutingTarget.DIRECT,
|
||||
),
|
||||
(
|
||||
WriteRoutingPolicy.REQUIRE,
|
||||
True,
|
||||
WriteRoutingTarget.DAEMON,
|
||||
),
|
||||
(
|
||||
WriteRoutingPolicy.REQUIRE,
|
||||
False,
|
||||
WriteRoutingTarget.BLOCKED,
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_hook_route_decision_matrix(policy, available, target):
|
||||
config = _HookConfig(policy)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"mempalace.hooks_cli.MempalaceConfig",
|
||||
return_value=config,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._daemon_available",
|
||||
return_value=available,
|
||||
),
|
||||
):
|
||||
routing = hooks_cli._compute_hook_write_routing()
|
||||
|
||||
assert routing.decision is not None
|
||||
assert routing.decision.target is target
|
||||
assert routing.use_daemon is (target is WriteRoutingTarget.DAEMON)
|
||||
assert routing.blocked is (target is WriteRoutingTarget.BLOCKED)
|
||||
|
||||
|
||||
def test_invalid_policy_blocks_instead_of_falling_back_direct():
|
||||
config = _HookConfig(
|
||||
routing_error=WriteRoutingError("bad hook policy"),
|
||||
)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"mempalace.hooks_cli.MempalaceConfig",
|
||||
return_value=config,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._daemon_available",
|
||||
) as probe,
|
||||
patch("mempalace.hooks_cli._log"),
|
||||
):
|
||||
routing = hooks_cli._compute_hook_write_routing()
|
||||
|
||||
probe.assert_not_called()
|
||||
assert routing.blocked is True
|
||||
assert routing.decision is None
|
||||
assert "invalid" in routing.notice
|
||||
assert "No direct ChromaDB fallback" in routing.notice
|
||||
|
||||
|
||||
def test_unrelated_config_failure_preserves_historical_direct_fallback():
|
||||
with (
|
||||
patch(
|
||||
"mempalace.hooks_cli.MempalaceConfig",
|
||||
side_effect=RuntimeError("config unreadable"),
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._daemon_available",
|
||||
side_effect=AssertionError("direct fallback must not probe daemon"),
|
||||
),
|
||||
patch("mempalace.hooks_cli._log") as log,
|
||||
):
|
||||
routing = hooks_cli._compute_hook_write_routing()
|
||||
|
||||
assert routing.decision is not None
|
||||
assert routing.decision.target is WriteRoutingTarget.DIRECT
|
||||
assert routing.source == "config-unavailable fallback"
|
||||
assert routing.blocked is False
|
||||
assert "defaulting to direct" in log.call_args.args[0]
|
||||
|
||||
|
||||
def test_context_reuses_one_daemon_probe_for_whole_hook_fire():
|
||||
config = _HookConfig(WriteRoutingPolicy.PREFER)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"mempalace.hooks_cli.MempalaceConfig",
|
||||
return_value=config,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._daemon_available",
|
||||
return_value=True,
|
||||
) as probe,
|
||||
patch("mempalace.hooks_cli._log"),
|
||||
):
|
||||
with hooks_cli._hook_write_routing_context() as routing:
|
||||
assert hooks_cli._current_hook_write_routing() is routing
|
||||
assert hooks_cli._current_hook_write_routing() is routing
|
||||
assert routing.use_daemon is True
|
||||
|
||||
probe.assert_called_once_with()
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("policy", "available", "expected"),
|
||||
[
|
||||
(WriteRoutingPolicy.DIRECT, False, "direct"),
|
||||
(WriteRoutingPolicy.PREFER, False, "direct"),
|
||||
(WriteRoutingPolicy.PREFER, True, "daemon"),
|
||||
(WriteRoutingPolicy.REQUIRE, False, "blocked"),
|
||||
(WriteRoutingPolicy.REQUIRE, True, "daemon"),
|
||||
],
|
||||
)
|
||||
def test_project_auto_ingest_applies_policy(
|
||||
policy,
|
||||
available,
|
||||
expected,
|
||||
):
|
||||
config = _HookConfig(policy)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"mempalace.hooks_cli.MempalaceConfig",
|
||||
return_value=config,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._get_mine_targets",
|
||||
return_value=[("/project", "projects")],
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._daemon_available",
|
||||
return_value=available,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._submit_daemon_job",
|
||||
) as submit,
|
||||
patch(
|
||||
"mempalace.hooks_cli._spawn_mine",
|
||||
) as spawn,
|
||||
patch("mempalace.hooks_cli._log"),
|
||||
):
|
||||
hooks_cli._maybe_auto_ingest()
|
||||
|
||||
if expected == "daemon":
|
||||
submit.assert_called_once()
|
||||
spawn.assert_not_called()
|
||||
elif expected == "direct":
|
||||
submit.assert_not_called()
|
||||
spawn.assert_called_once()
|
||||
else:
|
||||
submit.assert_not_called()
|
||||
spawn.assert_not_called()
|
||||
|
||||
|
||||
def test_require_unavailable_blocks_every_direct_hook_write_path(
|
||||
tmp_path,
|
||||
):
|
||||
config = _HookConfig(WriteRoutingPolicy.REQUIRE)
|
||||
transcript = tmp_path / "session.jsonl"
|
||||
_write_transcript(transcript)
|
||||
|
||||
fake_mcp = types.ModuleType("mempalace.mcp_server")
|
||||
fake_mcp.tool_diary_write = MagicMock()
|
||||
|
||||
with (
|
||||
patch(
|
||||
"mempalace.hooks_cli.MempalaceConfig",
|
||||
return_value=config,
|
||||
),
|
||||
patch("mempalace.hooks_cli.STATE_DIR", tmp_path),
|
||||
patch(
|
||||
"mempalace.hooks_cli._get_mine_targets",
|
||||
return_value=[("/project", "projects")],
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._daemon_available",
|
||||
return_value=False,
|
||||
) as probe,
|
||||
patch(
|
||||
"mempalace.hooks_cli._submit_daemon_job",
|
||||
) as submit,
|
||||
patch(
|
||||
"mempalace.hooks_cli._spawn_mine",
|
||||
) as spawn,
|
||||
patch(
|
||||
"mempalace.hooks_cli.subprocess.run",
|
||||
) as sync_run,
|
||||
patch("mempalace.hooks_cli._log"),
|
||||
patch.dict(
|
||||
sys.modules,
|
||||
{"mempalace.mcp_server": fake_mcp},
|
||||
),
|
||||
):
|
||||
with hooks_cli._hook_write_routing_context():
|
||||
hooks_cli._maybe_auto_ingest()
|
||||
hooks_cli._mine_sync()
|
||||
|
||||
result = hooks_cli._save_diary_direct(
|
||||
str(transcript),
|
||||
"session",
|
||||
agent_name="claude",
|
||||
)
|
||||
|
||||
hooks_cli._ingest_transcript(str(transcript))
|
||||
|
||||
probe.assert_called_once_with()
|
||||
submit.assert_not_called()
|
||||
spawn.assert_not_called()
|
||||
sync_run.assert_not_called()
|
||||
fake_mcp.tool_diary_write.assert_not_called()
|
||||
|
||||
assert result["count"] == 0
|
||||
assert result["routing_blocked"] is True
|
||||
|
||||
|
||||
def test_daemon_submission_failure_never_falls_back_to_direct():
|
||||
config = _HookConfig(WriteRoutingPolicy.REQUIRE)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"mempalace.hooks_cli.MempalaceConfig",
|
||||
return_value=config,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._get_mine_targets",
|
||||
return_value=[("/project", "projects")],
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._daemon_available",
|
||||
return_value=True,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._submit_daemon_job",
|
||||
side_effect=RuntimeError("daemon disappeared"),
|
||||
) as submit,
|
||||
patch(
|
||||
"mempalace.hooks_cli._spawn_mine",
|
||||
) as spawn,
|
||||
patch("mempalace.hooks_cli._log"),
|
||||
):
|
||||
hooks_cli._maybe_auto_ingest()
|
||||
|
||||
submit.assert_called_once()
|
||||
spawn.assert_not_called()
|
||||
|
||||
|
||||
def test_session_start_warns_when_required_daemon_unavailable(
|
||||
tmp_path,
|
||||
):
|
||||
config = _HookConfig(WriteRoutingPolicy.REQUIRE)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"mempalace.hooks_cli._palace_root_exists",
|
||||
return_value=True,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli.MempalaceConfig",
|
||||
return_value=config,
|
||||
),
|
||||
patch("mempalace.hooks_cli.STATE_DIR", tmp_path),
|
||||
patch(
|
||||
"mempalace.hooks_cli._daemon_available",
|
||||
return_value=False,
|
||||
),
|
||||
patch("mempalace.hooks_cli._log"),
|
||||
):
|
||||
output = _capture_output(
|
||||
lambda: hooks_cli.hook_session_start(
|
||||
{"session_id": "s1"},
|
||||
"claude-code",
|
||||
)
|
||||
)
|
||||
|
||||
assert "systemMessage" in output
|
||||
assert "require" in output["systemMessage"]
|
||||
assert "no direct ChromaDB fallback" in output["systemMessage"]
|
||||
|
||||
|
||||
def test_stop_require_unavailable_warns_and_does_not_advance_marker(
|
||||
tmp_path,
|
||||
):
|
||||
config = _HookConfig(WriteRoutingPolicy.REQUIRE)
|
||||
transcript = tmp_path / "session.jsonl"
|
||||
_write_transcript(transcript, hooks_cli.SAVE_INTERVAL)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"mempalace.hooks_cli._palace_root_exists",
|
||||
return_value=True,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli.MempalaceConfig",
|
||||
return_value=config,
|
||||
),
|
||||
patch("mempalace.hooks_cli.STATE_DIR", tmp_path),
|
||||
patch(
|
||||
"mempalace.hooks_cli._daemon_available",
|
||||
return_value=False,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._save_diary_direct",
|
||||
) as diary,
|
||||
patch(
|
||||
"mempalace.hooks_cli._ingest_transcript",
|
||||
) as ingest,
|
||||
patch(
|
||||
"mempalace.hooks_cli._maybe_auto_ingest",
|
||||
) as auto_ingest,
|
||||
patch("mempalace.hooks_cli._log"),
|
||||
):
|
||||
output = _capture_output(
|
||||
lambda: hooks_cli.hook_stop(
|
||||
{
|
||||
"session_id": "s1",
|
||||
"stop_hook_active": False,
|
||||
"transcript_path": str(transcript),
|
||||
},
|
||||
"claude-code",
|
||||
)
|
||||
)
|
||||
|
||||
diary.assert_not_called()
|
||||
ingest.assert_not_called()
|
||||
auto_ingest.assert_not_called()
|
||||
|
||||
assert not (tmp_path / "s1_last_save").exists()
|
||||
assert "systemMessage" in output
|
||||
assert "require" in output["systemMessage"]
|
||||
|
||||
|
||||
def test_precompact_require_unavailable_skips_all_writes(
|
||||
tmp_path,
|
||||
):
|
||||
config = _HookConfig(WriteRoutingPolicy.REQUIRE)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"mempalace.hooks_cli._palace_root_exists",
|
||||
return_value=True,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli.MempalaceConfig",
|
||||
return_value=config,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._daemon_available",
|
||||
return_value=False,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._ingest_transcript",
|
||||
) as ingest,
|
||||
patch(
|
||||
"mempalace.hooks_cli._mine_sync",
|
||||
) as mine_sync,
|
||||
patch("mempalace.hooks_cli._log"),
|
||||
):
|
||||
output = _capture_output(
|
||||
lambda: hooks_cli.hook_precompact(
|
||||
{
|
||||
"session_id": "s1",
|
||||
"transcript_path": str(tmp_path / "session.jsonl"),
|
||||
},
|
||||
"claude-code",
|
||||
)
|
||||
)
|
||||
|
||||
ingest.assert_not_called()
|
||||
mine_sync.assert_not_called()
|
||||
assert "systemMessage" in output
|
||||
|
||||
|
||||
def test_session_end_require_unavailable_skips_all_writes_and_cleans_marker(
|
||||
tmp_path,
|
||||
):
|
||||
config = _HookConfig(WriteRoutingPolicy.REQUIRE)
|
||||
transcript = tmp_path / "session.jsonl"
|
||||
_write_transcript(transcript)
|
||||
|
||||
marker = tmp_path / "s1_last_save"
|
||||
marker.write_text("15", encoding="utf-8")
|
||||
|
||||
with (
|
||||
patch(
|
||||
"mempalace.hooks_cli._palace_root_exists",
|
||||
return_value=True,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli.MempalaceConfig",
|
||||
return_value=config,
|
||||
),
|
||||
patch("mempalace.hooks_cli.STATE_DIR", tmp_path),
|
||||
patch(
|
||||
"mempalace.hooks_cli._daemon_available",
|
||||
return_value=False,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._save_diary_direct",
|
||||
) as diary,
|
||||
patch(
|
||||
"mempalace.hooks_cli._ingest_transcript",
|
||||
) as ingest,
|
||||
patch(
|
||||
"mempalace.hooks_cli._maybe_auto_ingest",
|
||||
) as auto_ingest,
|
||||
patch("mempalace.hooks_cli._log"),
|
||||
):
|
||||
output = _capture_output(
|
||||
lambda: hooks_cli.hook_session_end(
|
||||
{
|
||||
"session_id": "s1",
|
||||
"transcript_path": str(transcript),
|
||||
},
|
||||
"claude-code",
|
||||
)
|
||||
)
|
||||
|
||||
diary.assert_not_called()
|
||||
ingest.assert_not_called()
|
||||
auto_ingest.assert_not_called()
|
||||
|
||||
assert not marker.exists()
|
||||
assert "systemMessage" in output
|
||||
|
||||
|
||||
def test_stop_uses_one_daemon_probe_for_all_write_helpers(
|
||||
tmp_path,
|
||||
):
|
||||
config = _HookConfig(WriteRoutingPolicy.PREFER)
|
||||
transcript = tmp_path / "session.jsonl"
|
||||
_write_transcript(transcript, hooks_cli.SAVE_INTERVAL)
|
||||
|
||||
def save(*args, **kwargs):
|
||||
assert hooks_cli._current_hook_write_routing().use_daemon is True
|
||||
return {
|
||||
"count": hooks_cli.SAVE_INTERVAL,
|
||||
"themes": [],
|
||||
}
|
||||
|
||||
def use_current_route(*args, **kwargs):
|
||||
assert hooks_cli._current_hook_write_routing().use_daemon is True
|
||||
|
||||
with (
|
||||
patch(
|
||||
"mempalace.hooks_cli._palace_root_exists",
|
||||
return_value=True,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli.MempalaceConfig",
|
||||
return_value=config,
|
||||
),
|
||||
patch("mempalace.hooks_cli.STATE_DIR", tmp_path),
|
||||
patch(
|
||||
"mempalace.hooks_cli._daemon_available",
|
||||
return_value=True,
|
||||
) as probe,
|
||||
patch(
|
||||
"mempalace.hooks_cli._save_diary_direct",
|
||||
side_effect=save,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._ingest_transcript",
|
||||
side_effect=use_current_route,
|
||||
),
|
||||
patch(
|
||||
"mempalace.hooks_cli._maybe_auto_ingest",
|
||||
side_effect=use_current_route,
|
||||
),
|
||||
patch("mempalace.hooks_cli._log"),
|
||||
):
|
||||
output = _capture_output(
|
||||
lambda: hooks_cli.hook_stop(
|
||||
{
|
||||
"session_id": "s1",
|
||||
"stop_hook_active": False,
|
||||
"transcript_path": str(transcript),
|
||||
},
|
||||
"claude-code",
|
||||
)
|
||||
)
|
||||
|
||||
probe.assert_called_once_with()
|
||||
assert "memories woven" in output["systemMessage"]
|
||||
Loading…
Reference in New Issue