Skip to content

Commit c93528b

Browse files
author
Samuel LEGROS
committed
mcp: factor empty-filter normalization into _normalize_optional_filter helper
Follow-up to eb27e00 (port of upstream PR MemPalace#1097). The original port only fixed tool_search; tool_list_rooms and tool_find_tunnels still treated whitespace-only filters as truthy, which let an LLM-supplied wing=" " turn into a literal `WHERE wing = ' '` SQL filter that returned zero rows. Extracts the strip-and-empty-as-None logic into _normalize_optional_filter and routes every LLM-callable tool with optional filter args through it: tool_search(wing, room) — replaced inline normalization tool_list_rooms(wing) — was the latent bug tool_find_tunnels(wing_a, wing_b) — was the latent bug tool_diary_write(wing) — replaced inline normalization tool_diary_read(wing) — replaced inline normalization The helper also strips leading/trailing whitespace on real values, so wing=" wing_code " is now treated as wing="wing_code" instead of silently matching nothing. Six tests added: helper itself (3 cases), tool_list_rooms with empty + real wing (2 cases), tool_find_tunnels with empty + real wings (1 case). Found during second-order analysis of the 2026-04-25 upstream sync — caught before any LLM agent hit the latent bug in the wild.
1 parent 2e23e49 commit c93528b

2 files changed

Lines changed: 123 additions & 15 deletions

File tree

mempalace/mcp_server.py

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,20 @@ def _no_palace():
6969
}
7070

7171

72+
def _normalize_optional_filter(value):
73+
"""Treat empty/whitespace-only strings as no filter.
74+
75+
LLM agents tend to fill every optional parameter with "" rather than
76+
omitting it. Without normalization a wing/room filter of " " is
77+
truthy and reaches the DB layer as a literal filter, scoping results
78+
to a non-existent empty string and returning zero rows. Returns the
79+
stripped value when it has content, ``None`` otherwise.
80+
"""
81+
if isinstance(value, str) and not value.strip():
82+
return None
83+
return value.strip() if isinstance(value, str) else value
84+
85+
7286
# ==================== READ TOOLS ====================
7387

7488

@@ -131,6 +145,7 @@ def tool_list_wings():
131145

132146

133147
def tool_list_rooms(wing: str = None):
148+
wing = _normalize_optional_filter(wing)
134149
db = _get_db()
135150
cur = db.conn().cursor()
136151
if wing:
@@ -159,13 +174,8 @@ def tool_get_taxonomy():
159174

160175

