Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions mempalace/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,36 @@ def hooks_auto_save(self):
hooks = self._file_config.get("hooks", {})
return hooks.get("auto_save", True)

@property
def hooks_transcript_mining(self):
"""Whether hooks may spawn detached transcript-mining subprocesses.

Transcript mining (via ``_ingest_transcript`` / ``_maybe_auto_ingest``
/ ``_mine_sync``) is heavyweight. Under multi-writer deployments
(multiple concurrent Claude Code / Codex sessions filing through
the same palace), an in-hook mine can starve interactive MCP writes
queued behind it in the daemon (#1497), or contend with concurrent
processes for the palace storage lock.

When ``False``, hooks emit reminders (governed by ``hooks.silent_save``)
but never spawn mining. Bulk mining can then be scheduled out-of-band
via a systemd timer / cron job that runs when no session is active —
the pattern @anastasiiaanfimova ships as an out-of-tree ``mine off``
patch and @jphein's palace-daemon replicates at gateway layer.

Master ``hooks.auto_save`` still gates this: if auto-save is off,
this is always effectively off.

Env override: ``MEMPALACE_HOOKS_TRANSCRIPT_MINING``.
"""
env_val = os.environ.get("MEMPALACE_HOOKS_TRANSCRIPT_MINING")
if env_val is not None:
return env_val.lower() not in ("false", "0", "no")
if not self.hooks_auto_save:
return False
hooks = self._file_config.get("hooks", {})
return hooks.get("transcript_mining", True)

@property
def topic_wings(self):
"""List of topic wing names."""
Expand Down
26 changes: 24 additions & 2 deletions mempalace/hooks_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,15 @@ def _maybe_auto_ingest():
target gets its own PID slot, so distinct targets never block each
other but a re-fire of the same target while the previous one is
still running is silently skipped.

