Skip to content

Commit f92095e

Browse files
committed
fix(mcp): purge stale closets on drawer update and delete
Closets are mined once from a source file's content and only rebuilt by re-mining that file. tool_update_drawer and tool_delete_drawer never touched them, so a corrected or deleted drawer left a closet quoting the old text indefinitely, and search boosts ranking by source_file, so a stale closet could outrank the corrected drawer. Purge the matching source's closets through _purge_source_closets, the helper delete_by_source already uses for this reason. update_drawer purges only when content changes; a wing/room move alone leaves the quoted text correct. Fixes #2325
1 parent 88247ac commit f92095e

3 files changed

Lines changed: 93 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
1111
### Bug Fixes
1212

1313
- **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)
14+
- **`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)
1415

1516
---
1617

mempalace/mcp_server.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3218,13 +3218,24 @@ def tool_delete_drawer(drawer_id: str):
32183218
col.delete(ids=record["ids"])
32193219
_invalidate_overview_caches()
32203220

3221-
logger.info("Deleted drawer: %s (%s rows)", drawer_id, len(record["ids"]))
3221+
# Closets are keyed by source_file, not drawer_id (#1722), so a
3222+
# drawer-only delete strands a closet quoting the now-deleted text (#2325).
3223+
source_file = record["metadata"].get("source_file")
3224+
closets_deleted = _purge_source_closets(source_file, commit=True) if source_file else 0
3225+
3226+
logger.info(
3227+
"Deleted drawer: %s (%s rows, %s closet(s) purged)",
3228+
drawer_id,
3229+
len(record["ids"]),
3230+
closets_deleted,
3231+
)
32223232

32233233
return {
32243234
"success": True,
32253235
"drawer_id": drawer_id,
32263236
"deleted_ids": record["ids"],
32273237
"chunks_deleted": len(record["ids"]),
3238+
"closets_deleted": closets_deleted,
32283239
}
32293240
except Exception as e:
32303241
return {"success": False, "error": str(e)}
@@ -3833,6 +3844,13 @@ def tool_update_drawer(drawer_id: str, content: str = None, wing: str = None, ro
38333844
},
38343845
)
38353846

3847+
# A closet quotes the source file, not the stored drawer, so it only
3848+
# goes stale on a content change; wing/room alone leaves it correct (#2325).
3849+
closets_deleted = 0
3850+
source_file = old_meta.get("source_file")
3851+
if content is not None and source_file:
3852+
closets_deleted = _purge_source_closets(source_file, commit=True)
3853+
38363854
chunk_size = max(1, int(getattr(_config, "chunk_size", 800) or 800))
38373855
should_chunk = bool(record.get("chunked")) or len(new_doc) > chunk_size
38383856

@@ -3862,6 +3880,7 @@ def tool_update_drawer(drawer_id: str, content: str = None, wing: str = None, ro
38623880
"room": new_meta.get("room", ""),
38633881
"chunks": len(chunk_ids),
38643882
"chunk_ids": chunk_ids,
3883+
"closets_deleted": closets_deleted,
38653884
}
38663885

38673886
update_kwargs = {"ids": [record["ids"][0]]}
@@ -3879,6 +3898,7 @@ def tool_update_drawer(drawer_id: str, content: str = None, wing: str = None, ro
38793898
"drawer_id": drawer_id,
38803899
"wing": new_meta.get("wing", ""),
38813900
"room": new_meta.get("room", ""),
3901+
"closets_deleted": closets_deleted,
38823902
}
38833903
except Exception as e:
38843904
return {"success": False, "error": str(e)}

tests/test_mcp_server.py

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2227,6 +2227,31 @@ def test_delete_drawer_not_found(self, monkeypatch, config, palace_path, seeded_
22272227
result = tool_delete_drawer("nonexistent_drawer")
22282228
assert result["success"] is False
22292229

2230+
def test_delete_drawer_purges_matching_closets(
2231+
self, monkeypatch, config, palace_path, seeded_collection, kg
2232+
):
2233+
"""Deleting a drawer purges its source's closets too, so the AAAK
2234+
index keeps no stale pointer at the now-deleted drawer (#2325)."""
2235+
_patch_mcp_server(monkeypatch, config, kg)
2236+
from mempalace.mcp_server import tool_delete_drawer
2237+
from mempalace.palace import get_closets_collection
2238+
2239+
closets_col = get_closets_collection(palace_path, create=True)
2240+
closets_col.add(
2241+
ids=["auth_closet_01"],
2242+
documents=["topic: JWT session tokens"],
2243+
metadatas=[{"source_file": "auth.py"}],
2244+
)
2245+
2246+
result = tool_delete_drawer("drawer_proj_backend_aaa")
2247+
assert result["success"] is True
2248+
assert result["closets_deleted"] == 1
2249+
2250+
# Re-acquire: the staleness reconnect drops chromadb's path-keyed
2251+
# System cache (#2002), so a handle taken before the call is dead now.
2252+
closets_col = get_closets_collection(palace_path, create=False)
2253+
assert closets_col.get(include=[])["ids"] == []
2254+
22302255
def test_check_duplicate_handles_none_metadata(self, monkeypatch, config, kg):
22312256
"""tool_check_duplicate must tolerate None entries in the result lists
22322257
that ChromaDB 1.5.x returns for partially-flushed rows.
@@ -2880,6 +2905,52 @@ def test_update_drawer_wing_and_room(
28802905
assert result["wing"] == "new_wing"
28812906
assert result["room"] == "new_room"
28822907

2908+
def test_update_drawer_content_purges_matching_closets(
2909+
self, monkeypatch, config, palace_path, seeded_collection, kg
2910+
):
2911+
"""Correcting a drawer's content purges its source's closets, which
2912+
otherwise keep quoting the pre-correction text indefinitely (#2325)."""
2913+
_patch_mcp_server(monkeypatch, config, kg)
2914+
from mempalace.mcp_server import tool_update_drawer
2915+
from mempalace.palace import get_closets_collection
2916+
2917+
closets_col = get_closets_collection(palace_path, create=True)
2918+
closets_col.add(
2919+
ids=["auth_closet_01"],
2920+
documents=["topic: JWT session tokens"],
2921+
metadatas=[{"source_file": "auth.py"}],
2922+
)
2923+
2924+
result = tool_update_drawer("drawer_proj_backend_aaa", content="[RETRACTED]")
2925+
assert result["success"] is True
2926+
assert result["closets_deleted"] == 1
2927+
2928+
closets_col = get_closets_collection(palace_path, create=False)
2929+
assert closets_col.get(include=[])["ids"] == []
2930+
2931+
def test_update_drawer_wing_and_room_does_not_purge_closets(
2932+
self, monkeypatch, config, palace_path, seeded_collection, kg
2933+
):
2934+
"""A wing/room move alone leaves the quoted text correct, so it must
2935+
not purge closets the way a content edit does (#2325)."""
2936+
_patch_mcp_server(monkeypatch, config, kg)
2937+
from mempalace.mcp_server import tool_update_drawer
2938+
from mempalace.palace import get_closets_collection
2939+
2940+
closets_col = get_closets_collection(palace_path, create=True)
2941+
closets_col.add(
2942+
ids=["auth_closet_01"],
2943+
documents=["topic: JWT session tokens"],
2944+
metadatas=[{"source_file": "auth.py"}],
2945+
)
2946+
2947+
result = tool_update_drawer("drawer_proj_backend_aaa", wing="new_wing", room="new_room")
2948+
assert result["success"] is True
2949+
assert result["closets_deleted"] == 0
2950+
2951+
closets_col = get_closets_collection(palace_path, create=False)
2952+
assert len(closets_col.get(include=[])["ids"]) == 1
2953+
28832954
def test_update_drawer_not_found(self, monkeypatch, config, palace_path, seeded_collection, kg):
28842955
_patch_mcp_server(monkeypatch, config, kg)
28852956
from mempalace.mcp_server import tool_update_drawer

0 commit comments

Comments
 (0)