161176
def tool_search(query: str, limit: int = 5, wing: str = None, room: str = None):
162-
# LLM agents tend to fill every optional parameter with "" rather than
163-
# omitting it. Treat empty/whitespace-only wing/room as no filter so the
164-
# search isn't silently scoped to a non-existent empty wing/room.
165-
if isinstance(wing, str) and not wing.strip():
166-
wing = None
167-
if isinstance(room, str) and not room.strip():
168-
room = None
177+
wing = _normalize_optional_filter(wing)
178+
room = _normalize_optional_filter(room)
169179
return search_memories(
170180
query,
171181
palace_path=_config.palace_path,
@@ -221,7 +231,7 @@ def tool_traverse_graph(start_room: str, max_hops: int = 2):
221231

222232
def tool_find_tunnels(wing_a: str = None, wing_b: str = None):
223233
"""Find rooms that bridge two wings — the hallways connecting domains."""
224-
return find_tunnels(wing_a, wing_b)
234+
return find_tunnels(_normalize_optional_filter(wing_a), _normalize_optional_filter(wing_b))
225235

226236

227237
def tool_graph_stats():
@@ -329,9 +339,8 @@ def tool_diary_write(agent_name: str, entry: str, topic: str = "general", wing:
329339
agent's default ``wing_<agent_name>`` wing — used by hooks to scope
330340
entries to the project the session belongs to.
331341
"""
332-
if isinstance(wing, str) and wing.strip():
333-
wing = wing.strip()
334-
else:
342+
wing = _normalize_optional_filter(wing)
343+
if not wing:
335344
wing = f"wing_{agent_name.lower().replace(' ', '_')}"
336345
db = _get_db()
337346

@@ -380,12 +389,10 @@ def tool_diary_read(agent_name: str, last_n: int = 10, wing: str = ""):
380389
PR #659 review).
381390
"""
382391
db = _get_db()
383-
use_wing_filter = isinstance(wing, str) and wing.strip()
384-
if use_wing_filter:
385-
wing = wing.strip()
392+
wing = _normalize_optional_filter(wing)
386393

387394
try:
388-
if use_wing_filter:
395+
if wing:
389396
where = {
390397
"$and": [
391398
{"wing": wing},

tests/test_mcp_server.py

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,31 @@ def fake_handler(**kwargs):
138138
assert calls == [{}]
139139

140140

141+
# ── _normalize_optional_filter: empty/whitespace = no filter (upstream #1097
142+
# pattern, factored out across every LLM-callable tool)
143+
144+
145+
def test_normalize_optional_filter_returns_none_for_empty_strings():
146+
from mempalace.mcp_server import _normalize_optional_filter
147+
148+
assert _normalize_optional_filter("") is None
149+
assert _normalize_optional_filter(" ") is None
150+
assert _normalize_optional_filter("\t\n") is None
151+
152+
153+
def test_normalize_optional_filter_strips_then_returns_value():
154+
from mempalace.mcp_server import _normalize_optional_filter
155+
156+
assert _normalize_optional_filter("wing_code") == "wing_code"
157+
assert _normalize_optional_filter(" wing_code ") == "wing_code"
158+
159+
160+
def test_normalize_optional_filter_passes_none_through():
161+
from mempalace.mcp_server import _normalize_optional_filter
162+
163+
assert _normalize_optional_filter(None) is None
164+
165+
141166
# ── tool_search: empty/whitespace wing/room treated as no filter (upstream #1097)
142167

143168

@@ -240,6 +265,82 @@ def test_diary_read_whitespace_wing_spans_all_wings_for_agent(monkeypatch):
240265
assert all("wing" not in c for c in conds)
241266

242267

268+
# ── tool_list_rooms / tool_find_tunnels: same empty-filter normalization
269+
270+
271+
def test_tool_list_rooms_empty_wing_lists_all_rooms(monkeypatch):
272+
"""tool_list_rooms with wing=" " must hit the unfiltered branch
273+
(counts rooms across all wings) instead of running
274+
`WHERE wing = ' '` and returning nothing."""
275+
captured = {"sql": None, "params": None}
276+
277+
class _FakeCursor:
278+
def execute(self, sql, params=None):
279+
captured["sql"] = sql
280+
captured["params"] = params
281+
282+
def fetchall(self):
283+
return []
284+
285+
class _FakeConn:
286+
def cursor(self):
287+
return _FakeCursor()
288+
289+
class _FakeDB:
290+
def conn(self):
291+
return _FakeConn()
292+
293+
monkeypatch.setattr(mcp_server, "_get_db", lambda: _FakeDB())
294+
295+
result = mcp_server.tool_list_rooms(wing=" ")
296+
assert result["wing"] == "all"
297+
assert "WHERE wing" not in captured["sql"]
298+
assert captured["params"] is None
299+
300+
301+
def test_tool_list_rooms_with_real_wing_filters(monkeypatch):
302+
captured = {"sql": None, "params": None}
303+
304+
class _FakeCursor:
305+
def execute(self, sql, params=None):
306+
captured["sql"] = sql
307+
captured["params"] = params
308+
309+
def fetchall(self):
310+
return []
311+
312+
class _FakeConn:
313+
def cursor(self):
314+
return _FakeCursor()
315+
316+
class _FakeDB:
317+
def conn(self):
318+
return _FakeConn()
319+
320+
monkeypatch.setattr(mcp_server, "_get_db", lambda: _FakeDB())
321+
322+
result = mcp_server.tool_list_rooms(wing=" wing_code ")
323+
assert result["wing"] == "wing_code" # stripped
324+
assert "WHERE wing" in captured["sql"]
325+
assert captured["params"] == ("wing_code",)
326+
327+
328+
def test_tool_find_tunnels_normalizes_both_wings(monkeypatch):
329+
captured = {"args": None}
330+
331+
def fake_find_tunnels(a, b):
332+
captured["args"] = (a, b)
333+
return {"tunnels": []}
334+
335+
monkeypatch.setattr(mcp_server, "find_tunnels", fake_find_tunnels)
336+
337+
mcp_server.tool_find_tunnels(wing_a="", wing_b=" ")
338+
assert captured["args"] == (None, None)
339+
340+
mcp_server.tool_find_tunnels(wing_a=" wing_code ", wing_b="wing_user")
341+
assert captured["args"] == ("wing_code", "wing_user")
342+
343+
243344
def test_tools_call_unknown_tool_returns_error():
244345
"""Unknown tool name yields a JSON-RPC error, not a crash."""
245346
request = {

0 commit comments

Comments
 (0)