Skip to content
Merged
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
32 changes: 23 additions & 9 deletions mempalace/mcp_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -1864,7 +1864,7 @@ def tool_kg_stats():
# ==================== AGENT DIARY ====================


def tool_diary_write(agent_name: str, entry: str = None, topic: str = "general", wing: str = "", content: str = None):
def tool_diary_write(agent_name: str, entry: str, topic: str = "general", wing: str = ""):
"""
Write a diary entry for this agent. Entries are timestamped and
accumulate over time in a diary room.
Expand All @@ -1876,13 +1876,6 @@ def tool_diary_write(agent_name: str, entry: str = None, topic: str = "general",
that diary reads are case-insensitive (see #1243). "Claude",
"claude", and "CLAUDE" all resolve to the same agent.
"""
# Accept 'content' as an alias for 'entry' — add_drawer uses 'content', making
# it natural to pattern off it. Accepting both avoids a silent -32000 error.
if entry is None and content is not None:
entry = content
elif entry is None:
return {"success": False, "error": "'entry' (or 'content') is required"}

try:
agent_name = sanitize_name(agent_name, "agent_name").lower()
entry = sanitize_content(entry)
Expand Down Expand Up @@ -2664,8 +2657,18 @@ def tool_reconnect():
"type": "string",
"description": "Target wing for this diary entry (optional). If omitted, uses wing_{agent_name}. Use this to write diary entries to a project wing instead of an agent-specific wing.",
},
"content": {
"type": "string",
"description": "Alias for 'entry' — accepted because add_drawer uses 'content'. Provide either 'entry' or 'content'; 'entry' wins if both are given.",
},
},
"required": ["agent_name", "entry"],
# agent_name is always required; 'entry' or its alias 'content' must
# be present (the server remaps content->entry at dispatch).
"required": ["agent_name"],
"anyOf": [
{"required": ["entry"]},
{"required": ["content"]},
],
},
Comment on lines 2664 to 2672
"handler": tool_diary_write,
},
Expand Down Expand Up @@ -2872,6 +2875,17 @@ def handle_request(request):
"error": {"code": -32602, "message": f"Invalid value for parameter '{key}'"},
}
tool_args.pop("wait_for_previous", None)
# 'content' is an accepted alias for diary_write's 'entry' (callers often
# reuse add_drawer's 'content' name). Map it in here, before dispatch, so a
# content-only call still satisfies the required 'entry' param while the
# signature-based missing-parameter diagnostic (-32602) keeps working.
# 'entry' wins if both are supplied.
if tool_name == "mempalace_diary_write" and "content" in tool_args:
content_val = tool_args.pop("content")
# Only fill from the alias when the caller did not supply 'entry' at
# all (or passed it as null). An explicit entry — even "" — wins.
if "entry" not in tool_args or tool_args["entry"] is None:
tool_args["entry"] = content_val
Comment on lines +2883 to +2888
try:
result = TOOLS[tool_name]["handler"](**tool_args)
return {
Expand Down
84 changes: 46 additions & 38 deletions mempalace/searcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -870,6 +870,49 @@ def _open_search_collection(palace_path: str, collection_name: str):
}


def _query_drawers_with_filter_fallback(drawers_col, dkwargs, query, n_results, wing, room):
"""Run the filtered drawer query, falling back to an unfiltered query plus a
Python-side post-filter when ChromaDB raises on the filtered query.

A ChromaDB HNSW/SQLite index mismatch makes filtered queries fail with
"Error finding id" even when unfiltered search works fine — it happens when
drawers are ingested via two different paths (e.g. bulk import vs MCP tool
calls), leaving the vector index inconsistent with the metadata store. We
retry unfiltered (over-fetching) and re-apply the wing/room filter in Python.
See #1245 / #1035.
"""
where = dkwargs.get("where")
try:
return drawers_col.query(**dkwargs)
except Exception as filter_err:
if not where:
raise
logger.warning(
"Filtered search failed (%s); falling back to unfiltered + post-filter",
filter_err,
)
raw = drawers_col.query(
query_texts=[query],
n_results=min(n_results * 15, 500),
include=["documents", "metadatas", "distances"],
)
fdocs, fmetas, fdists = [], [], []
for doc, meta, dist in zip(
_first_or_empty(raw, "documents"),
_first_or_empty(raw, "metadatas"),
_first_or_empty(raw, "distances"),
):
meta = meta or {}
if wing and meta.get("wing") != wing:
continue
if room and meta.get("room") != room:
continue
fdocs.append(doc)
fmetas.append(meta)
fdists.append(dist)
return {"documents": [fdocs], "metadatas": [fmetas], "distances": [fdists]}