Respects ``hooks.transcript_mining`` — when disabled, returns
immediately without spawning any mine subprocess.
"""
try:
if not MempalaceConfig().hooks_transcript_mining:
return
except Exception:
pass # config unreadable: fall through to legacy behavior
targets = _get_mine_targets()
if not targets:
return
Expand Down Expand Up @@ -622,7 +630,15 @@ def _mine_sync():
Transcript convos are ingested separately via ``_ingest_transcript``
in ``hook_precompact`` — keeping them out of this function avoids
timeout stacking against the harness 30s ceiling (#1231 review).

Respects ``hooks.transcript_mining`` — when disabled, returns
immediately without running the sync mine.
"""
try:
if not MempalaceConfig().hooks_transcript_mining:
return
except Exception:
pass # config unreadable: fall through to legacy behavior
targets = _get_mine_targets()
if not targets:
return
Expand Down Expand Up @@ -855,7 +871,11 @@ def _save_diary_direct(


def _ingest_transcript(transcript_path: str):
"""Mine a Claude Code session transcript into the palace as a conversation."""
"""Mine a Claude Code session transcript into the palace as a conversation.

Respects ``hooks.transcript_mining`` — when disabled, returns
immediately without spawning the transcript-mine subprocess.
"""
path = _validate_transcript_path(transcript_path)
if path is None:
return
Expand All @@ -866,9 +886,11 @@ def _ingest_transcript(transcript_path: str):
return

try:
MempalaceConfig() # validate config loads
config = MempalaceConfig() # validate config loads
except Exception:
return
if not config.hooks_transcript_mining:
return

try:
if _hooks_daemon_enabled() and _daemon_available():
Expand Down
74 changes: 74 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,80 @@ def test_hooks_auto_save_env_override_true():
del os.environ["MEMPALACE_HOOKS_AUTO_SAVE"]


# --- hooks.transcript_mining ---


def test_hooks_transcript_mining_default(monkeypatch):
"""Default is True — mining happens in hooks, matches pre-PR behavior."""
monkeypatch.delenv("MEMPALACE_HOOKS_TRANSCRIPT_MINING", raising=False)
cfg = MempalaceConfig(config_dir=tempfile.mkdtemp())
assert cfg.hooks_transcript_mining is True


def test_hooks_transcript_mining_from_config(monkeypatch, tmp_path):
monkeypatch.delenv("MEMPALACE_HOOKS_TRANSCRIPT_MINING", raising=False)
with open(tmp_path / "config.json", "w") as f:
json.dump({"hooks": {"transcript_mining": False}}, f)
cfg = MempalaceConfig(config_dir=str(tmp_path))
assert cfg.hooks_transcript_mining is False


def test_hooks_transcript_mining_env_override_false(monkeypatch):
monkeypatch.setenv("MEMPALACE_HOOKS_TRANSCRIPT_MINING", "false")
cfg = MempalaceConfig(config_dir=tempfile.mkdtemp())
assert cfg.hooks_transcript_mining is False


def test_hooks_transcript_mining_env_override_zero(monkeypatch):
monkeypatch.setenv("MEMPALACE_HOOKS_TRANSCRIPT_MINING", "0")
cfg = MempalaceConfig(config_dir=tempfile.mkdtemp())
assert cfg.hooks_transcript_mining is False


def test_hooks_transcript_mining_env_override_no(monkeypatch):
monkeypatch.setenv("MEMPALACE_HOOKS_TRANSCRIPT_MINING", "no")
cfg = MempalaceConfig(config_dir=tempfile.mkdtemp())
assert cfg.hooks_transcript_mining is False


def test_hooks_transcript_mining_env_override_true(monkeypatch, tmp_path):
"""Env var set to 'true' overrides config file even if config says false."""
with open(tmp_path / "config.json", "w") as f:
json.dump({"hooks": {"transcript_mining": False}}, f)
monkeypatch.setenv("MEMPALACE_HOOKS_TRANSCRIPT_MINING", "true")
cfg = MempalaceConfig(config_dir=str(tmp_path))
assert cfg.hooks_transcript_mining is True


def test_hooks_transcript_mining_masked_by_auto_save_off(monkeypatch, tmp_path):
"""Master auto_save=False forces transcript_mining to False.

Preserves back-compat: existing users who have opted out of hooks
entirely via ``hooks.auto_save: false`` continue to get zero mining
activity, regardless of what ``hooks.transcript_mining`` is set to.
"""
monkeypatch.delenv("MEMPALACE_HOOKS_TRANSCRIPT_MINING", raising=False)
with open(tmp_path / "config.json", "w") as f:
json.dump({"hooks": {"auto_save": False, "transcript_mining": True}}, f)
cfg = MempalaceConfig(config_dir=str(tmp_path))
assert cfg.hooks_auto_save is False
assert cfg.hooks_transcript_mining is False


def test_hooks_transcript_mining_env_wins_over_auto_save_masking(monkeypatch, tmp_path):
"""Env override is checked BEFORE the auto_save mask.

Intentional: an operator setting the env explicitly should not be
silently overridden by config-file state. This mirrors how
hooks_auto_save's own env override wins over config.
"""
with open(tmp_path / "config.json", "w") as f:
json.dump({"hooks": {"auto_save": False}}, f)
monkeypatch.setenv("MEMPALACE_HOOKS_TRANSCRIPT_MINING", "true")
cfg = MempalaceConfig(config_dir=str(tmp_path))
assert cfg.hooks_transcript_mining is True


def test_hook_use_daemon_default_false(monkeypatch):
monkeypatch.delenv("MEMPALACE_HOOKS_DAEMON", raising=False)
cfg = MempalaceConfig(config_dir=tempfile.mkdtemp())
Expand Down
83 changes: 83 additions & 0 deletions tests/test_hooks_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +973,89 @@ def test_mine_sync_with_env_uses_projects_mode(tmp_path):
assert cmd[cmd.index("--mode") + 1] == "projects"


# --- hooks.transcript_mining belt-and-suspenders guards ---


def test_maybe_auto_ingest_skipped_when_transcript_mining_off(tmp_path):
"""With transcript_mining=false, _maybe_auto_ingest is a no-op.

Even with MEMPAL_DIR set to a valid directory (which would normally
trigger a mine subprocess), no Popen call happens when the config
flag is off.
"""
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._MINE_PID_DIR", tmp_path / "mine_pids"):
with patch("mempalace.hooks_cli.MempalaceConfig") as mock_cfg_cls:
mock_cfg_cls.return_value.hooks_transcript_mining = False
with patch("mempalace.hooks_cli.subprocess.Popen") as mock_popen:
_maybe_auto_ingest()
mock_popen.assert_not_called()


def test_mine_sync_skipped_when_transcript_mining_off(tmp_path):
"""With transcript_mining=false, _mine_sync is a no-op.

Precompact sync mine would otherwise spawn a subprocess.run — this
guard prevents that during a hook fire when the operator has
scheduled mining out-of-band via a timer instead.
"""
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.MempalaceConfig") as mock_cfg_cls:
mock_cfg_cls.return_value.hooks_transcript_mining = False
with patch("mempalace.hooks_cli.subprocess.run") as mock_run:
_mine_sync()
mock_run.assert_not_called()


def test_ingest_transcript_skipped_when_transcript_mining_off(tmp_path):
"""With transcript_mining=false, _ingest_transcript is a no-op.

Even with a valid transcript path large enough to normally trigger
the mine, no daemon submission and no subprocess Popen happens when
the config flag is off.
"""
from mempalace.hooks_cli import _ingest_transcript

transcript = tmp_path / "session.jsonl"
# Write enough bytes to pass the 100-byte size gate
transcript.write_text('{"message": {"role": "user", "content": "x" * 200}}\n' * 5)
with patch("mempalace.hooks_cli.STATE_DIR", tmp_path):
with patch("mempalace.hooks_cli.MempalaceConfig") as mock_cfg_cls:
mock_cfg_cls.return_value.hooks_transcript_mining = False
with patch("mempalace.hooks_cli._daemon_available") as mock_daemon_avail:
with patch("mempalace.hooks_cli.subprocess.Popen") as mock_popen:
_ingest_transcript(str(transcript))
mock_popen.assert_not_called()
# daemon shouldn't even be checked — we bail before that
mock_daemon_avail.assert_not_called()


def test_maybe_auto_ingest_default_true_still_mines(tmp_path):
"""Regression: default config (transcript_mining not set) still mines.

Guards against a bug where the belt-and-suspenders guard misreads
the default and turns mining off for existing users.
"""
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._MINE_PID_DIR", tmp_path / "mine_pids"):
# Real MempalaceConfig from a temp dir with no config.json →
# hooks_transcript_mining defaults to True
with patch("mempalace.hooks_cli.MempalaceConfig") as mock_cfg_cls:
mock_cfg_cls.return_value.hooks_transcript_mining = True
with patch("mempalace.hooks_cli.subprocess.Popen") as mock_popen:
_maybe_auto_ingest()
mock_popen.assert_called_once()


def test_mine_sync_uses_mempalace_python(tmp_path):
"""Sync mine command uses _mempalace_python(), not bare sys.executable."""
mempal_dir = tmp_path / "project"
Expand Down