Add an MCP client for reactome-mcp, harvested from #127 and #137

bin/mcp-probe

@@ -0,0 +1,12 @@
+#!/usr/bin/env python3
+"""Entry point for the MCP probe; see src/reactome_mcp/probe.py."""
+
+import sys
+from pathlib import Path
+
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent / "src"))
+
+from reactome_mcp.probe import main
+
+if __name__ == "__main__":
+    main()

src/reactome_mcp/README.md

@@ -0,0 +1,84 @@
+# Reactome MCP client
+
+Talks to [`reactome-mcp`](https://github.com/reactome/reactome-mcp), which
+exposes Reactome's Content and Analysis services as MCP tools.
+
+Harvested from #127 and #137 by @GovindhKishore.
+
+## Why the package is called `reactome_mcp`
+
+The official MCP Python SDK is published on PyPI as `mcp`. A local package of
+that name shadows it, and the failure appears only the day someone adds the
+dependency — as an import that resolves to the wrong thing rather than an error.
+
+## What is here
+
+| | |
+|---|---|
+| `process.py` | starts and stops the server subprocess |
+| `client.py` | JSON-RPC over its stdio |
+| `tools.py` | five of the server's 53 tools, as LangChain tools |
+| `probe.py` | `./bin/mcp-probe` — check the integration end to end |
+
+Nothing here is wired into the chat graph yet. Routing questions to these tools
+is `specs/007-answer-cascade`, and it belongs on the existing
+`intent_classifier` rather than in a second one.
+
+## Checking it works
+
+```bash
+export REACTOME_MCP_SERVER=~/git/reactome-mcp/dist/index.js
+./bin/mcp-probe
+```
+
+```
+  connected to reactome 1.4.0
+  server exposes 53 tools
+  chatbot wraps 5 of them
+  reactome_database_info             ok   0.15s      64 chars
+  reactome_species                   ok   0.39s    1988 chars
+  reactome_search                    ok   0.13s    3974 chars
+  reactome_get_pathway               ok   0.05s    2425 chars
+  reactome_analyze_identifiers       ok   0.04s    2667 chars
+```
+
+Several things fail identically from inside the chatbot — wrong path, no node,
+stale build, Content Service down, handshake rejected — and a question that
+quietly falls back to retrieval reports none of them. This separates them, and
+exits non-zero, so it can gate a deployment.
+
+## Five tools, not fifty-three
+
+Every tool description is spent from the model's context before it answers
+anything, and a model choosing between 53 similarly-named tools chooses worse
+than one choosing between five. The five cover what the bundle cannot do: live
+search, live pathway lookup, enrichment analysis, and the two metadata
+questions a snapshot cannot answer.
+
+Add to the list when a question is being answered wrongly without the tool —
+not because the tool exists.
+
+## Transport
+
+stdio, by spawning the server. reactome-mcp also serves Streamable HTTP, so a
+hosted instance can be used instead once there is one; that is
+`specs/006-mcp-hosting`. Keeping the transport inside `MCPProcessManager` is
+what makes that swap small.
+
+## Three things the original client did not do
+
+**The `initialize` handshake.** It went straight to `tools/call`. The server
+accepts that today because the SDK is lenient — verified — but the protocol
+requires it, and relying on leniency means the day an SDK release enforces it,
+every call fails at once. Doing it properly also replaced an arbitrary
+`sleep(1)` used to decide the server had started: a successful initialize *is*
+the readiness check.
+
+**Matching replies to requests.** It returned the next line on stdout, whatever
+it was. A notification arriving between request and reply would be read as the
+answer, and every later call would be one reply out of step — answering each
+question with the previous question's answer. Nothing raises; it just returns
+the wrong thing, plausibly. Two tests pin this.
+
+**One call at a time.** A lock serialises each write/read pair, because two
+coroutines interleaving on one pipe is the same desync by another route.

src/reactome_mcp/__init__.py

@@ -0,0 +1,17 @@
+"""Client for the Reactome MCP server.
+
+Named `reactome_mcp`, not `mcp`. The official MCP Python SDK is published on
+PyPI as `mcp`; a local package of that name shadows it, and the failure only
+appears the day someone adds the dependency. Harvested from #127/#137 by
+@GovindhKishore, where it was `src/mcp/`.
+"""
+
+from reactome_mcp.client import MCPClient, MCPToolError
+from reactome_mcp.process import MCPConnectionError, MCPProcessManager
+
+__all__ = [
+    "MCPClient",
+    "MCPConnectionError",
+    "MCPProcessManager",
+    "MCPToolError",
+]

src/reactome_mcp/client.py

@@ -0,0 +1,174 @@
+"""JSON-RPC over the MCP server's stdio.
+
+Harvested from #127 by @GovindhKishore, with three changes.
+
+**It performs the MCP initialize handshake.** The original went straight to
+`tools/call`. The server accepts that today because the SDK is lenient, but the
+protocol requires initialize first, and relying on leniency means the day an SDK
+release enforces it, every call fails at once. Doing it properly also removes
+the arbitrary `sleep(1)` the original used to decide the server had started: a
+successful initialize *is* the readiness check.
+
+**It matches responses to requests by id.** The original returned the next line
+on stdout, whatever it was. A server notification arriving between request and
+response would have been read as the answer, and every later call would be one
+reply out of step -- returning the previous question's answer, with nothing
+raising.
+
+**One call at a time.** A lock serialises the write/read pair, because two
+coroutines interleaving on one pipe is the same desync by another route.
+"""
+
+import asyncio
+import json
+import logging
+from typing import Any
+
+logger = logging.getLogger(__name__)
+
+PROTOCOL_VERSION = "2024-11-05"
+
+
+class MCPToolError(RuntimeError):
+    """The server returned a JSON-RPC error."""
+
+
+class MCPClient:
+    def __init__(
+        self,
+        process: asyncio.subprocess.Process,
+        timeout: float = 30.0,
+    ) -> None:
+        self.process = process
+        self.timeout = timeout
+        self._next_id = 0
+        self._lock = asyncio.Lock()
+        self._initialized = False
+
+    async def initialize(self, client_name: str = "reactome-chatbot") -> dict[str, Any]:
+        """Complete the handshake. Doubles as the readiness check."""
+        result = await self.call(
+            "initialize",
+            {
+                "protocolVersion": PROTOCOL_VERSION,
+                "capabilities": {},
+                "clientInfo": {"name": client_name, "version": "1"},
+            },
+        )
+        await self._notify("notifications/initialized")
+        self._initialized = True
+        server = result.get("serverInfo", {})
+        logger.info(
+            "MCP server ready: %s %s",
+            server.get("name", "unknown"),
+            server.get("version", ""),
+        )
+        return result
+
+    async def _write(self, payload: dict[str, Any]) -> None:
+        if self.process.stdin is None:
+            raise MCPToolError("MCP server stdin is closed")
+        self.process.stdin.write((json.dumps(payload) + "\n").encode("utf-8"))
+        await self.process.stdin.drain()
+
+    async def _notify(self, method: str) -> None:
+        """A notification has no id and gets no reply."""
+        async with self._lock:
+            await self._write({"jsonrpc": "2.0", "method": method})
+
+    async def _read_reply(self, request_id: int) -> dict[str, Any]:
+        """Read until the reply to this request arrives.
+
+        Anything that is not a reply to us -- a notification, a stray line --
+        is logged and skipped rather than returned. Returning it would answer
+        the caller's question with someone else's answer.
+        """
+        if self.process.stdout is None:
+            raise MCPToolError("MCP server stdout is closed")
+
+        while True:
+            line = await self.process.stdout.readline()
+            if not line:
+                raise MCPToolError("MCP server closed the connection")
+
+            text = line.decode("utf-8", errors="replace").strip()
+            if not text:
+                continue
+
+            try:
+                message = json.loads(text)
+            except json.JSONDecodeError:
+                logger.debug("ignoring non-JSON line from MCP server: %.200s", text)
+                continue
+
+            if message.get("id") != request_id:
+                logger.debug("ignoring MCP message not addressed to %s", request_id)
+                continue
+
+            if "error" in message:
+                error = message["error"]
+                raise MCPToolError(
+                    f"MCP error {error.get('code')}: {error.get('message')}"
+                )
+
+            # Checked, not asserted. json.loads gives Any, and casting it to
+            # the shape we hoped for is how reactome-mcp shipped ten formatters
+            # that read fields the API never returned.
+            result = message.get("result", {})
+            if not isinstance(result, dict):
+                raise MCPToolError(
+                    f"MCP returned a {type(result).__name__} result for "
+                    f"request {request_id}, expected an object"
+                )
+            return result
+
+    async def call(
+        self, method: str, params: dict[str, Any] | None = None
+    ) -> dict[str, Any]:
+        async with self._lock:
+            self._next_id += 1
+            request_id = self._next_id
+            await self._write(
+                {
+                    "jsonrpc": "2.0",
+                    "id": request_id,
+                    "method": method,
+                    "params": params or {},
+                }
+            )
+            return await asyncio.wait_for(
+                self._read_reply(request_id), timeout=self.timeout
+            )
+
+    async def call_tool(
+        self, name: str, arguments: dict[str, Any] | None = None
+    ) -> str:
+        """Call a tool and return its text, joined across content blocks."""
+        if not self._initialized:
+            await self.initialize()
+
+        result = await self.call(
+            "tools/call", {"name": name, "arguments": arguments or {}}
+        )
+        blocks = result.get("content", [])
+        if not isinstance(blocks, list):
+            raise MCPToolError(f"{name} returned no content blocks")
+        text = "\n".join(
+            str(block.get("text", ""))
+            for block in blocks
+            if isinstance(block, dict) and block.get("type") == "text"
+        )
+        if result.get("isError"):
+            raise MCPToolError(text or f"{name} failed with no message")
+        return text
+
+    async def list_tools(self) -> list[dict[str, Any]]:
+        if not self._initialized:
+            await self.initialize()
+        result = await self.call("tools/list")
+        tools = result.get("tools", [])
+        if not isinstance(tools, list):
+            raise MCPToolError(
+                f"MCP returned a {type(tools).__name__} tool list, expected an array"
+            )
+        return [tool for tool in tools if isinstance(tool, dict)]