def search_memories(
query: str,
palace_path: str,
Expand Down Expand Up @@ -952,44 +995,9 @@ def search_memories(
}
if where:
dkwargs["where"] = where
try:
drawer_results = drawers_col.query(**dkwargs)
except Exception as filter_err:
if not where:
raise
# ChromaDB HNSW/SQLite index mismatch causes filtered queries to fail with
# "Error finding id" even when unfiltered search works fine. This happens when
# drawers are ingested via two different paths (e.g. bulk import vs MCP tool
# calls), leaving the vector index inconsistent with the metadata store.
# Workaround: retry unfiltered, then post-filter in Python.
logger.warning(
"Filtered search failed (%s); falling back to unfiltered + post-filter",
filter_err,
)
fallback_kwargs = {
"query_texts": [query],
"n_results": min(n_results * 15, 500),
"include": ["documents", "metadatas", "distances"],
}
raw = drawers_col.query(**fallback_kwargs)
wing_f, room_f = wing, room
fdocs, fmetas, fdists = [], [], []
for doc, meta, dist in zip(
raw["documents"][0], raw["metadatas"][0], raw["distances"][0]
):
meta = meta or {}
if wing_f and meta.get("wing") != wing_f:
continue
if room_f and meta.get("room") != room_f:
continue
fdocs.append(doc)
fmetas.append(meta)
fdists.append(dist)
drawer_results = {
"documents": [fdocs],
"metadatas": [fmetas],
"distances": [fdists],
}
drawer_results = _query_drawers_with_filter_fallback(
drawers_col, dkwargs, query, n_results, wing, room
)
except Exception as e:
return {"error": f"Search error: {e}"}

Expand Down
82 changes: 82 additions & 0 deletions tests/test_mcp_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -3323,6 +3323,88 @@ def test_two_missing_required_lists_both_names(self):
assert "'entry'" in message
assert " and " not in message.split("for tool")[0]

def test_diary_write_content_aliases_entry(self, monkeypatch):
"""A content-only diary_write call is remapped to 'entry' before
dispatch (#1245 alias), so it satisfies the required param and the
alias key is consumed rather than passed through to the handler.
"""
from mempalace import mcp_server

captured = {}

def capture(**kwargs):
captured.update(kwargs)
return {"success": True}

monkeypatch.setitem(mcp_server.TOOLS["mempalace_diary_write"], "handler", capture)
resp = mcp_server.handle_request(
{
"method": "tools/call",
"id": 5,
"params": {
"name": "mempalace_diary_write",
"arguments": {"agent_name": "test", "content": "hello world"},
},
}
)
assert "error" not in resp
assert captured.get("entry") == "hello world"
assert "content" not in captured

def test_diary_write_entry_wins_over_content(self, monkeypatch):
"""When both 'entry' and the 'content' alias are supplied, 'entry' wins
and the alias is dropped.
"""
from mempalace import mcp_server

captured = {}

def capture(**kwargs):
captured.update(kwargs)
return {"success": True}

monkeypatch.setitem(mcp_server.TOOLS["mempalace_diary_write"], "handler", capture)
resp = mcp_server.handle_request(
{
"method": "tools/call",
"id": 6,
"params": {
"name": "mempalace_diary_write",
"arguments": {"agent_name": "t", "entry": "real", "content": "alias"},
},
}
)
assert "error" not in resp
assert captured.get("entry") == "real"
assert "content" not in captured

def test_diary_write_explicit_empty_entry_not_overridden_by_content(self, monkeypatch):
"""An explicitly supplied (even falsy "") 'entry' wins over 'content' —
the alias only fills in when 'entry' is absent or null, not merely falsy.
"""
from mempalace import mcp_server

captured = {}

def capture(**kwargs):
captured.update(kwargs)
return {"success": True}

monkeypatch.setitem(mcp_server.TOOLS["mempalace_diary_write"], "handler", capture)
resp = mcp_server.handle_request(
{
"method": "tools/call",
"id": 7,
"params": {
"name": "mempalace_diary_write",
"arguments": {"agent_name": "t", "entry": "", "content": "alias"},
},
}
)
assert "error" not in resp
assert captured.get("entry") == ""
assert "content" not in captured

def test_handler_internal_signature_shape_stays_generic(self, monkeypatch):
"""A TypeError whose function name does not match the dispatched
handler — e.g. raised by a helper called inside the handler body —
Expand Down
Loading