Skip to content
Open
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
### Bug Fixes

- **A `config.json` this process could not read is no longer written over.** `MempalaceConfig` reads the file once and falls back to an empty dict on any failure, and every setter then serializes that dict back over the whole file. One unreadable read was therefore enough to lose every setting the file held: a config hand-edited into something that does not parse, one trailing comma or a byte-order mark an editor added, went from 219 bytes to `{"backend": "chroma"}` at the next `mempalace init --backend`, with `palace_path` back to the default and `people_map` gone. Such files also come from the setters themselves, since one `set_hook_setting` on a 3.6 MB config is a window during which the file on disk is not the config, and ten of ten runs killed inside it left a truncated file behind. The file is now read as bytes, and only `FileNotFoundError` counts as "there is no config here". A file that does not parse is renamed aside, with its new name printed, before a fresh one is written; a file that exists and cannot be read at all is not touched and the setter says so instead of returning as though it had saved. Writes go through a temporary file named after this process and are renamed into place, so an interrupted write leaves the previous config exactly where it was, and a setter that cannot write reports it rather than swallowing the error. `people_map.json` is written the same way. A config reached through a symlink is written through it rather than replaced, so a dotfiles checkout keeps receiving the settings; a directory that will not take a temporary file gets the write in place with a message rather than losing the setting; a UTF-8 byte-order mark is no longer read as a parse failure; and a setter that has to create `~/.mempalace` restricts it to the owner. (#2356)
- **Tunnel reads/deletes and MCP hallway reads/deletes now stay scoped to the selected palace.** `create_tunnel(..., config=...)` already wrote to that palace's `tunnels.json`, but `follow_tunnels` silently read the ambient default while `list_tunnels` and `delete_tunnel` could not accept the config at all. In multi-palace callers this made a successful write appear missing and could delete a same-ID tunnel from another palace. All tunnel helpers and their MCP handlers now propagate one canonical config through load, lock, and save; the adjacent hallway list/delete handlers also forward the MCP server's selected config instead of reading or mutating the ambient palace. (#2263)
- **A palace with no database is no longer reported as one that passed its integrity check.** `sqlite_integrity_errors` answers `[]` when `chroma.sqlite3` is absent, and the MCP gate published that as `checked: true, ok: true`. Absence is now decided by `ENOENT` alone, which proves that nothing resolves under the path, and reported as the not-applicable shape #1931 introduced, `checked: false`/`ok: null` plus a reason. Every state that is not proven absent reaches the probe, and a probe that cannot open the file reports `PRAGMA quick_check failed`, which trips the existing `-32002` refusal: a dangling symlink, a database under an unreadable directory, a symlink loop, a name the filesystem rejects, an embedded NUL in the path, and, on POSIX, a palace path whose parent is a file. A palace directory named with a byte that is not valid UTF-8 reached the probe and, up to Python 3.12, raised out of it, which `mempalace mine` and `mempalace repair` never guarded against; it is now reported like every other unreadable path. `/statusz` reads an absent verdict as healthy, so the new `ok: null` does not turn a fresh install red, and non-chroma backends stop reporting themselves unhealthy, which they had done since the #1931 fix. The size-limited startup skip still publishes a clean verdict; the only change there is that it no longer inherits the previous probe's absence reason. (#2290)
- **`update_drawer` and `delete_drawer` no longer strand stale closets at the mutated drawer's source.** Closets are the AAAK search-index layer, built once from a source file's raw content at mine time, and neither per-drawer mutation tool ever touched them: correcting a drawer in place left its closet quoting the pre-correction text indefinitely, and deleting a drawer left its closet's `->drawer_id` pointer dangling. `search_memories` boosts ranking by `source_file` and previews the closet document verbatim, so a retracted instruction could keep out-ranking its own corrected drawer. Both tools now purge the matching source's closets through the existing `_purge_source_closets` helper (already used by `delete_by_source`, #1722) rather than rebuild them, since closets are LLM-derived from file content this call path does not have: `delete_drawer` purges unconditionally, `update_drawer` only when `content` actually changes (a wing/room move alone leaves the quoted text correct). Rebuilding stale closets from the stored drawers, rather than only purging them, is a separate, larger change. (#2325)
- **`mempalace_mine` accepts a single conversation file again, so hook transcript ingest survives a running hub.** `cli.py` has always documented the mine source as "Directory to mine, or one conversation file with `--mode convos`", and `hooks_cli._ingest_transcript` submits exactly one `.jsonl`. The MCP tool validated `os.path.isdir` regardless of mode, and `cmd_mine` forwards to the hub whenever one is registered and healthy, so the documented single-file form was unreachable in the configuration most users run: every Stop and PreCompact transcript ingest failed with `source directory not found`. Nothing surfaced it, because `hook_precompact` returns the same empty object on the success path, leaving a compaction that captured nothing indistinguishable from one that captured everything. `convos` now accepts a file or a directory; the tree-walking modes still require a directory. (#2281)
Expand Down
11 changes: 6 additions & 5 deletions mempalace/mcp_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -2682,6 +2682,7 @@ def tool_create_tunnel(
label=label,
source_drawer_id=source_drawer_id,
target_drawer_id=target_drawer_id,
config=_config,
)
except ValueError as e:
return {"error": str(e)}
Expand All @@ -2693,14 +2694,14 @@ def tool_list_tunnels(wing: str = None):
wing = _sanitize_optional_name(wing, "wing")
except ValueError as e:
return {"error": str(e)}
return list_tunnels(wing)
return list_tunnels(wing, config=_config)


def tool_delete_tunnel(tunnel_id: str):
"""Delete an explicit tunnel by its ID."""
if not tunnel_id or not isinstance(tunnel_id, str):
return {"error": "tunnel_id is required"}
return delete_tunnel(tunnel_id)
return delete_tunnel(tunnel_id, config=_config)


def tool_list_hallways(wing: str = None):
Expand All @@ -2709,14 +2710,14 @@ def tool_list_hallways(wing: str = None):
wing = _sanitize_optional_name(wing, "wing")
except ValueError as e:
return {"error": str(e)}
return list_hallways(wing)
return list_hallways(wing, config=_config)


def tool_delete_hallway(hallway_id: str):
"""Delete a hallway record by its ID."""
if not hallway_id or not isinstance(hallway_id, str):
return {"error": "hallway_id is required"}
return {"deleted": delete_hallway(hallway_id)}
return {"deleted": delete_hallway(hallway_id, config=_config)}


def tool_follow_tunnels(wing: str, room: str):
Expand All @@ -2729,7 +2730,7 @@ def tool_follow_tunnels(wing: str, room: str):
col = _get_collection()
if not col:
return _collection_error_or_no_palace()
return follow_tunnels(wing, room, col=col)
return follow_tunnels(wing, room, col=col, config=_config)


# ==================== WRITE TOOLS ====================
Expand Down
29 changes: 20 additions & 9 deletions mempalace/palace_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -737,14 +737,19 @@ def create_tunnel(
return tunnel


def list_tunnels(wing: str = None):
def list_tunnels(wing: str = None, config=None):
"""List all explicit tunnels, optionally filtered by wing.

Returns tunnels where ``wing`` appears as either source or target
(tunnels are symmetric, so either endpoint is a valid filter match).

Args:
wing: Optional source or target wing filter.
config: Optional ``MempalaceConfig`` selecting the palace tunnel
sidecar. Explicit-path callers must pass the matching config.
"""
norm_wing = _normalize_wing(wing)
tunnels = _load_tunnels()
tunnels = _load_tunnels(config)
if norm_wing:
# Normalize stored wings too: older tunnels.json records hold the
# underscore form (from the prior write-path normalization), while
Expand All @@ -761,28 +766,34 @@ def list_tunnels(wing: str = None):
return tunnels


def delete_tunnel(tunnel_id: str):
"""Delete an explicit tunnel by ID. Returns ``{"deleted": <id>}``."""
with mine_lock(_get_tunnel_file()):
tunnels = _load_tunnels()
def delete_tunnel(tunnel_id: str, config=None):
"""Delete an explicit tunnel by ID from the selected palace sidecar.

Returns ``{"deleted": <id>}``.
"""
config = config or MempalaceConfig()
with mine_lock(_get_tunnel_file(config)):
tunnels = _load_tunnels(config)
tunnels = [t for t in tunnels if t.get("id") != tunnel_id]
_save_tunnels(tunnels)
_save_tunnels(tunnels, config)
return {"deleted": tunnel_id}


def follow_tunnels(wing: str, room: str, col=None, config=None):
"""Follow explicit tunnels from a room — returns connected drawers.

Given a location (wing/room), finds all tunnels leading from or to it,
and optionally fetches the connected drawer content.
and optionally fetches the connected drawer content. ``config`` selects
the palace tunnel sidecar; explicit-path callers must pass the matching
config.
"""
# Fall back to raw ``wing`` so an empty/whitespace query string still
# produces a value to compare with; ``_normalize_wing`` returns ``None``
# for empty input. Stored wings are normalized on the read path so the
# mempalace.yaml slug (underscore) and an explicit ``--wing`` slug
# (verbatim) both resolve through the same comparison.
norm_wing = _normalize_wing(wing) or wing
tunnels = _load_tunnels()
tunnels = _load_tunnels(config)
connections = []

for t in tunnels:
Expand Down
85 changes: 85 additions & 0 deletions tests/test_mcp_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -3046,6 +3046,50 @@ def _raise(*args, **kwargs):

assert result == {"error": msg}

def test_tunnel_tools_forward_server_config(self, monkeypatch):
"""Every tunnel handler must stay scoped to the MCP server's palace."""
from mempalace import mcp_server

config = object()
collection = object()
seen = {}

monkeypatch.setattr(mcp_server, "_config", config)
monkeypatch.setattr(mcp_server, "_get_collection", lambda: collection)

def fake_create(*args, config=None, **kwargs):
seen["create"] = config
return {"id": "tunnel_1"}

def fake_list(*args, config=None, **kwargs):
seen["list"] = config
return []

def fake_delete(*args, config=None, **kwargs):
seen["delete"] = config
return {"deleted": "tunnel_1"}

def fake_follow(*args, config=None, **kwargs):
seen["follow"] = config
return []

monkeypatch.setattr(mcp_server, "create_tunnel", fake_create)
monkeypatch.setattr(mcp_server, "list_tunnels", fake_list)
monkeypatch.setattr(mcp_server, "delete_tunnel", fake_delete)
monkeypatch.setattr(mcp_server, "follow_tunnels", fake_follow)

mcp_server.tool_create_tunnel("wing_a", "room_a", "wing_b", "room_b")
mcp_server.tool_list_tunnels("wing_a")
mcp_server.tool_delete_tunnel("tunnel_1")
mcp_server.tool_follow_tunnels("wing_a", "room_a")

assert seen == {
"create": config,
"list": config,
"delete": config,
"follow": config,
}

# ── hallway MCP tools (mirror the tunnel pattern) ──

def _seed_hallways(self, monkeypatch, tmp_path):
Expand Down Expand Up @@ -3080,6 +3124,47 @@ def _seed_hallways(self, monkeypatch, tmp_path):
hallways._save_hallways(seeded)
return seeded

def _seed_two_hallway_palaces(self, monkeypatch, tmp_path):
"""Seed the same hallway ID in selected A and ambient B."""
from mempalace import config as config_module
from mempalace import hallways, mcp_server
from mempalace.config import MempalaceConfig

config_a = MempalaceConfig(palace_path=tmp_path / "palace-a" / "palace")
config_b = MempalaceConfig(palace_path=tmp_path / "palace-b" / "palace")
hallway_id = "hallway_wing_shared_X_Y_same"
hallways._save_hallways(
[{"id": hallway_id, "wing": "wing_shared", "label": "A-only"}],
config=config_a,
)
hallways._save_hallways(
[{"id": hallway_id, "wing": "wing_shared", "label": "B-only"}],
config=config_b,
)

monkeypatch.setattr(config_module, "MempalaceConfig", lambda *args, **kwargs: config_b)
monkeypatch.setattr(mcp_server, "_config", config_a)
return config_a, config_b, hallway_id

def test_tool_list_hallways_reads_only_server_palace(self, monkeypatch, tmp_path):
"""The MCP-selected palace wins over the ambient config fallback."""
from mempalace import hallways, mcp_server

_, config_b, _ = self._seed_two_hallway_palaces(monkeypatch, tmp_path)

assert [item["label"] for item in mcp_server.tool_list_hallways()] == ["A-only"]
assert [item["label"] for item in hallways.list_hallways(config=config_b)] == ["B-only"]

def test_tool_delete_hallway_mutates_only_server_palace(self, monkeypatch, tmp_path):
"""A same-ID hallway in the ambient palace must survive MCP deletion."""
from mempalace import hallways, mcp_server

config_a, config_b, hallway_id = self._seed_two_hallway_palaces(monkeypatch, tmp_path)

assert mcp_server.tool_delete_hallway(hallway_id) == {"deleted": True}
assert hallways.list_hallways(config=config_a) == []
assert [item["label"] for item in hallways.list_hallways(config=config_b)] == ["B-only"]

def test_tool_list_hallways_returns_all_without_filter(self, monkeypatch, tmp_path):
"""tool_list_hallways with no wing returns every record."""
from mempalace import mcp_server
Expand Down
56 changes: 56 additions & 0 deletions tests/test_palace_graph_tunnels.py
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,62 @@ def test_no_legacy_warning_when_paths_match(self, tmp_path, monkeypatch, caplog)

assert "Legacy tunnels file" not in caplog.text

@staticmethod
def _seed_two_palaces(tmp_path, monkeypatch):
"""Create the same tunnel ID in two isolated sidecars.

Palace B is the ambient fallback. Explicit ``config=A`` calls must
never read or mutate B, even though both files contain the same ID.
"""
from mempalace.config import MempalaceConfig

config_a = MempalaceConfig(palace_path=tmp_path / "palace-a" / "palace")
config_b = MempalaceConfig(palace_path=tmp_path / "palace-b" / "palace")
tunnel_a = palace_graph.create_tunnel(
"wing_alpha",
"topic:shared",
"wing_beta",
"topic:shared",
label="A-only",
kind="topic",
config=config_a,
)
tunnel_b = palace_graph.create_tunnel(
"wing_alpha",
"topic:shared",
"wing_beta",
"topic:shared",
label="B-only",
kind="topic",
config=config_b,
)
assert tunnel_a["id"] == tunnel_b["id"]

monkeypatch.setattr(palace_graph, "MempalaceConfig", lambda: config_b)
return config_a, config_b, tunnel_a["id"]

def test_list_tunnels_reads_only_selected_palace(self, tmp_path, monkeypatch):
config_a, config_b, _ = self._seed_two_palaces(tmp_path, monkeypatch)

assert [t["label"] for t in palace_graph.list_tunnels(config=config_a)] == ["A-only"]
assert [t["label"] for t in palace_graph.list_tunnels(config=config_b)] == ["B-only"]

def test_follow_tunnels_reads_only_selected_palace(self, tmp_path, monkeypatch):
config_a, config_b, _ = self._seed_two_palaces(tmp_path, monkeypatch)

followed_a = palace_graph.follow_tunnels("wing_alpha", "topic:shared", config=config_a)
followed_b = palace_graph.follow_tunnels("wing_alpha", "topic:shared", config=config_b)

assert [t["label"] for t in followed_a] == ["A-only"]
assert [t["label"] for t in followed_b] == ["B-only"]

def test_delete_tunnel_mutates_only_selected_palace(self, tmp_path, monkeypatch):
config_a, config_b, tunnel_id = self._seed_two_palaces(tmp_path, monkeypatch)

assert palace_graph.delete_tunnel(tunnel_id, config=config_a) == {"deleted": tunnel_id}
assert palace_graph._load_tunnels(config_a) == []
assert [t["label"] for t in palace_graph._load_tunnels(config_b)] == ["B-only"]


# =============================================================================
# Regression: create_tunnel validates explicit-tunnel endpoints (#1468)
Expand Down