Merge pull request #139 from igorls/fix/error-handling

fix: sanitize error responses and remove sys.exit from library code

mempalace/cli.py

@@ -97,16 +97,19 @@ def cmd_mine(args):
 
 
 def cmd_search(args):
-    from .searcher import search
+    from .searcher import search, SearchError
 
     palace_path = os.path.expanduser(args.palace) if args.palace else MempalaceConfig().palace_path
-    search(
+    try:
-        query=args.query,
+        search(
-        palace_path=palace_path,
+            query=args.query,
-        wing=args.wing,
+            palace_path=palace_path,
-        room=args.room,
+            wing=args.wing,
-        n_results=args.results,
+            room=args.room,
-    )
+            n_results=args.results,
+        )
+    except SearchError:
+        sys.exit(1)
 
 
 def cmd_wakeup(args):

mempalace/mcp_server.py

@@ -53,7 +53,6 @@ def _get_collection(create=False):
 def _no_palace():
     return {
         "error": "No palace found",
-        "palace_path": _config.palace_path,
         "hint": "Run: mempalace init <dir> && mempalace mine <dir>",
     }
 
@@ -744,9 +743,13 @@ def handle_request(request):
                 "id": req_id,
                 "result": {"content": [{"type": "text", "text": json.dumps(result, indent=2)}]},
             }
-        except Exception as e:
+        except Exception:
-            logger.error(f"Tool error in {tool_name}: {e}")
+            logger.exception(f"Tool error in {tool_name}")
-            return {"jsonrpc": "2.0", "id": req_id, "error": {"code": -32000, "message": str(e)}}
+            return {
+                "jsonrpc": "2.0",
+                "id": req_id,
+                "error": {"code": -32000, "message": "Internal tool error"},
+            }
 
     return {
         "jsonrpc": "2.0",

mempalace/searcher.py

@@ -6,11 +6,17 @@ Semantic search against the palace.
 Returns verbatim text — the actual words, never summaries.
 """
 
-import sys
+import logging
 from pathlib import Path
 
 import chromadb
 
+logger = logging.getLogger("mempalace_mcp")
+
+
+class SearchError(Exception):
+    """Raised when search cannot proceed (e.g. no palace found)."""
+
 
 def search(query: str, palace_path: str, wing: str = None, room: str = None, n_results: int = 5):
     """
@@ -23,7 +29,7 @@ def search(query: str, palace_path: str, wing: str = None, room: str = None, n_r
     except Exception:
         print(f"\n  No palace found at {palace_path}")
         print("  Run: mempalace init <dir> then mempalace mine <dir>")
-        sys.exit(1)
+        raise SearchError(f"No palace found at {palace_path}")
 
     # Build where filter
     where = {}
@@ -47,7 +53,7 @@ def search(query: str, palace_path: str, wing: str = None, room: str = None, n_r
 
     except Exception as e:
         print(f"\n  Search error: {e}")
-        sys.exit(1)
+        raise SearchError(f"Search error: {e}") from e
 
     docs = results["documents"][0]
     metas = results["metadatas"][0]
@@ -95,7 +101,11 @@ def search_memories(
         client = chromadb.PersistentClient(path=palace_path)
         col = client.get_collection("mempalace_drawers")
     except Exception as e:
-        return {"error": f"No palace found at {palace_path}: {e}"}
+        logger.error("No palace found at %s: %s", palace_path, e)
+        return {
+            "error": "No palace found",
+            "hint": "Run: mempalace init <dir> && mempalace mine <dir>",
+        }
 
     # Build where filter
     where = {}

tests/conftest.py

@@ -35,7 +35,7 @@ from mempalace.knowledge_graph import KnowledgeGraph  # noqa: E402
 
 
 @pytest.fixture(scope="session", autouse=True)
-def _isolate_home(tmp_path_factory):
+def _isolate_home():
     """Ensure HOME points to a temp dir for the entire test session.
 
     The env vars were already set at module level (above) so that

tests/test_mcp_server.py

@@ -13,6 +13,9 @@ def _patch_mcp_server(monkeypatch, config, palace_path, kg):
     """Patch the mcp_server module globals to use test fixtures."""
     from mempalace import mcp_server
 
+    assert getattr(config, "palace_path", None) == palace_path, (
+        f"config.palace_path ({getattr(config, 'palace_path', None)!r}) does not match palace_path fixture ({palace_path!r})"
+    )
     monkeypatch.setattr(mcp_server, "_config", config)
     monkeypatch.setattr(mcp_server, "_kg", kg)
 
@@ -149,6 +152,7 @@ class TestReadTools:
         assert result["taxonomy"]["notes"]["planning"] == 1
 
     def test_no_palace_returns_error(self, monkeypatch, config, kg):
+        config._file_config["palace_path"] = "/nonexistent/path"
         _patch_mcp_server(monkeypatch, config, "/nonexistent/path", kg)
         from mempalace.mcp_server import tool_status
 

uv.lock

@@ -966,6 +966,13 @@ dependencies = [
     { name = "pyyaml" },
 ]
 
+[package.optional-dependencies]
+dev = [
+    { name = "pytest", version = "8.4.2", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version < '3.10'" },
+    { name = "pytest", version = "9.0.3", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version >= '3.10'" },
+    { name = "ruff" },
+]
+
 [package.dev-dependencies]
 dev = [
     { name = "pytest", version = "8.4.2", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version < '3.10'" },
@@ -976,8 +983,11 @@ dev = [
 [package.metadata]
 requires-dist = [
     { name = "chromadb", specifier = ">=0.4.0,<1" },
+    { name = "pytest", marker = "extra == 'dev'", specifier = ">=7.0" },
     { name = "pyyaml", specifier = ">=6.0" },
+    { name = "ruff", marker = "extra == 'dev'", specifier = ">=0.4.0" },
 ]
+provides-extras = ["dev"]
 
 [package.metadata.requires-dev]
 dev = [