fix(hooks): treat absent ~/.mempalace as auto-save off#1305
Merged
igorls merged 2 commits intoMemPalace:developfrom May 6, 2026
Merged
fix(hooks): treat absent ~/.mempalace as auto-save off#1305igorls merged 2 commits intoMemPalace:developfrom
igorls merged 2 commits intoMemPalace:developfrom
Conversation
|
Hi, The new kill-switch treats any existing Severity: remediation recommended | Category: reliability How to fix: Use is_dir for PALACE_ROOT Agent prompt to fix - you can give this to your LLM of choice:
Found by Qodo code review |
igorls
added
area/cli
Member
|
Thanks @lcatlett — small, correct, and the new tests look good. Two changes before merge: 1. Rebase against 2. Address the Qodo bot comment — def _palace_root_exists() -> bool:
return PALACE_ROOT.is_dir()Once both are in, I'll authorize CI on the fork PR and we can land it for v3.3.5. |
When the user removes ~/.mempalace/ (a strong "do not auto-capture"
signal), the next hook fire would silently recreate the entire dir
hierarchy and ingest existing transcripts:
1. _log() at hooks_cli.py:148 unconditionally calls
STATE_DIR.mkdir(parents=True, exist_ok=True), so the act of
writing the hook log line recreated ~/.mempalace/hook_state/
2. With no config file present, hook_stop_auto_save and
hook_precompact_auto_save defaulted to True (no override to read)
3. The full save path then ran, materializing palace/, wal/,
knowledge_graph.sqlite3, and N drawers from existing transcripts
in ~/.claude/projects/*.jsonl
All four entry points (hook_stop, hook_precompact, hook_session_start,
and _log itself) now check a new PALACE_ROOT = Path.home() / ".mempalace"
constant first and short-circuit (returning {} on stdout, never logging)
when the dir is absent. The user-removable directory is now a kill-switch.
Five unit tests in tests/test_hooks_cli.py cover: hook_stop /
hook_precompact / hook_session_start do not create the dir when absent;
_log() does not create it when absent; existing dir proceeds normally
(regression).
Caught in the wild on a downstream fork: ~146 drawers materialized in
under a second after a deliberate `rm -rf ~/.mempalace/`, into a planning
session that was explicitly not meant to be captured.
Both @igorls and the Qodo bot flagged that `_palace_root_exists()` used `Path.exists()`, which returns True for a regular file. A stray file at `~/.mempalace` would let the kill-switch be bypassed and crash later in `STATE_DIR.mkdir()` with NotADirectoryError. Switched to `Path.is_dir()`. Also fold `_log()`'s inline check through `_palace_root_exists()` so both kill-switch sites use the same predicate. New test pins the behavior: a regular file at the palace root path is treated as absent (hook short-circuits, _log does not crash, the stray file is left untouched).
lcatlett
force-pushed
the
upstream/respect-absent-palace-dir
branch
from
672bf0f to
2d50b21
Compare
This was referenced May 6, 2026
igorls
added a commit
to cantenesse/mempalace
that referenced
this pull request
May 6, 2026
The alias was placed below an explanatory comment block introduced by MemPalace#1305, which trips ruff E402 (module-level import not at top of file). Moved next to the existing 'from mempalace.hooks_cli import (...)' line. CI lint went red on develop after MemPalace#1305 merged with the failing check; this re-greens it so subsequent PRs do not inherit the failure.
xcarbo
added a commit
to xcarbo/mempalace
that referenced
this pull request
May 7, 2026
Catches up on a heavy upstream day — 22 fixes merged in 24h plus prior backlog. Highlights pulled in: - MemPalace#1305 hooks: ~/.mempalace/ deletion is now a stable kill-switch (hooks no longer rebuild the dir hierarchy on Stop/PreCompact/SessionStart) - MemPalace#1214 KG: reject inverted intervals (valid_to < valid_from) at write time — prevents silently invisible triples - MemPalace#1067/MemPalace#1105 chroma: ChromaBackend.close_palace() now actually releases the SQLite file lock (PersistentClient.close() on evict + invalidation) - MemPalace#1215 entity_registry: atomic save (tmp+fsync+rename) — no more corruption on crash mid-write - MemPalace#1073/MemPalace#1107 mempalace compress: paginated drawer fetch — no longer trips SQLITE_MAX_VARIABLE_NUMBER on palaces >32k drawers - MemPalace#1282 stdio: Windows console UTF-8 reconfig for cli/mcp_server/hooks_cli - MemPalace#1164/MemPalace#1167 mcp KG: sanitize_iso_date() blocks malformed date strings silently producing empty result sets - MemPalace#1136/MemPalace#1160 mcp: per-path KG cache for multi-tenant hosts that rotate MEMPALACE_PALACE_PATH between tool calls - MemPalace#1286 mcp: retry _get_collection() once on transient failure - MemPalace#1138 lint cleanup, MemPalace#1019 search-crash fix - 4 new tools/ scripts (backup_claude_jsonls, find_orphan_claude_jsonls, render_jsonl, save.md) Conflict resolution (CHANGELOG.md only — code files all auto-merged): - 3.3.5 section: untouched (already merged in our prior commit; upstream added several new bug-fix entries which auto-merged cleanly) - 3.3.4 Bug Fixes: kept upstream's new MemPalace#1305 entry; preserved our richer detail on topic-tunnels (MemPalace#1194/MemPalace#1195/MemPalace#1197), HNSW-bloat (MemPalace#1191), max_seq_id (MemPalace#1135), and auto-ingest (MemPalace#1230/MemPalace#1231) — upstream's shorter topic-tunnels entry was a strict subset of ours. xdev patches preserved (still on this branch, untouched by merge): - 6ef44cb fix(hooks): route CC transcripts via convo_miner with cwd-based wings - 3fad61d fix(config): allow leading dash in wing names - 3fc821a fix(config): tighten leading-char to allow dash but not underscore Tests: 1557 passed, 1 skipped (full unit suite excluding benchmarks). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When the user removes
~/.mempalace/(a strong "do not auto-capture" signal), the next hook fire silently recreates the entire dir and ingests existing transcripts. This PR makes the absence of~/.mempalace/a kill-switch across all hooks.Caught in the wild: ~146 drawers materialized in <1s after a deliberate
rm -rf ~/.mempalace/, into a session that was explicitly not meant to be captured.Root cause (three layers)
_log()athooks_cli.py:148unconditionally callsSTATE_DIR.mkdir(parents=True, exist_ok=True). So writing the hook log line itself recreates~/.mempalace/hook_state/.True(no override to read). The fail-open semantics treat "no config" as "save enabled" — exactly backwards when the entire dir was just deleted.hook_session_starthas no toggle at all and unconditionally_log()s +STATE_DIR.mkdir()s.Fix
All four entry points (
hook_stop,hook_precompact,hook_session_start, and_logitself) now check a newPALACE_ROOT = Path.home() / ".mempalace"constant and short-circuit (returning{}on stdout, never logging) when the directory is absent. The user-removable directory is now a kill-switch._output()writes directly to stdout fd 1 — no disk side effect. So the short-circuit response can be sent without recreating anything.Tests
5 unit tests in
tests/test_hooks_cli.py:test_hook_stop_does_not_create_palace_dir_when_absenttest_hook_precompact_does_not_create_palace_dir_when_absenttest_hook_session_start_does_not_create_palace_dir_when_absenttest_log_does_not_create_palace_dir_when_absenttest_existing_dir_proceeds_normally(regression)Full suite: 1469 passed, 1 skipped → 1474 passed, 1 skipped (+5, 0 regressions).
Related
mempalace mineprocesses that survive across sessions and bypass PID guard #1212 (concurrent mine processes) — same family of "hooks doing things users didn't authorize"mempalace mineon chat transcript parent with default flags → polluted mega-wing, no opt-out #1083 (polluted mega-wing) — symptom-adjacentNot a duplicate of any of the above, but adjacent.
Test plan
python3 -m pytest tests/test_hooks_cli.py -k "palace_dir or proceeds_normally" -vpython3 -m pytest tests/ --ignore=tests/benchmarks -qrm -rf ~/.mempalace, fire all three hooks, confirm~/.mempalacestays absent