From c9135aad67cf62a8e3bab2776f6570efe5dc9a76 Mon Sep 17 00:00:00 2001 From: Igor Lins e Silva <4753812+igorls@users.noreply.github.com> Date: Tue, 7 Apr 2026 17:25:47 -0300 Subject: [PATCH 1/3] fix: sanitize error responses and remove sys.exit from library code - Remove palace_path from _no_palace() error response (prevents leaking filesystem paths to the LLM) - Replace str(e) with generic 'Internal tool error' in MCP dispatch catch block (full error is still logged server-side via stderr) - Replace sys.exit(1) with return in searcher.search() CLI function (prevents process termination if called from library context) - Remove unused sys import from searcher.py Findings: #12 (HIGH), #5 (MEDIUM), #15 (LOW) Includes test infrastructure from PR #131. 92 tests pass. --- mempalace/mcp_server.py | 3 +-- mempalace/searcher.py | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/mempalace/mcp_server.py b/mempalace/mcp_server.py index 3861195..6f4677f 100644 --- a/mempalace/mcp_server.py +++ b/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 && mempalace mine ", } @@ -746,7 +745,7 @@ def handle_request(request): } except Exception as e: logger.error(f"Tool error in {tool_name}: {e}") - 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", diff --git a/mempalace/searcher.py b/mempalace/searcher.py index 9972dbe..5c325ae 100644 --- a/mempalace/searcher.py +++ b/mempalace/searcher.py @@ -6,7 +6,7 @@ Semantic search against the palace. Returns verbatim text — the actual words, never summaries. """ -import sys + from pathlib import Path import chromadb @@ -23,7 +23,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 then mempalace mine ") - sys.exit(1) + return # Build where filter where = {} @@ -47,7 +47,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) + return docs = results["documents"][0] metas = results["metadatas"][0] From 5ac4947d023f309e02faa3d86a1a0225e03cdabb Mon Sep 17 00:00:00 2001 From: Igor Lins e Silva <4753812+igorls@users.noreply.github.com> Date: Tue, 7 Apr 2026 17:38:53 -0300 Subject: [PATCH 2/3] fix: preserve CLI exit codes, log tracebacks, sanitize search errors, validate fixture --- mempalace/cli.py | 19 +++++++++++-------- mempalace/mcp_server.py | 10 +++++++--- mempalace/searcher.py | 18 ++++++++++++++---- tests/test_mcp_server.py | 4 ++++ uv.lock | 10 ++++++++++ 5 files changed, 46 insertions(+), 15 deletions(-) diff --git a/mempalace/cli.py b/mempalace/cli.py index 8b5edf4..1599b08 100644 --- a/mempalace/cli.py +++ b/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( - query=args.query, - palace_path=palace_path, - wing=args.wing, - room=args.room, - n_results=args.results, - ) + try: + search( + query=args.query, + palace_path=palace_path, + wing=args.wing, + room=args.room, + n_results=args.results, + ) + except SearchError: + sys.exit(1) def cmd_wakeup(args): diff --git a/mempalace/mcp_server.py b/mempalace/mcp_server.py index 6f4677f..94cbf99 100644 --- a/mempalace/mcp_server.py +++ b/mempalace/mcp_server.py @@ -743,9 +743,13 @@ def handle_request(request): "id": req_id, "result": {"content": [{"type": "text", "text": json.dumps(result, indent=2)}]}, } - except Exception as e: - logger.error(f"Tool error in {tool_name}: {e}") - return {"jsonrpc": "2.0", "id": req_id, "error": {"code": -32000, "message": "Internal tool error"}} + except Exception: + logger.exception(f"Tool error in {tool_name}") + return { + "jsonrpc": "2.0", + "id": req_id, + "error": {"code": -32000, "message": "Internal tool error"}, + } return { "jsonrpc": "2.0", diff --git a/mempalace/searcher.py b/mempalace/searcher.py index 5c325ae..163abd8 100644 --- a/mempalace/searcher.py +++ b/mempalace/searcher.py @@ -6,11 +6,17 @@ Semantic search against the palace. Returns verbatim text — the actual words, never summaries. """ - +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 then mempalace mine ") - return + 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}") - return + 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 && mempalace mine ", + } # Build where filter where = {} diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index 2ca177e..cf37a27 100644 --- a/tests/test_mcp_server.py +++ b/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 diff --git a/uv.lock b/uv.lock index 0b00717..9d99313 100644 --- a/uv.lock +++ b/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 = [ From d3145e9a7b14e20da203edfcf0ff45ab0851e4b5 Mon Sep 17 00:00:00 2001 From: Igor Lins e Silva <4753812+igorls@users.noreply.github.com> Date: Tue, 7 Apr 2026 18:58:25 -0300 Subject: [PATCH 3/3] fix: update dialect tests for PR #147 stats API and remove unused fixture param --- tests/conftest.py | 2 +- tests/test_dialect.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index d0c30c2..22b5e42 100644 --- a/tests/conftest.py +++ b/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 diff --git a/tests/test_dialect.py b/tests/test_dialect.py index 2ef1df6..8edc7ec 100644 --- a/tests/test_dialect.py +++ b/tests/test_dialect.py @@ -109,11 +109,11 @@ class TestCompressionStats: original = "We decided to use GraphQL instead of REST. " * 10 compressed = d.compress(original) stats = d.compression_stats(original, compressed) - assert stats["ratio"] > 1 - assert stats["original_chars"] > stats["compressed_chars"] + assert stats["size_ratio"] > 1 + assert stats["original_chars"] > stats["summary_chars"] def test_count_tokens(self): - assert Dialect.count_tokens("hello world") == len("hello world") // 3 + assert Dialect.count_tokens("hello world") == 2 class TestZettelEncoding: