diff --git a/mempalace/mcp_server.py b/mempalace/mcp_server.py index 8187cca..e5ecb12 100644 --- a/mempalace/mcp_server.py +++ b/mempalace/mcp_server.py @@ -1535,6 +1535,213 @@ def tool_delete_drawer(drawer_id: str): return {"success": False, "error": str(e)} +def _capture_fd_stdout(fn): + """Run ``fn()`` with its stdout captured at both the Python and fd level. + + The mining engines (``miner.mine`` / ``convo_miner.mine_convos`` / + ``format_miner.mine_formats``) print progress and a summary to stdout. In + the MCP server stdout is the JSON-RPC channel (``_restore_stdout`` runs once + in ``main`` before the protocol loop), so that output would corrupt the + protocol. Two layers are needed: + + * ``contextlib.redirect_stdout`` captures Python-level ``print`` into a + buffer — this is what becomes the returned summary, and it works even when + ``sys.stdout`` has been swapped (e.g. under pytest capture). + * an ``os.dup2`` of fd 1 to a temp file contains C-level banners emitted by + onnxruntime / chromadb during embedding, which bypass ``sys.stdout`` + entirely (the same reason the module redirects fd 1 at import, #225), and + keeps any direct fd-1 write off the live JSON-RPC channel. + + Returns ``(result, captured_text)``. ``captured_text`` is handed back to the + caller verbatim as an opaque summary; it is never parsed into fields. Falls + back to Python-level capture alone on platforms without fd-level stdio + (embedded interpreters), matching the import-time fallback. + """ + import contextlib + import io + import tempfile + + buf = io.StringIO() + sys.stdout.flush() + sys.stderr.flush() + try: + saved_fd = os.dup(1) + except (OSError, AttributeError): + with contextlib.redirect_stdout(buf): + result = fn() + return result, buf.getvalue() + + try: + with tempfile.TemporaryFile() as tmp: + os.dup2(tmp.fileno(), 1) + try: + with contextlib.redirect_stdout(buf): + result = fn() + finally: + sys.stdout.flush() + os.dup2(saved_fd, 1) + tmp.seek(0) + fd_text = tmp.read().decode("utf-8", "replace") + return result, buf.getvalue() + fd_text + finally: + os.close(saved_fd) + + +def tool_mine( + source: str, + mode: str = "projects", + wing: str = None, + agent: str = "mempalace", + limit: int = 0, + dry_run: bool = False, + extract: str = "exchange", +): + """Mine a directory into the palace — the MCP equivalent of ``mempalace mine``. + + Lets MCP clients that cannot shell out (Claude Desktop, LM Studio, Aionui, + Desktop Commander) trigger indexing in-conversation (#1662). Wraps the same + in-process miners the CLI's ``cmd_mine`` calls; it adds no new ingestion + logic of its own. + + mode: + ``"projects"`` (default) — code/docs via ``miner.mine``. + ``"convos"`` — chat transcripts via ``convo_miner.mine_convos``. + ``"extract"`` — office documents (PDF/DOCX/RTF/…) via + ``format_miner.mine_formats``; requires the + optional ``mempalace[extract]`` dependency. + wing: target wing (default: derived from the source directory name). + agent: recorded on every drawer (default ``"mempalace"``). + limit: max files to process (0 = all). + dry_run: walk + chunk and report, but file nothing. + extract: convos extraction strategy — ``"exchange"`` (default) or + ``"general"``; ignored by the other modes. + + Runs synchronously and mirrors the :func:`tool_sync` contract: success + returns ``{success: True, mode, dry_run, output[, output_truncated]}`` where ``output`` is + the miner's human-readable summary (captured so it cannot corrupt the + JSON-RPC stream); failure returns ``{success: False, error[, error_class]}``. + The palace write lock is held by the miners themselves, so a concurrent mine + surfaces as a structured already-running error. Orphan cleanup is not part of + mining — use ``mempalace_sync`` for that. + """ + global _metadata_cache + from .palace import MineAlreadyRunning, MineValidationError + + if not _config.palace_path: + np = _no_palace() + return {"success": False, "error": np.get("error", "no palace"), "hint": np.get("hint")} + + valid_modes = ("projects", "convos", "extract") + if mode not in valid_modes: + return { + "success": False, + "error": f"invalid mode '{mode}'; expected one of: {', '.join(valid_modes)}", + } + + src = os.path.expanduser(source) if source else "" + if not src or not os.path.isdir(src): + return {"success": False, "error": f"source directory not found: {source!r}"} + + def _run(): + if mode == "convos": + from .convo_miner import mine_convos + + return mine_convos( + convo_dir=src, + palace_path=_config.palace_path, + wing=wing, + agent=agent, + limit=limit, + dry_run=dry_run, + extract_mode=extract, + ) + if mode == "extract": + from .format_miner import mine_formats + + return mine_formats( + format_dir=src, + palace_path=_config.palace_path, + wing=wing, + agent=agent, + limit=limit, + dry_run=dry_run, + ) + from .miner import mine + + return mine( + project_dir=src, + palace_path=_config.palace_path, + wing_override=wing, + agent=agent, + limit=limit, + dry_run=dry_run, + ) + + try: + try: + _result, output = _capture_fd_stdout(_run) + # Order matters: typed handlers precede the bare Exception (mirroring + # tool_sync) so MineAlreadyRunning / MineValidationError / ValueError + # don't fall into the generic "mine failed" branch. + except MineAlreadyRunning as exc: + return { + "success": False, + "error": f"another mine is in progress: {exc}", + "error_class": "LockHeldByOtherProcess", + } + except MineValidationError as exc: + return { + "success": False, + "error": f"palace integrity check failed after mine: {exc}", + "error_class": "MineValidationError", + } + except ImportError as exc: + # 'extract' mode pulls in the optional mempalace[extract] stack; + # name it so the caller knows to install the extra. Other modes have + # no optional imports, so an ImportError there is a real bug, not a + # missing extra — log the traceback and surface its type. + if mode == "extract": + return { + "success": False, + "error": f"mode 'extract' needs the mempalace[extract] extra: {exc}", + "error_class": "MissingDependency", + } + logger.exception("tool_mine: unexpected ImportError (mode=%s)", mode) + return {"success": False, "error": f"mine failed: {exc}", "error_class": "ImportError"} + except ValueError as exc: + return {"success": False, "error": str(exc), "error_class": "ValueError"} + except SystemExit as exc: + # A library mine() must never terminate the MCP server. miner.mine + # converts Ctrl-C into sys.exit(130) (CLI semantics); in-process + # that SystemExit is a BaseException that would slip past the + # protocol loop's `except Exception` and kill the server with no + # response. Convert it to a structured error instead. + return { + "success": False, + "error": f"mine exited early (code {exc.code})", + "error_class": "Interrupted", + } + except Exception as exc: + logger.exception("tool_mine: mine failed (mode=%s)", mode) + return { + "success": False, + "error": f"mine failed: {exc}", + "error_class": type(exc).__name__, + } + # Cap the echoed summary so a very large mine cannot return a multi-MB + # payload to the MCP client. The useful summary is at the tail, so keep + # the end and flag the truncation (never silently). + payload = {"success": True, "mode": mode, "dry_run": dry_run, "output": output} + cap = 4000 + if len(output) > cap: + payload["output"] = output[-cap:] + payload["output_truncated"] = True + return payload + finally: + if not dry_run: + _metadata_cache = None + + def tool_sync(project_dir: str = None, wing: str = None, apply: bool = False): """Prune drawers whose source files are gitignored, missing, or moved (#1252).""" global _metadata_cache @@ -2562,6 +2769,60 @@ TOOLS = { }, "handler": tool_delete_drawer, }, + "mempalace_mine": { + "description": ( + "Mine a directory into the palace — the MCP equivalent of `mempalace mine`. " + "mode='projects' (default) ingests code/docs; mode='convos' ingests chat " + "transcripts; mode='extract' ingests office documents (PDF/DOCX/RTF, requires " + "the mempalace[extract] extra). Runs synchronously and returns the miner's " + "summary as `output`. The palace write lock is automatic; a concurrent mine " + "returns a structured already-running error. Orphan cleanup is separate — use " + "mempalace_sync." + ), + "input_schema": { + "type": "object", + "properties": { + "source": { + "type": "string", + "description": "Directory to mine.", + }, + "mode": { + "type": "string", + "enum": ["projects", "convos", "extract"], + "description": ( + "Ingest mode: projects (code/docs, default), convos (chat " + "transcripts), extract (office docs)." + ), + }, + "wing": { + "type": "string", + "description": "Target wing (default: source directory name).", + }, + "agent": { + "type": "string", + "description": "Recorded on every drawer (default: mempalace).", + }, + "limit": { + "type": "integer", + "description": "Max files to process (0 = all). Default: 0.", + }, + "dry_run": { + "type": "boolean", + "description": "Report what would be filed without writing. Default: false.", + }, + "extract": { + "type": "string", + "enum": ["exchange", "general"], + "description": ( + "Convos extraction strategy: exchange (default) or general. " + "Ignored by other modes." + ), + }, + }, + "required": ["source"], + }, + "handler": tool_mine, + }, "mempalace_sync": { "description": "Prune drawers whose source files are gitignored, deleted, or moved. Returns dry-run report by default; pass apply=true to commit deletions.", "input_schema": { diff --git a/tests/test_mcp_mine.py b/tests/test_mcp_mine.py new file mode 100644 index 0000000..0f73add --- /dev/null +++ b/tests/test_mcp_mine.py @@ -0,0 +1,262 @@ +""" +test_mcp_mine.py — Tests for the ``mempalace_mine`` MCP tool (#1662). + +Mining was previously CLI-only (``mempalace mine``); non-Claude-Code MCP clients +(Desktop Commander, LM Studio, Aionui) had no MCP-callable mine. ``tool_mine`` +wraps the same in-process miners the CLI uses — projects / convos / extract — +synchronously, mirroring the ``tool_sync`` contract. + +The miners print progress + a summary to stdout, which in the MCP server is the +JSON-RPC channel. ``tool_mine`` therefore redirects stdout at the file-descriptor +level around the miner and returns the text as an opaque ``output`` field rather +than letting it corrupt the protocol. These tests assert the dispatch/return +contract, that convos mining actually files drawers (the #1662 gap), and that the +stdout isolation holds. +""" + +import os + +import chromadb + + +def _patch(monkeypatch, config): + from mempalace import mcp_server + + monkeypatch.setattr(mcp_server, "_config", config) + + +def _write(path, text): + with open(path, "w", encoding="utf-8") as fh: + fh.write(text) + + +# ── Registration ───────────────────────────────────────────────────────── + + +def test_registered_in_tools(): + from mempalace import mcp_server + + assert "mempalace_mine" in mcp_server.TOOLS + entry = mcp_server.TOOLS["mempalace_mine"] + assert entry["handler"] is mcp_server.tool_mine + assert entry["input_schema"]["required"] == ["source"] + + +# ── Guard rails ────────────────────────────────────────────────────────── + + +def test_no_palace_returns_structured_error(monkeypatch): + from mempalace import mcp_server + + class _EmptyConfig: + palace_path = "" + collection_name = "mempalace_drawers" + + monkeypatch.setattr(mcp_server, "_config", _EmptyConfig()) + result = mcp_server.tool_mine(source="/tmp") + assert result["success"] is False + assert "error" in result + + +def test_invalid_mode_returns_structured_error(monkeypatch, config, tmp_dir): + from mempalace import mcp_server + + _patch(monkeypatch, config) + src = os.path.join(tmp_dir, "src") + os.makedirs(src) + result = mcp_server.tool_mine(source=src, mode="bogus") + assert result["success"] is False + assert "invalid mode" in result["error"].lower() + + +def test_missing_source_dir_returns_structured_error(monkeypatch, config): + from mempalace import mcp_server + + _patch(monkeypatch, config) + result = mcp_server.tool_mine(source="/nonexistent/path/xyz") + assert result["success"] is False + assert "source" in result["error"].lower() + + +# ── Dispatch + return contract ─────────────────────────────────────────── + + +def test_dry_run_projects_returns_success_and_output(monkeypatch, config, tmp_dir): + from mempalace import mcp_server + + _patch(monkeypatch, config) + src = os.path.join(tmp_dir, "proj") + os.makedirs(src) + _write(os.path.join(src, "notes.md"), "# Title\n\n" + ("Some real content. " * 40)) + + result = mcp_server.tool_mine(source=src, mode="projects", dry_run=True) + assert result["success"] is True + assert result["mode"] == "projects" + assert result["dry_run"] is True + assert isinstance(result["output"], str) and result["output"] + + +def test_convos_mode_files_drawers(monkeypatch, config, tmp_dir): + """The #1662 core ask: mine conversation transcripts via MCP. + + Proves the tool eliminates the gap rather than masking it — after a real + convos mine the palace collection actually holds the drawers. + """ + from mempalace import mcp_server + + _patch(monkeypatch, config) + src = os.path.join(tmp_dir, "convos") + os.makedirs(src) + _write( + os.path.join(src, "chat.txt"), + "> What is memory?\nMemory is persistence.\n\n" + "> Why does it matter?\nIt enables continuity across sessions.\n\n" + "> How do we build it?\nWith structured verbatim storage.\n", + ) + + result = mcp_server.tool_mine(source=src, mode="convos", wing="test_convos") + assert result["success"] is True + assert result["mode"] == "convos" + assert result["dry_run"] is False + + client = chromadb.PersistentClient(path=config.palace_path) + try: + col = client.get_collection("mempalace_drawers") + assert col.count() >= 2 + finally: + del client + + +def test_stdout_captured_not_leaked_to_fd(monkeypatch, config, tmp_dir, capfd): + """Miner stdout must land in ``output``, never on the real fd-1 JSON-RPC + channel. ``tool_mine`` redirects fd 1 around the in-process miner.""" + from mempalace import mcp_server + + _patch(monkeypatch, config) + src = os.path.join(tmp_dir, "convos") + os.makedirs(src) + _write( + os.path.join(src, "chat.txt"), + "> Q one?\nAnswer one is reasonably long so it forms a chunk here.\n\n" + "> Q two?\nAnswer two is also long enough to be filed as a drawer here.\n", + ) + + result = mcp_server.tool_mine(source=src, mode="convos", wing="cap", dry_run=True) + captured = capfd.readouterr() + assert "Done." in result["output"] + assert "Done." not in captured.out + + +def test_mine_already_running_surfaces_structured_error(monkeypatch, config, tmp_dir): + """A held palace lock (MineAlreadyRunning) surfaces as a structured + already-running error, mirroring tool_sync.""" + from mempalace import mcp_server + from mempalace.palace import MineAlreadyRunning + + _patch(monkeypatch, config) + src = os.path.join(tmp_dir, "proj") + os.makedirs(src) + _write(os.path.join(src, "a.md"), "content " * 50) + + def _boom(*args, **kwargs): + raise MineAlreadyRunning("held by pid 999") + + monkeypatch.setattr("mempalace.miner.mine", _boom) + result = mcp_server.tool_mine(source=src, mode="projects") + assert result["success"] is False + assert result.get("error_class") == "LockHeldByOtherProcess" + + +def test_large_output_is_tail_truncated(monkeypatch, config, tmp_dir): + """A very large miner summary is tail-trimmed (and flagged, never silently) + so the MCP response stays bounded.""" + from mempalace import mcp_server + + _patch(monkeypatch, config) + src = os.path.join(tmp_dir, "proj") + os.makedirs(src) + + def _chatty(*args, **kwargs): + print("X" * 5000) + return None + + monkeypatch.setattr("mempalace.miner.mine", _chatty) + result = mcp_server.tool_mine(source=src, mode="projects") + assert result["success"] is True + assert result["output_truncated"] is True + assert len(result["output"]) == 4000 + + +def test_import_error_outside_extract_is_not_mislabeled(monkeypatch, config, tmp_dir): + """An ImportError outside extract mode is a real bug, not a missing extra — + it must not be labelled MissingDependency.""" + from mempalace import mcp_server + + _patch(monkeypatch, config) + src = os.path.join(tmp_dir, "proj") + os.makedirs(src) + + def _broken(*args, **kwargs): + raise ImportError("no module named 'totally_internal'") + + monkeypatch.setattr("mempalace.miner.mine", _broken) + result = mcp_server.tool_mine(source=src, mode="projects") + assert result["success"] is False + assert result.get("error_class") == "ImportError" + assert "mine failed" in result["error"] + + +def test_extract_missing_dependency_is_named(monkeypatch, config, tmp_dir): + """extract mode surfaces a MissingDependency error pointing at the extra.""" + from mempalace import mcp_server + + _patch(monkeypatch, config) + src = os.path.join(tmp_dir, "docs") + os.makedirs(src) + + def _no_extra(*args, **kwargs): + raise ImportError("No module named 'markitdown'") + + monkeypatch.setattr("mempalace.format_miner.mine_formats", _no_extra) + result = mcp_server.tool_mine(source=src, mode="extract") + assert result["success"] is False + assert result.get("error_class") == "MissingDependency" + assert "mempalace[extract]" in result["error"] + + +def test_system_exit_from_miner_does_not_kill_server(monkeypatch, config, tmp_dir): + """miner.mine turns Ctrl-C into sys.exit(130); in-process that SystemExit + would escape the protocol loop (which only catches Exception) and kill the + server. tool_mine converts it to a structured error instead.""" + from mempalace import mcp_server + + _patch(monkeypatch, config) + src = os.path.join(tmp_dir, "proj") + os.makedirs(src) + + def _exit(*args, **kwargs): + raise SystemExit(130) + + monkeypatch.setattr("mempalace.miner.mine", _exit) + result = mcp_server.tool_mine(source=src, mode="projects") + assert result["success"] is False + assert result.get("error_class") == "Interrupted" + + +def test_generic_exception_carries_error_class(monkeypatch, config, tmp_dir): + """An unexpected miner failure is surfaced with its exception type so the + caller can distinguish error kinds.""" + from mempalace import mcp_server + + _patch(monkeypatch, config) + src = os.path.join(tmp_dir, "proj") + os.makedirs(src) + + def _boom(*args, **kwargs): + raise RuntimeError("disk gone") + + monkeypatch.setattr("mempalace.miner.mine", _boom) + result = mcp_server.tool_mine(source=src, mode="projects") + assert result["success"] is False + assert "mine failed" in result["error"] + assert result.get("error_class") == "RuntimeError" diff --git a/website/reference/mcp-tools.md b/website/reference/mcp-tools.md index 220b15e..a40fa27 100644 --- a/website/reference/mcp-tools.md +++ b/website/reference/mcp-tools.md @@ -1,6 +1,6 @@ # MCP Tools Reference -Detailed parameter schemas for all 30 MCP tools. +Detailed parameter schemas for all 31 MCP tools. ## Palace — Read Tools @@ -114,6 +114,24 @@ Delete a drawer by ID. Irreversible. --- +### `mempalace_mine` + +Mine a directory into the palace — the MCP equivalent of `mempalace mine`. Wraps the same in-process miners the CLI uses; runs synchronously and returns the miner's summary as `output`. The palace write lock is automatic — a concurrent mine returns a structured already-running error. Orphan cleanup is separate (see `mempalace_sync`). + +| Parameter | Type | Required | Description | +|-----------|------|----------|-------------| +| `source` | string | **Yes** | Directory to mine | +| `mode` | string | No | `projects` (code/docs, default), `convos` (chat transcripts), or `extract` (office docs; needs the `mempalace[extract]` extra) | +| `wing` | string | No | Target wing (default: source directory name) | +| `agent` | string | No | Recorded on every drawer (default: `mempalace`) | +| `limit` | integer | No | Max files to process (0 = all; default 0) | +| `dry_run` | boolean | No | Report what would be filed without writing (default false) | +| `extract` | string | No | Convos extraction strategy: `exchange` (default) or `general`; ignored by other modes | + +**Returns:** `{ success, mode, dry_run, output }` on success (`output` is the miner's human-readable summary; `output_truncated: true` is added when a very large summary is tail-trimmed), or `{ success: false, error, error_class? }` on failure. + +--- + ### `mempalace_sync` Prune drawers whose source files are gitignored, deleted, or moved. Returns a dry-run report by default; pass `apply=true` to commit deletions.