fix(mcp): diary_read(wing='') spans all wings for agent (#1145)
#1097 fixed mempalace_search to treat empty-string wing/room as no filter, matching how LLM agents default to filling every optional parameter with ''. The same pattern wasn't applied to diary_read: passing wing='' defaulted to wing_<agent_name>, siloing away entries that hooks had written to project-derived wings per #659. When wing is empty/omitted, filter only on agent + room=diary so callers get a unified view of the agent's journal across every wing it has written to. Explicit wing=<name> continues to scope reads to that wing only. Adds test covering empty-wing read after writing to both the default and a non-default wing.
This commit is contained in:
+13
-13
@@ -995,10 +995,11 @@ def tool_diary_read(agent_name: str, last_n: int = 10, wing: str = ""):
|
|||||||
Read an agent's recent diary entries. Returns the last N entries
|
Read an agent's recent diary entries. Returns the last N entries
|
||||||
in chronological order — the agent's personal journal.
|
in chronological order — the agent's personal journal.
|
||||||
|
|
||||||
When ``wing`` is provided, reads from that wing instead of the
|
When ``wing`` is provided, reads only from that wing. When ``wing``
|
||||||
agent's default ``wing_<agent_name>`` wing. This lets hooks
|
is empty or omitted, returns entries from every wing this agent has
|
||||||
direct diary reads to a project-specific wing derived from
|
written to. Diary writes from hooks land in project-derived wings
|
||||||
the transcript path.
|
(``wing_<project>``), so requiring a specific wing on read would
|
||||||
|
silo those entries from agent-initiated reads.
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
agent_name = sanitize_name(agent_name, "agent_name")
|
agent_name = sanitize_name(agent_name, "agent_name")
|
||||||
@@ -1007,21 +1008,20 @@ def tool_diary_read(agent_name: str, last_n: int = 10, wing: str = ""):
|
|||||||
except ValueError as e:
|
except ValueError as e:
|
||||||
return {"error": str(e)}
|
return {"error": str(e)}
|
||||||
last_n = max(1, min(last_n, 100))
|
last_n = max(1, min(last_n, 100))
|
||||||
if not wing:
|
|
||||||
wing = f"wing_{agent_name.lower().replace(' ', '_')}"
|
|
||||||
col = _get_collection()
|
col = _get_collection()
|
||||||
if not col:
|
if not col:
|
||||||
return _no_palace()
|
return _no_palace()
|
||||||
|
|
||||||
|
# Build filter: always scope by agent + room=diary. Wing is optional —
|
||||||
|
# when empty, return entries across all wings for this agent (matches
|
||||||
|
# the #1097 empty-string-as-no-filter convention for LLM ergonomics).
|
||||||
|
conditions = [{"room": "diary"}, {"agent": agent_name}]
|
||||||
|
if wing:
|
||||||
|
conditions.insert(0, {"wing": wing})
|
||||||
|
|
||||||
try:
|
try:
|
||||||
results = col.get(
|
results = col.get(
|
||||||
where={
|
where={"$and": conditions},
|
||||||
"$and": [
|
|
||||||
{"wing": wing},
|
|
||||||
{"room": "diary"},
|
|
||||||
{"agent": agent_name},
|
|
||||||
]
|
|
||||||
},
|
|
||||||
include=["documents", "metadatas"],
|
include=["documents", "metadatas"],
|
||||||
limit=10000,
|
limit=10000,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -740,6 +740,40 @@ class TestDiaryTools:
|
|||||||
assert entry1 in contents
|
assert entry1 in contents
|
||||||
assert entry2 in contents
|
assert entry2 in contents
|
||||||
|
|
||||||
|
def test_diary_read_empty_wing_spans_all_wings(self, monkeypatch, config, palace_path, kg):
|
||||||
|
"""diary_read(wing='') must return entries from every wing this agent
|
||||||
|
wrote to. Hooks write to project-derived wings (#659); a reader that
|
||||||
|
silos by default wing would never see those entries."""
|
||||||
|
_patch_mcp_server(monkeypatch, config, kg)
|
||||||
|
_client, _col = _get_collection(palace_path, create=True)
|
||||||
|
del _client
|
||||||
|
from mempalace.mcp_server import tool_diary_read, tool_diary_write
|
||||||
|
|
||||||
|
w1 = tool_diary_write(
|
||||||
|
agent_name="TestAgent",
|
||||||
|
entry="default-wing entry",
|
||||||
|
topic="general",
|
||||||
|
)
|
||||||
|
w2 = tool_diary_write(
|
||||||
|
agent_name="TestAgent",
|
||||||
|
entry="project-wing entry",
|
||||||
|
topic="general",
|
||||||
|
wing="wing_someproject",
|
||||||
|
)
|
||||||
|
assert w1["success"] and w2["success"]
|
||||||
|
|
||||||
|
# Empty wing → return both entries
|
||||||
|
r = tool_diary_read(agent_name="TestAgent", wing="")
|
||||||
|
assert r["total"] == 2
|
||||||
|
contents = {e["content"] for e in r["entries"]}
|
||||||
|
assert "default-wing entry" in contents
|
||||||
|
assert "project-wing entry" in contents
|
||||||
|
|
||||||
|
# Explicit wing → return only that wing's entries
|
||||||
|
r_scoped = tool_diary_read(agent_name="TestAgent", wing="wing_someproject")
|
||||||
|
assert r_scoped["total"] == 1
|
||||||
|
assert r_scoped["entries"][0]["content"] == "project-wing entry"
|
||||||
|
|
||||||
|
|
||||||
# ── Cache Invalidation (inode/mtime) ──────────────────────────────────
|
# ── Cache Invalidation (inode/mtime) ──────────────────────────────────
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user