From c3930ecf6eae4b7ef451d40ff8aa995cba90ef58 Mon Sep 17 00:00:00 2001 From: jp Date: Mon, 25 May 2026 20:49:17 -0700 Subject: [PATCH 1/2] =?UTF-8?q?fix(tests):=20repair=20main=20test=20suite?= =?UTF-8?q?=20=E2=80=94=20daemon=20fast-path=20cascade=20+=20entry-point?= =?UTF-8?q?=20pollution?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The August REST fast-path landings (`_daemon_search_fast`, `_daemon_status_fast`) introduced shape and routing assumptions that the existing test fixtures didn't model. Fixtures only mocked POST/MCP envelopes; new fast paths issue GET to `/search/fast` and `/status/fast`, so the same mocks crashed on `req.data.decode()` (data is None on GET) or returned an envelope shape that `_daemon_search_fast` then iterated as a bare list. Fixes: - `_daemon_search_fast` now accepts both `{"results": [...]}` and bare list shapes, defensively renames per-hit fields. - Test fixtures (test_cli_search_output, test_cli_stats, test_cli_daemon, test_cli_json) dispatch on `req.data is None` to serve REST GETs with a bare payload or HTTP 404 fall-through. - `sources.registry.reset_discovery()` lets fixtures re-discover entry-point adapters after a previous test's `unregister()` flipped the `_discovered` cache. Without it, `available_adapters()` permanently returned `[]` for later tests. - README tool-count tests skip markdown table rows so competitor counts in comparison tables stop triggering spurious failures. - README version badge bumped 3.3.5 → 3.3.6 to match version.py. 3193 passed, 35 skipped locally (full suite). The 4 remaining local failures are environmental (developer's `~/.mempalace/config.json` leaking past `patch.dict({}, clear=True)` and one chroma lock flake) and do not reproduce in CI. Co-Authored-By: Claude Opus 4.7 --- README.md | 2 +- mempalace/cli.py | 20 ++++++++++++++++---- mempalace/sources/__init__.py | 2 ++ mempalace/sources/registry.py | 12 ++++++++++++ tests/test_cli_search_output.py | 22 +++++++++++++++++++++- tests/test_cli_source.py | 3 +++ tests/test_cli_stats.py | 20 ++++++++++++++++++++ tests/test_readme_claims.py | 33 +++++++++++++++++++++++++++------ tests/test_sources.py | 3 +++ 9 files changed, 105 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index ded425cd83..0dfb33a7a3 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ > Need the shortest recovery/setup path? Use the > [Claude Code retention setup checklist](https://mempalaceofficial.com/guide/claude-code-retention.html). -[![version-shield](https://img.shields.io/badge/version-3.3.5-4dc9f6?style=flat-square&labelColor=0a0e14)](https://github.com/techempower-org/mempalace/releases) [![upstream-shield](https://img.shields.io/badge/upstream-3.3.5-7dd8f8?style=flat-square&labelColor=0a0e14)](https://github.com/MemPalace/mempalace/releases) +[![version-shield](https://img.shields.io/badge/version-3.3.6-4dc9f6?style=flat-square&labelColor=0a0e14)](https://github.com/techempower-org/mempalace/releases) [![upstream-shield](https://img.shields.io/badge/upstream-3.3.5-7dd8f8?style=flat-square&labelColor=0a0e14)](https://github.com/MemPalace/mempalace/releases) [![python-shield](https://img.shields.io/badge/python-3.9+-7dd8f8?style=flat-square&labelColor=0a0e14&logo=python&logoColor=7dd8f8)](https://www.python.org/) [![license-shield](https://img.shields.io/badge/license-MIT-b0e8ff?style=flat-square&labelColor=0a0e14)](LICENSE) diff --git a/mempalace/cli.py b/mempalace/cli.py index 0be9977aec..8a7ca744d8 100644 --- a/mempalace/cli.py +++ b/mempalace/cli.py @@ -1457,12 +1457,24 @@ def _daemon_search_fast(query: str, n_results: int, wing: str = None) -> dict | raw = _call_daemon_rest("/search/fast", rest_params) if raw is None: return None - for hit in raw: - hit["text"] = hit.pop("snippet", "") - hit["bm25_score"] = round(hit.pop("rank", 0), 3) + if isinstance(raw, dict): + hits = raw.get("results") + elif isinstance(raw, list): + hits = raw + else: + hits = None + if not isinstance(hits, list): + return None + for hit in hits: + if "snippet" in hit: + hit["text"] = hit.pop("snippet") + elif "text" not in hit: + hit["text"] = "" + if "rank" in hit: + hit["bm25_score"] = round(hit.pop("rank"), 3) if hit.get("source_file"): hit["source"] = hit["source_file"] - return {"results": raw, "query": query, "source": "bm25-fast"} + return {"results": hits, "query": query, "source": "bm25-fast"} def _daemon_search_hybrid( diff --git a/mempalace/sources/__init__.py b/mempalace/sources/__init__.py index 6fe02723fe..281e4ca887 100644 --- a/mempalace/sources/__init__.py +++ b/mempalace/sources/__init__.py @@ -41,6 +41,7 @@ get_adapter_class, register, reset_adapters, + reset_discovery, resolve_adapter_for_source, unregister, ) @@ -69,6 +70,7 @@ "get_adapter_class", "register", "reset_adapters", + "reset_discovery", "resolve_adapter_for_source", "unregister", ] diff --git a/mempalace/sources/registry.py b/mempalace/sources/registry.py index cb50737d37..e81dc6b0af 100644 --- a/mempalace/sources/registry.py +++ b/mempalace/sources/registry.py @@ -140,6 +140,18 @@ def reset_adapters() -> None: _instances.clear() +def reset_discovery() -> None: + """Force the next ``available_adapters()`` call to re-scan entry points. + + Tests that ``unregister`` entry-point-discovered adapters must call + this — otherwise the cached ``_discovered=True`` flag suppresses + rediscovery and ``available_adapters()`` permanently returns ``[]``. + """ + global _discovered + with _lock: + _discovered = False + + def resolve_adapter_for_source( *, explicit: str | None = None, diff --git a/tests/test_cli_search_output.py b/tests/test_cli_search_output.py index e13c6f2d2f..93e451bbee 100644 --- a/tests/test_cli_search_output.py +++ b/tests/test_cli_search_output.py @@ -43,9 +43,16 @@ def _envelope(payload: dict) -> bytes: def _make_search_dispatcher(payload: dict): - """Return a fake ``urlopen`` that returns ``payload`` for every call.""" + """Return a fake ``urlopen`` that returns ``payload`` for every call. + + ``GET`` requests (REST fast-path) get the bare payload; ``POST`` + requests (MCP tools/call) get the JSON-RPC envelope. Lets the same + fixture serve ``/search/fast`` and ``mempalace_search`` fallback. + """ def fake_urlopen(req, timeout=None): + if getattr(req, "data", None) is None: + return _FakeResp(json.dumps(payload).encode()) return _FakeResp(_envelope(payload)) return fake_urlopen @@ -429,6 +436,13 @@ def test_limit_overrides_results(self): captured = {} def fake_urlopen(req, timeout=None): + if getattr(req, "data", None) is None: + # REST fast-path GET — capture ``limit`` from query string. + from urllib.parse import urlparse, parse_qs + + qs = parse_qs(urlparse(req.full_url).query) + captured["arguments"] = {"limit": int(qs["limit"][0])} + return _FakeResp(json.dumps({"results": [], "warnings": []}).encode()) captured["arguments"] = json.loads(req.data.decode())["params"]["arguments"] return _FakeResp(_envelope({"results": [], "warnings": []})) @@ -445,6 +459,12 @@ def test_results_used_when_limit_unset(self): captured = {} def fake_urlopen(req, timeout=None): + if getattr(req, "data", None) is None: + from urllib.parse import urlparse, parse_qs + + qs = parse_qs(urlparse(req.full_url).query) + captured["arguments"] = {"limit": int(qs["limit"][0])} + return _FakeResp(json.dumps({"results": [], "warnings": []}).encode()) captured["arguments"] = json.loads(req.data.decode())["params"]["arguments"] return _FakeResp(_envelope({"results": [], "warnings": []})) diff --git a/tests/test_cli_source.py b/tests/test_cli_source.py index d59d8dac5a..ce67b61841 100644 --- a/tests/test_cli_source.py +++ b/tests/test_cli_source.py @@ -19,6 +19,7 @@ SourceItemMetadata, register, reset_adapters, + reset_discovery, unregister, ) @@ -61,6 +62,8 @@ def _isolate_registry(): unregister(name) except Exception: pass + # Allow the next test to rediscover the in-tree entry-point adapters. + reset_discovery() @pytest.fixture() diff --git a/tests/test_cli_stats.py b/tests/test_cli_stats.py index dc51c78264..4088351da6 100644 --- a/tests/test_cli_stats.py +++ b/tests/test_cli_stats.py @@ -45,9 +45,24 @@ def _make_dispatcher(responses: dict): Any tool not in ``responses`` returns an empty object. The dispatcher inspects the JSON-RPC request body to pick the right response so the same fixture can serve multiple tools fired by a single command. + + ``GET`` requests (REST fast-path, e.g. ``/status/fast``) get the bare + payload mapped from the path's matching MCP tool — ``/status/fast`` + mirrors ``mempalace_status``. """ + _rest_to_tool = { + "/status/fast": "mempalace_status", + "/search/fast": "mempalace_search", + } + def fake_urlopen(req, timeout=None): + if getattr(req, "data", None) is None: + url = req.full_url + for path, tool in _rest_to_tool.items(): + if path in url: + return _FakeResp(json.dumps(responses.get(tool, {})).encode()) + return _FakeResp(b"{}") body = json.loads(req.data.decode()) name = body["params"]["name"] return _FakeResp(_envelope(responses.get(name, {}))) @@ -302,6 +317,11 @@ def test_kg_failure_does_not_blank_the_dashboard(self, capsys): from mempalace import cli def fake_urlopen(req, timeout=None): + if getattr(req, "data", None) is None: + # REST fast-path GET — ``/status/fast`` mirrors mempalace_status. + return _FakeResp( + json.dumps({"total_drawers": 5, "wings": {"projects": 5}}).encode() + ) body = json.loads(req.data.decode()) name = body["params"]["name"] if name == "mempalace_kg_stats": diff --git a/tests/test_readme_claims.py b/tests/test_readme_claims.py index f096b3d6f9..2affc87a72 100644 --- a/tests/test_readme_claims.py +++ b/tests/test_readme_claims.py @@ -58,19 +58,36 @@ def _doc_tool_names() -> list: # --------------------------------------------------------------------------- +def _readme_self_tool_counts(readme: str) -> list[str]: + """Extract "N tools" claims that refer to mempalace itself. + + Skips markdown table rows (lines containing ``|``) so competitor + rows like "Longhand … 17 tools" don't poison the count — those are + facts about other projects, not claims about mempalace. + """ + counts = [] + for line in readme.splitlines(): + if "|" in line: + continue + counts.extend(re.findall(r"(\d+)\s+tools", line)) + return counts + + class TestToolCount: - """README claims '19 tools available through MCP' in multiple places.""" + """README claims 'N tools available through MCP' in multiple places.""" def test_readme_tool_count_matches_code(self): - """Claim: README says 19 tools. Actual TOOLS dict may differ. + """Claim: README says N tools. Actual TOOLS dict may differ. This test asserts the REAL tool count so the README can be updated. If TOOLS has 25 entries, the README should say 25, not 19. + + Only counts mempalace's own self-claims — competitor tool counts + in comparison tables are out of scope. """ actual_count = len(_tools_dict_keys()) readme = _readme() - # Find all "19 tools" claims in README - claimed_counts = re.findall(r"(\d+)\s+tools", readme) + claimed_counts = _readme_self_tool_counts(readme) for claimed in claimed_counts: assert int(claimed) == actual_count, ( f"README claims {claimed} tools but TOOLS dict has {actual_count}. " @@ -732,9 +749,13 @@ class TestReadmeToolCountConsistency: """README mentions tool count in multiple places — they must all agree.""" def test_all_tool_count_mentions_consistent(self): - """Every place README says 'N tools' must use the same number.""" + """Every place README says 'N tools' about mempalace must agree. + + Scoped to self-claims (non-table-row lines) so competitor counts + in the comparison tables don't trigger spurious failures. + """ readme = _readme() - counts = re.findall(r"(\d+)\s+tools", readme) + counts = _readme_self_tool_counts(readme) if len(counts) > 1: unique = set(counts) assert len(unique) == 1, ( diff --git a/tests/test_sources.py b/tests/test_sources.py index cb010337f7..57f245dfd9 100644 --- a/tests/test_sources.py +++ b/tests/test_sources.py @@ -20,6 +20,7 @@ get_adapter_class, register, reset_adapters, + reset_discovery, resolve_adapter_for_source, unregister, ) @@ -66,6 +67,8 @@ def _isolate_registry(): reset_adapters() for name in list(available_adapters()): unregister(name) + # Allow the next test to rediscover the in-tree entry-point adapters. + reset_discovery() # --------------------------------------------------------------------------- From 1a9fe090d2d6033d4e22d00460f7f6f59c35db34 Mon Sep 17 00:00:00 2001 From: jp Date: Mon, 25 May 2026 20:50:27 -0700 Subject: [PATCH 2/2] =?UTF-8?q?fix(tests):=20complete=20cascade=20?= =?UTF-8?q?=E2=80=94=20daemon-routing=20CLI=20tests=20fall=20through=20to?= =?UTF-8?q?=20MCP?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-up to the previous commit. Same shape as the search_output/stats fixture pattern, but for tests that exercise the routing-to-daemon path in cmd_search and cmd_status. With the REST fast-path landing first, these fixtures crashed on `req.data.decode()` (data is None on GET). - test_cli_daemon.py: `_rest_fastpath_404(req)` helper raises HTTPError on GET so cmd_search falls through to the MCP POST envelope the test is actually verifying. - test_cli_json.py: patches `_call_daemon_rest` alongside `_call_daemon_tool` so the cmd_status/cmd_search tests see no fast path at all and exercise the MCP envelope/error paths intended. Co-Authored-By: Claude Opus 4.7 --- tests/test_cli_daemon.py | 11 +++++++++++ tests/test_cli_json.py | 28 +++++++++++++++++----------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/tests/test_cli_daemon.py b/tests/test_cli_daemon.py index 7ce81faf20..94dbd2eb1a 100644 --- a/tests/test_cli_daemon.py +++ b/tests/test_cli_daemon.py @@ -14,6 +14,7 @@ import argparse import json +import urllib.error from unittest.mock import MagicMock, patch import pytest @@ -33,6 +34,12 @@ def read(self): return self._body +def _rest_fastpath_404(req): + """Raise HTTPError(404) for REST fast-path GETs so cmd_search/cmd_status + fall through to the MCP POST envelope these tests actually verify.""" + raise urllib.error.HTTPError(req.full_url, 404, "Not Found", {}, None) + + # ── _daemon_strict ────────────────────────────────────────────────────── @@ -229,6 +236,8 @@ def test_routes_to_daemon_when_strict(self, capsys): ).encode() def fake_urlopen(req, timeout=None): + if getattr(req, "data", None) is None: + return _rest_fastpath_404(req) captured_body = json.loads(req.data.decode()) assert captured_body["params"]["name"] == "mempalace_search" assert captured_body["params"]["arguments"]["query"] == "graphql" @@ -264,6 +273,8 @@ def test_sends_limit_not_max_results(self): captured = {} def fake_urlopen(req, timeout=None): + if getattr(req, "data", None) is None: + return _rest_fastpath_404(req) captured["body"] = json.loads(req.data.decode()) return _FakeResp(body) diff --git a/tests/test_cli_json.py b/tests/test_cli_json.py index 4379dd7c1b..7c79df6369 100644 --- a/tests/test_cli_json.py +++ b/tests/test_cli_json.py @@ -182,11 +182,12 @@ def test_daemon_routing_emits_daemon_payload_as_json(self, mock_cfg, capsys): args = argparse.Namespace(palace=None, json=True, quiet=False) daemon_payload = {"total_drawers": 7, "wings": {"wing_x": 7}} - with patch("mempalace.cli._call_daemon_tool", return_value=daemon_payload): - with patch("mempalace.cli._daemon_strict", return_value=True): - from mempalace.cli import cmd_status + with patch("mempalace.cli._call_daemon_rest", return_value=None): + with patch("mempalace.cli._call_daemon_tool", return_value=daemon_payload): + with patch("mempalace.cli._daemon_strict", return_value=True): + from mempalace.cli import cmd_status - cmd_status(args) + cmd_status(args) out = capsys.readouterr().out payload = json.loads(out) @@ -204,11 +205,15 @@ def test_daemon_error_emits_json_error_and_exit_2(self, mock_cfg, capsys): with patch("mempalace.cli._daemon_strict", return_value=True): with patch( - "mempalace.cli._call_daemon_tool", + "mempalace.cli._call_daemon_rest", side_effect=DaemonError("connection refused"), ): - with pytest.raises(SystemExit) as exc_info: - cmd_status(args) + with patch( + "mempalace.cli._call_daemon_tool", + side_effect=DaemonError("connection refused"), + ): + with pytest.raises(SystemExit) as exc_info: + cmd_status(args) assert exc_info.value.code == 2 out = capsys.readouterr().out @@ -365,11 +370,12 @@ def test_daemon_search_emits_results_with_query_key(self, mock_cfg, capsys): } with patch("mempalace.cli._daemon_strict", return_value=True): - with patch("mempalace.cli._call_daemon_tool", return_value=daemon_payload): - with pytest.raises(SystemExit) as exc_info: - from mempalace.cli import cmd_search + with patch("mempalace.cli._call_daemon_rest", return_value=None): + with patch("mempalace.cli._call_daemon_tool", return_value=daemon_payload): + with pytest.raises(SystemExit) as exc_info: + from mempalace.cli import cmd_search - cmd_search(args) + cmd_search(args) assert exc_info.value.code == 0 out = capsys.readouterr().out