From 3f0cfd5ed45712ee09430ea7b4bceb9f104b85ec Mon Sep 17 00:00:00 2001 From: jp Date: Sat, 18 Apr 2026 12:28:03 -0700 Subject: [PATCH] fix(mcp): guard tool_status/list_wings/list_rooms/get_taxonomy against None metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- mempalace/mcp_server.py | 4 ++++ tests/test_mcp_server.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/mempalace/mcp_server.py b/mempalace/mcp_server.py index 3918a19..06355c4 100644 --- a/mempalace/mcp_server.py +++ b/mempalace/mcp_server.py @@ -315,6 +315,7 @@ def tool_status(): try: all_meta = _get_cached_metadata(col) for m in all_meta: + m = m or {} w = m.get("wing", "unknown") r = m.get("room", "unknown") wings[w] = wings.get(w, 0) + 1 @@ -368,6 +369,7 @@ def tool_list_wings(): try: all_meta = _get_cached_metadata(col) for m in all_meta: + m = m or {} w = m.get("wing", "unknown") wings[w] = wings.get(w, 0) + 1 except Exception as e: @@ -391,6 +393,7 @@ def tool_list_rooms(wing: str = None): where = {"wing": wing} if wing else None all_meta = _fetch_all_metadata(col, where=where) for m in all_meta: + m = m or {} r = m.get("room", "unknown") rooms[r] = rooms.get(r, 0) + 1 except Exception as e: @@ -409,6 +412,7 @@ def tool_get_taxonomy(): try: all_meta = _get_cached_metadata(col) for m in all_meta: + m = m or {} w = m.get("wing", "unknown") r = m.get("room", "unknown") if w not in taxonomy: diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 09073f5..899e6a7 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -250,6 +250,38 @@ class TestReadTools: assert "project" 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): _patch_mcp_server(monkeypatch, config, kg) from mempalace.mcp_server import tool_list_wings