feat: community patches — mine deadlock fix, block_once precompact, KG auto-write, repair BLOB seq_id - #1335
Open
zhapostolski wants to merge 32 commits into
Open
Conversation
A triple with valid_to < valid_from satisfies neither of the temporal
filter clauses in query_entity():
valid_from <= as_of AND valid_to >= as_of
so the triple is invisible to every query — silently corrupt. Reject
at write time with a clear error instead of letting bad data pile up
in the SQLite store.
The guard only fires when both bounds are present; open intervals
(only valid_from or only valid_to) are still accepted, and same-day
intervals (valid_from == valid_to, point-in-time facts) are explicitly
allowed.
…rash EntityRegistry.save() called Path.write_text() directly, which truncates the target file and then writes — so a crash mid-write (power loss, OOM, filesystem-full mid-flush) leaves an empty or half-written entity_registry.json. The whole people/projects map is lost; the system falls back to an empty registry on next load. Switch to the standard atomic-write pattern: serialize to a sibling .tmp file in the same directory (so os.replace stays on one filesystem), fsync, chmod 0o600, then os.replace over the target. The replace is atomic on POSIX and Windows, so any crash leaves the previous registry intact instead of a truncated file. Tests cover: no leftover .tmp on success, and previous content preserved when os.replace itself raises mid-save.
Without this, on ext4 (and similar) filesystems the rename ack does not guarantee durability across power loss — a crash can revert to a state where the temp file is present and the target is at the old version. Suggested by @jphein on MemPalace#1215.
…nflation Repeated upserts to the mempalace_compressed collection across runs cause the HNSW link_lists.bin sparse file to grow without GC, eventually filling the disk (observed: 1.7 TB physical, 17 TB logical, on macOS ARM with chromadb 1.5.8). Drop and recreate the collection at the start of each compress run so the HNSW index is rebuilt from scratch each time. Re-vectorizing ~10K embeddings costs a few minutes on the local ONNX backend; far cheaper than risking TBs of disk. The miner code already does the equivalent (delete-by-source_file before re-insert, see miner.py:718) for the same hnswlib behavior. This brings the compress path in line. Related: MemPalace#1092
… boundary Add validate_iso_date() to config.py and apply it at the MCP boundary in tool_kg_query (as_of), tool_kg_add (valid_from), and tool_kg_invalidate (ended). Previously, non-ISO date strings like 'March 2026' or 'Jan 2025' were forwarded to SQLite without validation, silently producing empty result sets instead of matching facts. Now the MCP tools reject malformed dates early with a clear error message. Fixes MemPalace#1164
The validate_iso_date function uses PEP 604 union syntax (str | None) which requires Python 3.10+ at runtime. Adding future annotations import makes it work on the project's minimum supported Python 3.9. Addresses Qodo-Free-For-OSS review comment.
Replace with in validate_iso_date() to avoid potential SyntaxError on Python 3.9, even though is present. Some type-checking tools and runtime introspection may still evaluate annotations. Also add 11 new edge-case tests: - Leap year / non-leap year Feb 29 - Rejected formats: slash, dot, datetime, month 0, day 0 - Non-string input handling - Param name in error messages Ref: Qodo review on MemPalace#1283
MemPalace#976 protects `mempalace mine`, but MCP/direct backend writers still call ChromaCollection.add/upsert/update/delete without the palace lock. This moves the lock boundary to the Chroma backend seam so all Chroma writes share the same palace-level serialization, with a re-entrant guard for miner paths that already hold the lock. mine_palace_lock(palace_path) gains a per-thread re-entrant guard (threading.local + pid-tag against fork inheritance) so ChromaCollection write methods can take the lock without self-deadlocking when called from inside miner.mine()'s outer hold. ChromaCollection.__init__ accepts an optional palace_path; when set, add/upsert/update/delete wrap their underlying chromadb call with mine_palace_lock(palace_path). palace_path=None preserves the legacy no-lock behaviour for direct callers and tests. ChromaBackend's get_collection/create_collection pass palace_path through; mcp_server._get_collection forwards _config.palace_path so all MCP write tools inherit the wrapping. Tests: 5 new in tests/test_chroma_collection_lock.py covering opt-in, writer-blocks-during-mine, re-entrant-inside-mine, two-process serialization, and a source-level read-path-not-locked pin. Plus 1 new + 1 rewritten in tests/test_palace_locks.py for the re-entrant semantics. 52 passed in 1.01s including the existing test_backends.py regression suite. Refs MemPalace#1161.
Rollback cleanup was instantiating a fresh ChromaBackend, so the live backend that had opened the PersistentClient could keep file handles alive during restore. Close the active backend instance instead so rollback and CLI recovery can release Windows-safe locks before copying the backup back into place.
…lace#1299) `mcp_server._get_collection` bypassed `ChromaBackend.get_collection` and called `client.get_collection` / `client.create_collection` without `embedding_function=`. ChromaDB 1.x does not persist the EF identity with the collection, so the MCP server's reopen silently bound chromadb's built-in `DefaultEmbeddingFunction` while the miner / Stop hook ingest path bound `mempalace.embedding.get_embedding_function()`. On bleeding-edge interpreters (python 3.14 + chromadb 1.5.x on Apple Silicon, per MemPalace#1299) the default EF's lazy ONNX provider selection could SIGSEGV the host process on first `col.add()`, killing the MCP stdio server and leaving every subsequent tool call returning `Connection closed` until Claude Code was relaunched. Reads worked because `col.get(ids=...)` and metadata fetches don't invoke the EF; the auto-ingest path worked because mining routes through the backend abstraction. Diary writes were the consistent failure surface. Resolve the EF up front (matching `ChromaBackend._resolve_embedding_function`) and pass it into both reopen branches. Falls back to the chromadb default only if `mempalace.embedding.get_embedding_function` itself raises. Regression test patches the chromadb client class to capture `embedding_function=` on every `get_collection` / `create_collection` call from `_get_collection(create=True)` and `_get_collection()`, and fails if any call omits it. Follow-up to MemPalace#1262 / MemPalace#1289 (which fixed the metadata-mismatch SIGSEGV path); this addresses the EF-mismatch SIGSEGV path on the same surface.
…emPalace#1314) `tool_kg_add` previously accepted only `valid_from` and `source_closet`, silently dropping `valid_to`, `source_file`, and `source_drawer_id` at the MCP boundary. Backfilling already-ended historical facts therefore collapsed to "still current," and adapter provenance never reached the SQLite layer even though `KnowledgeGraph.add_triple` already supported every column. `tool_kg_invalidate` returned the literal string `"today"` whenever the caller omitted `ended`, hiding the actual stamped date from anyone trying to verify what got persisted. Changes: - Extend `tool_kg_add` signature + MCP input_schema with `valid_to`, `source_file`, `source_drawer_id`; forward all of them to `_kg.add_triple` and to the WAL log. - Resolve `ended` to `date.today().isoformat()` in `tool_kg_invalidate` before logging / returning, so the response always reports the actual date stored in `valid_to`. - Add regression tests for valid_to round-trip, source_file / source_drawer_id provenance, and the resolved-ended-date contract. - Leave TODO(MemPalace#1283) markers so the open ISO-8601 validation PR can drop `validate_iso_date` over `valid_from` / `valid_to` / `ended` cleanly. The underlying `KnowledgeGraph.add_triple` already accepted these kwargs (RFC 002 §5.5) — only the MCP edge needed wiring up. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…alace#1243) `tool_diary_write` stored the `agent` metadata verbatim after `sanitize_name` (which preserves case), while `tool_diary_read` filtered by exact match — so writing as "Claude" and reading as "claude" silently returned zero rows. Both endpoints now lowercase `agent_name` immediately after sanitization. The default per-agent wing slug is also stable across casings since it's derived from the same normalized form. Behavior change: entries written prior to this fix under mixed-case agent names will not match the new lowercase filter; documented under v3.3.5 in CHANGELOG with a `mempalace repair` pointer. Adds a regression test (`test_diary_read_case_insensitive_agent`) and updates the existing `test_diary_write_and_read` to assert the new lowercase agent identity. Closes MemPalace#1243 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…emPalace#1217) scan_convos() now prunes any directory named 'subagents' during os.walk. Claude Code records Explore/Plan/Grep subagent transcripts in <session-uuid>/subagents/agent-*.jsonl and on a typical workspace these outweigh main session files ~80:1, dominating mining time and producing near-zero additional signal (the parent session already summarizes them). Adds a --include-subagents opt-in flag for users who want full history. The shared SKIP_DIRS set in palace.py is left untouched, so project mining (miner.scan_project) still descends into legitimate user-created subagents/ directories in code projects.
- Case-insensitive directory match (d.lower() == 'subagents') so the filter still kicks in if Claude Code or a plugin ever emits 'Subagents/' on case-preserving filesystems (Windows, macOS APFS). - Drop defensive getattr in cmd_mine: argparse always defines the attribute since --include-subagents is unconditionally registered. Direct args.include_subagents access matches every neighbouring field and fails loudly if the registration is ever removed. - Soften CLI help and docstring: drop the 80:1 ratio (reporter- specific) and the in-code (MemPalace#1217) reference. Add explicit default=False on the argparse flag for symmetry with --extract. - Add 2 negative tests: 'mysubagents'/'subagentsbackup' must still be mined (regression guard against substring-match), and 'Subagents/' must be skipped (case-insensitive coverage).
layers.py (wake-up) always opens knowledge_graph.sqlite3 from the palace directory. The MCP server previously fell back to ~/.mempalace/knowledge_graph.sqlite3 when --palace was not passed, causing a split: kg_add wrote to the root KG while session wake-up read the palace KG. New KG facts never appeared in the next session's context. Fix: always resolve the KG path from _config.palace_path, which already reflects --palace, MEMPALACE_PALACE_PATH, or the default from config.json. The --palace arg still controls the palace path; the conditional around it was only needed for the now-removed special case.
Previously hook_session_start was a no-op stub. Now it:
- Parses cwd + source from the harness input
- Matches cwd basename to a known wing via tool_status()
- Builds additionalContext from MemoryStack.wake_up() + a protocol nudge
telling the LLM to kg_query/search before responding and add_drawer/kg_add
when making decisions
- Returns Claude-Code-compatible {hookSpecificOutput.additionalContext}
shape; harnesses that ignore the key see a no-op
This makes SessionStart actually useful: the LLM gets ~700 tokens of
project context on every session start instead of an empty {}.
Tests: replaced the pass-through assertion with two cases (context
injected when palace resolvable, no-op when MempalaceConfig fails).
All 121 existing tests still pass.
Adds _mirror_local_memory() called from hook_stop on every fire.
Scans default memory locations (~/.claude/projects, ~/.codex/memories,
~/.gemini/memory, ~/.qwen/memory) for *.md files; for each new or
modified file, calls tool_add_drawer with wing derived from the
project slug and room from the filename stem.
Why: even with the SessionStart protocol nudge, models (notably
Claude Code) reflexively write to their per-CLI auto-memory dirs
instead of mempalace MCP tools. This makes mempalace a guaranteed
mirror of those .md files regardless of LLM cooperation.
Behavior:
- Idempotent via {path: mtime} state in ~/.mempalace/hook_state/mirror_state.json
- Skips index files (MEMORY.md, CLAUDE.md, GEMINI.md, QWEN.md, AGENTS.md)
- Skips empty files (still records mtime so we don't keep re-reading)
- Records duplicates (mempalace dedupe) as "seen" to avoid retry loops
- Wing slug parser: -home-...-projects-<wing> → <wing> (Claude convention),
else first directory after the root
- All errors caught — never blocks the harness
- Disable via MEMPAL_MIRROR_DISABLED=1
- Override roots via MEMPAL_MIRROR_ROOTS=path:path:...
Verified: real run on 312 .md files added 248 new drawers (rest were
chromadb dupes or empty). Second run: 0 added, 312 skipped_unchanged.
All 128 tests pass (8 new mirror tests).
The save and precompact wrappers ran via bare 'python3', which resolves
to whatever interpreter is on PATH. From a CLI launched in a clean shell
(or any shell without the right venv activated), python3 is system
python — which lacks chromadb and other mempalace deps. The hooks
silently degraded: hook_session_start would no-op, and the new
_mirror_local_memory() would log
"mirror: cannot import tool_add_drawer (No module named 'chromadb')"
and skip the entire mirror, defeating Option-2 dual-write.
Fix: each wrapper now resolves $MEMPAL_PYTHON, defaulting to
$HOME/.mempalace/venv/bin/python (the canonical install location), and
falls back to plain python3 if that doesn't exist. All three wrappers
(save, precompact, session-start) follow the same pattern so they
behave identically across Claude / Codex / Gemini / Qwen.
Verified end-to-end from a stripped env (env -i PATH=/usr/bin:/bin):
- save_hook returns {} cleanly with no chromadb errors in hook.log
- precompact_hook returns {"decision":"allow"}
- session_start_hook returns Claude-shape additionalContext with
wing-matched wake-up text
- mempal_enrich_knowledge.sh: enriches KG with session insights post-stop - mempal_verify_knowledge.sh: validates KG entries for consistency - mempal_semantic_search.sh: CLI wrapper for semantic search from hooks - mempal_opencode_hook.sh: Stop hook adapter for OpenCode harness - mempal_opencode_simple.sh: lightweight OpenCode hook (no KG enrichment) - mempal_save_hook_throttled.sh: rate-limited save wrapper (Python handles intervals) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ommit The fix(hooks) commit that corrected CLI entry points and harness detection inadvertently replaced normalize.py with a truncated version (207 lines vs the correct 676). Restore from origin/main verbatim. No functional change to normalize.py — only the hooks_cli.py content from that commit was intentional. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
I think this PR improves the write-serialization story quite a bit, especially via the I’m not fully convinced it covers the entire repair/rebuild boundary yet, though. The part I’m still looking at is whether repair is treated as an exclusive palace-level operation from the point where it opens the live collection through backup, replacement, rollback, and restore. My concern there is a little different from the ordinary concurrent-writer case: even if normal collection writes are serialized, the destructive swap/restore portion of repair happens outside the So I think this gets us closer, but there may still be a gap around holding an outer repair-specific write lock across the full destructive section of the workflow. I’m working on that side separately, along with the handle-release part before filesystem moves/removals, just to make sure the repair boundary is fully covered. |
…apes Claude Code wraps every slash-command invocation in a five-tag envelope (<local-command-caveat>, <command-name>, <command-message>, <command-args>, <local-command-stdout>). strip_noise() previously covered only command-name and command-message; the other three survived into stored drawers as tag remnants like "<command-args></command-args>" or whole stdout payloads. Bash tool_result content captured by Claude Code preserves ANSI color codes verbatim. These pass through strip_noise() untouched, bloating embeddings (each escape is several BPE tokens) and garbling semantic search. Extend _NOISE_TAGS with the missing three, and add CSI + OSC ANSI strippers applied after tag removal in strip_noise(). Each ANSI pattern is anchored on the literal ESC byte so user prose that mentions e.g. "[1m]" by name stays intact — verbatim-safety preserved per the existing design. Tests: add coverage for each new tag, the full slash-command envelope, the empty <command-args></command-args> shape, ANSI CSI / truecolor / cursor / OSC-title / OSC-hyperlink sequences, ANSI inside a noise tag (no double-strip needed), and two preservation tests for user prose that documents these constructs by name. Closes MemPalace#1333
…hromadb version mismatch When an HNSW segment has a mtime gap > 7200s (2 h), quarantine it regardless of the metadata sniff-test result. ChromaDB flush-lag is measured in seconds; a 2+ hour gap means the segment was written by a different process/version. Observed: chromadb 0.6.x segfaults loading segments whose metadata passes the pickle format check but whose binary layout is incompatible with the current runtime. The original directory is renamed (not deleted), so manual recovery is still possible if the heuristic misfires. Includes cherry-pick of MemPalace#1334: fix(normalize): strip Claude Code local-command tags and ANSI escapes Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… timeout to 60s SessionStart now fires a background ChromaDB get_collection so the HNSW index and ONNX embedding model are loaded before the first Stop hook fires. Eliminates the 7 cancelled Stop hooks (30s cold-start timeout) observed in the ananas-os session. Separately: ~/.claude/settings.json hook timeouts bumped 30→60s so a slow cold ChromaDB start still completes rather than being cancelled. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- _ingest_transcript: check _mine_already_running() before spawning + register PID in _MINE_PID_FILE so subsequent Stop hook fires see the running mine - hook_stop (silent mode): skip diary save when mine is running; don't advance last_save counter so next Stop hook retries — eliminates 'another mine already running' failures seen in production hook.log - hook_precompact: change default mode from 'proceed' (no-op for claude-code and gemini) to 'block_once', so pre-compaction saves happen for all harnesses without infinite-loop risk (flag keyed by session_id) - _auto_kg_write: new helper that adds session-YYYY-MM-DD → covered_topic → theme relationships after each successful silent diary checkpoint - fix(repair): _detect_poisoned_max_seq_ids now handles BLOB-typed seq_id values from ChromaDB 1.5.x (8-byte big-endian uint64); previously crashed with ValueError: invalid literal for int() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
zhapostolski
changed the title
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
9 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.
What this PR does
Consolidates 32 commits combining community bug fixes and session/hook infrastructure.
Community fixes included
_get_collectionpassesembedding_function=(prevents SIGSEGV on reopen)collection_namekg_addforwardsvalid_to,source_file,source_drawer_idsubagents/directoriesSession & hook infrastructure
.mdfiles from~/.claude/projects,~/.codex/memories,~/.gemini/memory,~/.qwen/memoryinto palace drawers (cross-agent memory)Latest additions (this update)
_detect_poisoned_max_seq_idshandles BLOB-typedseq_idfrom ChromaDB 1.5.x — fixesValueError: invalid literal for int()blockingrepair --mode max-seq-idon 92k+ drawer palaces_ingest_transcriptchecks_mine_already_running()before spawning and registers its PID;hook_stopskips diary save when mine is running, retries on next fire — eliminates cascading "another mine already running" failuresproceed(no-op for claude-code/gemini) toblock_oncefor all harnessessession-YYYY-MM-DD → covered_topic → themeafter each silent diary checkpointTest plan
repair --mode max-seq-idcompletes without ValueError on palace with BLOB seq_ids{"decision":"block"}on first fire per session,{}on secondsession-YYYY-MM-DDwithcovered_topicpredicatepython -m pytest tests/ -v --ignore=tests/benchmarks