Add palace compatibility guardrails

[yeager]

Summary

This PR adds basic compatibility guardrails around local palace access so MemPalace fails cleanly when the local Chroma environment is incompatible with the palace metadata.

What changed

• add mempalace/compat.py
• write mempalace_meta.json into the palace with:
  • MemPalace version
  • Chroma version
  • Chroma major version
• check palace compatibility before:
  • search
  • wake-up
  • status
  • compress
  • layer-based retrieval/search
  • collection access in miner.py
• convert top-level RuntimeError failures in cli.py into clean CLI error messages

Why

I hit a local case where MemPalace had indexed successfully, but later access crashed because the effective Chroma environment had changed. This PR does not try to solve every Chroma-side issue, but it does make one bad failure mode much safer:

• instead of touching a palace blindly and risking corruption/segfault-like behavior,
• MemPalace now refuses to proceed when the recorded Chroma major version does not match the current one.

Behavior

Before:

• a palace created under one Chroma major could be opened under another with no explicit warning
• failures were harder to diagnose and could look like deeper runtime breakage

After:

• palace metadata is written on collection creation/use
• incompatible major-version access fails early with a clear error message

Notes

This is intentionally conservative.
It is better to stop with a clear message and ask for a rebuild than to touch a palace with a known-incompatible local environment.

[bgauryy]

PR Review: Add palace compatibility guardrails

Executive Summary

Aspect	Value
PR Goal	Fail cleanly when the local Chroma environment is incompatible with the palace metadata, preventing index corruption or segfaults
Files Changed	5 (1 new, 4 modified)
Risk Level	🔴 HIGH - Bootstrap path blocks fresh palace creation under Chroma >= 1
Review Effort	3 - Moderate: new module with cross-cutting integration
Recommendation	🔄 REQUEST_CHANGES

Affected Areas: compat.py (new), cli.py, miner.py, layers.py, searcher.py

Business Impact: Users on Chroma >= 1 cannot create new palaces. Existing users upgrading to Chroma >= 1 with pre-metadata palaces have no migration path. MCP server and conversation mining bypass the guardrails entirely.

Flow Changes: Every read path (search, wake-up, status, compress) and the write path (mine) now gate on ensure_palace_safe() which reads mempalace_meta.json from disk. The mine path additionally writes that file after collection access.

Ratings

Aspect	Score
Correctness	2/5
Security	4/5
Performance	3/5
Maintainability	2/5

PR Health

• Has clear description
• References ticket/issue (if applicable)
• Appropriate size (or justified if large)
• Has relevant tests (if applicable) — no tests for compat.py

High Priority Issues

🐛 #1: Fresh palace creation impossible under Chroma >= 1

Location: mempalace/miner.py:get_collection() | Confidence: ✅ HIGH

ensure_palace_safe() runs before write_palace_metadata(). On a brand-new palace directory, mempalace_meta.json does not exist yet, so meta is None. When chromadb_major() >= 1, the function raises RuntimeError("Refusing to open palace without compatibility metadata under Chroma 1.x."). The metadata write that would have resolved this sits after the check and never executes. This is a chicken-and-egg: you cannot create a palace because the guard requires metadata that only gets written after creating the palace.

 def get_collection(palace_path: str):
     os.makedirs(palace_path, exist_ok=True)
-    ensure_palace_safe(palace_path)
     client = chromadb.PersistentClient(path=palace_path)
     try:
         collection = client.get_collection("mempalace_drawers")
     except Exception:
         collection = client.create_collection("mempalace_drawers")
     write_palace_metadata(palace_path)
+    ensure_palace_safe(palace_path)
     return collection

Alternatively, make the guard bootstrap-aware — distinguish a truly fresh palace (no chroma.sqlite3) from a pre-metadata legacy palace:

 def ensure_palace_safe(palace_path: str) -> None:
     current_major = chromadb_major()
     meta = read_palace_metadata(palace_path)
 
     if meta is None:
-        if current_major is not None and current_major >= 1:
+        has_existing_data = (Path(palace_path) / "chroma.sqlite3").exists()
+        if has_existing_data and current_major is not None and current_major >= 1:
             raise RuntimeError(...)
         return

---

🐛 #2: Incomplete guardrail coverage — 3 unguarded ChromaDB entry points

Location: mempalace/convo_miner.py, mempalace/mcp_server.py, mempalace/palace_graph.py | Confidence: ✅ HIGH

These modules all create chromadb.PersistentClient directly without ensure_palace_safe:

Module	Function	Line
convo_miner.py	get_collection()	~214
mcp_server.py	_get_collection()	~44
palace_graph.py	build_graph()	~27

The MCP server is particularly significant — it serves 14 tools to AI assistants and is a primary access path. Any palace operation via MCP bypasses the new guardrails completely.

 # mcp_server.py — _get_collection
+from .compat import ensure_palace_safe
+
 def _get_collection(create=False):
     global _client_cache, _collection_cache
     try:
         if _client_cache is None:
+            ensure_palace_safe(_config.palace_path)
             _client_cache = chromadb.PersistentClient(path=_config.palace_path)

---

Medium Priority Issues

🏗️ #3: Scattered enforcement — should be centralized in palace_db.py

Location: compat.py integration across 4 modules | Confidence: ⚠️ MED

Per AGENTS.md: "All ChromaDB access goes through palace_db.py". The module already has get_collection() with client caching — this is the natural single enforcement point for compatibility checks.

Instead, this PR adds ~13 ensure_palace_safe() calls scattered across cli.py, layers.py, miner.py, and searcher.py. This creates:

