fix(mcp): guard tool_status/list_wings/list_rooms/get_taxonomy against None metadata
Four more MCP handlers iterate a metadata list and call m.get(...)
unconditionally. When the cache contains a None entry (drawers with no
metadata, common on older mining paths), the try block catches the
AttributeError and marks the response "partial: true" with an
error message — visible as {"error": "'NoneType' object has no
attribute 'get'", "partial": true} returned from mempalace_status even
though the palace data is otherwise fetchable.
Same m = m or {} guard we applied to searcher.py (d3a2d22, a51c3c2)
and miner.status() (66f08a1). None-metadata drawers now roll up under
the existing "unknown" fallback bucket instead of poisoning the
response with a misleading partial flag.
Regression test: mock the metadata cache with a None in the middle,
assert tool_status returns clean counts and no error/partial fields.
Verified the test fails without the guard.
998 tests pass.
This commit is contained in:
@@ -315,6 +315,7 @@ def tool_status():
|
|||||||
try:
|
try:
|
||||||
all_meta = _get_cached_metadata(col)
|
all_meta = _get_cached_metadata(col)
|
||||||
for m in all_meta:
|
for m in all_meta:
|
||||||
|
m = m or {}
|
||||||
w = m.get("wing", "unknown")
|
w = m.get("wing", "unknown")
|
||||||
r = m.get("room", "unknown")
|
r = m.get("room", "unknown")
|
||||||
wings[w] = wings.get(w, 0) + 1
|
wings[w] = wings.get(w, 0) + 1
|
||||||
@@ -368,6 +369,7 @@ def tool_list_wings():
|
|||||||
try:
|
try:
|
||||||
all_meta = _get_cached_metadata(col)
|
all_meta = _get_cached_metadata(col)
|
||||||
for m in all_meta:
|
for m in all_meta:
|
||||||
|
m = m or {}
|
||||||
w = m.get("wing", "unknown")
|
w = m.get("wing", "unknown")
|
||||||
wings[w] = wings.get(w, 0) + 1
|
wings[w] = wings.get(w, 0) + 1
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
@@ -391,6 +393,7 @@ def tool_list_rooms(wing: str = None):
|
|||||||
where = {"wing": wing} if wing else None
|
where = {"wing": wing} if wing else None
|
||||||
all_meta = _fetch_all_metadata(col, where=where)
|
all_meta = _fetch_all_metadata(col, where=where)
|
||||||
for m in all_meta:
|
for m in all_meta:
|
||||||
|
m = m or {}
|
||||||
r = m.get("room", "unknown")
|
r = m.get("room", "unknown")
|
||||||
rooms[r] = rooms.get(r, 0) + 1
|
rooms[r] = rooms.get(r, 0) + 1
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
@@ -409,6 +412,7 @@ def tool_get_taxonomy():
|
|||||||
try:
|
try:
|
||||||
all_meta = _get_cached_metadata(col)
|
all_meta = _get_cached_metadata(col)
|
||||||
for m in all_meta:
|
for m in all_meta:
|
||||||
|
m = m or {}
|
||||||
w = m.get("wing", "unknown")
|
w = m.get("wing", "unknown")
|
||||||
r = m.get("room", "unknown")
|
r = m.get("room", "unknown")
|
||||||
if w not in taxonomy:
|
if w not in taxonomy:
|
||||||
|
|||||||
@@ -250,6 +250,38 @@ class TestReadTools:
|
|||||||
assert "project" in result["wings"]
|
assert "project" in result["wings"]
|
||||||
assert "notes" in result["wings"]
|
assert "notes" in result["wings"]
|
||||||
|
|
||||||
|
def test_status_handles_none_metadata_without_partial(
|
||||||
|
self, monkeypatch, config, palace_path, kg
|
||||||
|
):
|
||||||
|
"""tool_status must not crash or go partial when the metadata cache
|
||||||
|
returns a ``None`` entry — palaces can contain drawers with no
|
||||||
|
metadata (older mining paths, third-party writes). Before the guard,
|
||||||
|
``m.get("wing")`` raised AttributeError mid-tally and the result
|
||||||
|
carried ``"error"`` + ``"partial": True`` even though the data was
|
||||||
|
perfectly fetchable."""
|
||||||
|
from unittest.mock import patch as _patch
|
||||||
|
|
||||||
|
_patch_mcp_server(monkeypatch, config, kg)
|
||||||
|
from mempalace.mcp_server import tool_status
|
||||||
|
|
||||||
|
# Inject a metadata cache where one entry is None
|
||||||
|
with _patch("mempalace.mcp_server._get_collection") as mock_get_col:
|
||||||
|
fake_col = type("C", (), {"count": lambda self: 2})()
|
||||||
|
mock_get_col.return_value = fake_col
|
||||||
|
with _patch(
|
||||||
|
"mempalace.mcp_server._get_cached_metadata",
|
||||||
|
return_value=[{"wing": "proj", "room": "r"}, None],
|
||||||
|
):
|
||||||
|
result = tool_status()
|
||||||
|
|
||||||
|
# The None-metadata drawer falls under 'unknown/unknown' — no crash,
|
||||||
|
# no partial flag.
|
||||||
|
assert "error" not in result
|
||||||
|
assert result.get("partial") is not True
|
||||||
|
assert result["total_drawers"] == 2
|
||||||
|
assert result["wings"].get("proj") == 1
|
||||||
|
assert result["wings"].get("unknown") == 1
|
||||||
|
|
||||||
def test_list_wings(self, monkeypatch, config, palace_path, seeded_collection, kg):
|
def test_list_wings(self, monkeypatch, config, palace_path, seeded_collection, kg):
|
||||||
_patch_mcp_server(monkeypatch, config, kg)
|
_patch_mcp_server(monkeypatch, config, kg)
|
||||||
from mempalace.mcp_server import tool_list_wings
|
from mempalace.mcp_server import tool_list_wings
|
||||||
|
|||||||
Reference in New Issue
Block a user