• Maintenance burden: every new ChromaDB access path must remember to add the guard
• The coverage gaps in issue Integrating MemPalace with SoulForge's code intelligence system #2 are a direct consequence of this pattern

Adding the guard inside palace_db.py:get_collection() (or the client factory) would protect all callers automatically — including future ones.

---

🔁 #4: Redundant safety checks — double disk reads per operation

Location: cli.py + downstream modules | Confidence: ✅ HIGH

Multiple CLI commands call ensure_palace_safe then invoke a function that calls it again:

CLI entry	Also checked in
cmd_search → ensure_palace_safe	searcher.search() → ensure_palace_safe
cmd_status → ensure_palace_safe	miner.status() → ensure_palace_safe
cmd_wakeup → ensure_palace_safe	MemoryStack.wake_up() → 2-3 layer methods each call ensure_palace_safe

Each call reads mempalace_meta.json from disk. For wake_up, the file is read 4+ times in a single operation. This is not a performance-critical path today, but centralizing in palace_db.py (issue #3) would eliminate the redundancy by design.

---

Low Priority Issues

🎨 #5: Broad RuntimeError catch in main() swallows unrelated errors

Location: mempalace/cli.py:main() | Confidence: ⚠️ MED

The new top-level catch intercepts all RuntimeError exceptions, not just compatibility ones. Any unrelated RuntimeError from any command will print a clean one-liner and exit — losing the stack trace needed for debugging.

-    except RuntimeError as e:
+    except PalaceCompatError as e:
         print(f"\n  Error: {e}")
         sys.exit(1)

Define class PalaceCompatError(RuntimeError) in compat.py and raise that instead.

---

🎨 #6: No migration path for existing pre-metadata palaces

Location: mempalace/compat.py:ensure_palace_safe() | Confidence: ⚠️ MED

Users who upgrade to Chroma >= 1 with an existing palace (created before this PR) will hit: "Refusing to open palace without compatibility metadata under Chroma 1.x." The error tells them to "rebuild the palace" but provides no command to do so without losing data. A mempalace stamp or mempalace migrate command that writes mempalace_meta.json to an existing palace would provide a clean upgrade path.

---

🧪 #7: No test coverage for compat.py

Location: mempalace/compat.py | Confidence: ✅ HIGH

The new module has 4 public functions and multiple branching paths (Chroma version parsing, metadata read/write, the bootstrap guard logic). None are tested. Key cases:

• chromadb_major() with valid, invalid, and missing __version__
• ensure_palace_safe() with: no metadata + Chroma < 1, no metadata + Chroma >= 1, matching major, mismatched major
• read_palace_metadata() with corrupt JSON, permission errors
• write_palace_metadata() creating parent dirs

---

Flow Impact Analysis

BEFORE (no guardrails):
  CLI/MCP → PersistentClient → ChromaDB (direct, any version)

AFTER (this PR):
  CLI commands:
    cmd_search ──→ ensure_palace_safe ──→ searcher.search ──→ ensure_palace_safe ──→ PersistentClient
    cmd_mine   ──→ get_collection ──→ ensure_palace_safe ──→ PersistentClient ──→ write_metadata
    cmd_wakeup ──→ ensure_palace_safe ──→ MemoryStack ──→ ensure_palace_safe (×N layers) ──→ PersistentClient

  UNGUARDED paths (bypass):
    convo_miner.mine_convos ──→ PersistentClient (NO CHECK)
    mcp_server._get_collection ──→ PersistentClient (NO CHECK)
    palace_graph.build_graph ──→ PersistentClient (NO CHECK)

---

Created by Octocode MCP https://octocode.ai

[web3guru888]

👀 Review of #217 — Add palace compatibility guardrails

Scope: +104/−3 · 5 file(s) · touches core

• mempalace/cli.py (modified: +11/−1)
• mempalace/compat.py (added: +75/−0)
• mempalace/layers.py (modified: +6/−0)
• mempalace/miner.py (modified: +8/−2)
• ⚠️ mempalace/searcher.py (modified: +4/−0)

Technical Analysis

• 🪟 Windows compatibility — verify path handling works cross-platform

Issues

• ⚠️ Touches mempalace/searcher.py — Core search — affects all retrieval paths

Suggestions

• 💡 No tests included — consider adding coverage for the new code paths

🟡 Needs attention — touches guarded files and has items to address.

---

_{🏛️ Reviewed by MemPalace-AGI · Autonomous research system with perfect memory · Showcase: Truth Palace of Atlantis}

[yeager]

Addressed the review concerns and pushed an update.

What changed:

• added full tests for compat.py in tests/test_compat.py
• added a core integration test in tests/test_searcher.py covering incompatible Chroma major versions
• added usedforsecurity=False to the hashlib.md5() call in knowledge_graph.py for FIPS-friendly behavior

Verification:

• Windows/path handling reviewed: compat.meta_path() uses Path(...).expanduser().resolve() and does not hardcode path separators
• test suite: .venv/bin/python3 -m pytest tests/ -x -q
• result: 115 passed

Latest commit: 73fc059

[igorls]

Hi, thanks for the contribution.

This PR has merge conflicts with develop, and the branch has not been updated in over 7 days, which puts it before our most recent release. The conflicts are likely against work that landed in that release.

Could you rebase onto develop so we can take another look?

If this change is no longer relevant, feel free to close the PR.

(This message is part of a periodic backlog pass, sent to all open PRs that match this state.)