From 00d9c8ffed160ade4ab075601b76706f5c4fd92d Mon Sep 17 00:00:00 2001
From: Kent Huang <kent@infuseai.io>
Date: Tue, 11 Aug 2026 15:32:07 +0800
Subject: [PATCH 01/12] fix(mcp): support both mcp 1.x and 2.0 SDKs

mcp 2.0 removed the low-level Server decorators (@server.list_tools(),
@server.call_tool()) in favour of constructor handlers with a
(ctx, params) -> Result signature, so `recce mcp-server` failed to start
with "'Server' object has no attribute 'list_tools'".

Select the registration path at import time via MCP_V2 and keep the two
handler bodies in their 1.x shapes; on 2.0 thin adapters wrap them into
ListToolsResult / CallToolResult. The call_tool adapter returns
isError=True explicitly because 2.0 turns a raised exception into a
JSON-RPC protocol error instead of a tool error.

Tests move to tests/mcp_compat.py helpers, since 2.0 drops the
request_handlers dict and renames Tool.inputSchema to input_schema.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Kent Huang <kent@infuseai.io>
---
 pyproject.toml                  |   2 +-
 recce/mcp_server.py             |  62 +++++++++++++--
 tests/mcp_compat.py             |  38 ++++++++++
 tests/test_mcp_cloud_backend.py |  50 ++++---------
 tests/test_mcp_server.py        | 129 ++++++++++++--------------------
 5 files changed, 152 insertions(+), 129 deletions(-)
 create mode 100644 tests/mcp_compat.py

diff --git a/pyproject.toml b/pyproject.toml
index 37ad77ec1..f6a9d455c 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -46,7 +46,7 @@ classifiers = [
 ]
 
 [project.optional-dependencies]
-mcp = ["mcp~=1.23"]
+mcp = ["mcp>=1.23,<3"]
 dev = [
     "pytest>=4.6",
     "pytest-asyncio>=0.23,<2.0",
diff --git a/recce/mcp_server.py b/recce/mcp_server.py
index 0746ba636..d2ad852e9 100644
--- a/recce/mcp_server.py
+++ b/recce/mcp_server.py
@@ -19,7 +19,7 @@
 import requests
 from mcp.server import Server
 from mcp.server.stdio import stdio_server
-from mcp.types import TextContent, Tool
+from mcp.types import CallToolResult, ListToolsResult, TextContent, Tool
 
 # DRC-3634: submit_run is the run-persistence entry point shared with the
 # recce server's run_router.  Local-mode ad-hoc diff tools import it to
@@ -43,6 +43,11 @@
 
 logger = logging.getLogger(__name__)
 
+# mcp 2.0 dropped the `@server.list_tools()` / `@server.call_tool()` decorators in
+# favour of constructor handlers with a `(ctx, params) -> Result` signature.
+# ponytail: one flag, two registration paths; drop the 1.x branch when the floor is mcp>=2.
+MCP_V2 = not hasattr(Server, "list_tools")
+
 try:
     from sentry_sdk import metrics as sentry_metrics
 except ImportError:  # pragma: no cover
@@ -661,9 +666,8 @@ def __init__(
         self.api_token = api_token
         self._backend_lock = asyncio.Lock()
         self._local_cache_key: Optional[tuple] = None
-        self.server = Server("recce", instructions=self._build_instructions())
         self.mcp_logger = MCPLogger(debug=debug, log_file=log_file)
-        self._setup_handlers()
+        self.server = self._build_server()
 
     def _build_instructions(self) -> Optional[str]:
         """Build MCP server instructions sent during initialize handshake."""
@@ -678,6 +682,47 @@ def _build_instructions(self) -> Optional[str]:
             "dbt docs generate --target-path target-base"
         )
 
+    def _build_server(self) -> Server:
+        """Create the low-level MCP server with tool handlers registered.
+
+        The handlers themselves (`_list_tools` / `_call_tool`) keep the mcp 1.x
+        shapes — `List[Tool]` and `List[TextContent]`, errors raised — because
+        that is what the 1.x SDK consumes directly. On mcp 2.0 the two thin
+        `_handle_*` adapters wrap them into `ListToolsResult` / `CallToolResult`.
+        """
+        self._list_tools, self._call_tool = self._make_handlers()
+
+        if not MCP_V2:
+            server = Server("recce", instructions=self._build_instructions())
+            server.list_tools()(self._list_tools)
+            server.call_tool()(self._call_tool)
+            return server
+
+        return Server(
+            "recce",
+            instructions=self._build_instructions(),
+            on_list_tools=self._handle_list_tools,
+            on_call_tool=self._handle_call_tool,
+        )
+
+    async def _handle_list_tools(self, ctx, params) -> ListToolsResult:
+        """`tools/list` in the mcp 2.0 handler signature (also used by tests on 1.x)."""
+        return ListToolsResult(tools=await self._list_tools())
+
+    async def _handle_call_tool(self, ctx, params) -> CallToolResult:
+        """`tools/call` in the mcp 2.0 handler signature (also used by tests on 1.x).
+
+        1.x turned a raised exception into `isError=True`; 2.0 turns it into a
+        JSON-RPC protocol error instead, which an agent reads as a transport
+        failure rather than a tool failure. Return the tool error explicitly to
+        keep the response identical across both versions.
+        """
+        try:
+            content = await self._call_tool(params.name, params.arguments or {})
+        except Exception as e:
+            return CallToolResult(content=[TextContent(type="text", text=str(e))], isError=True)
+        return CallToolResult(content=content)
+
     @staticmethod
     def _classify_db_error(error_msg: str) -> Optional[str]:
         """Classify a database error message into a known category.
@@ -769,10 +814,9 @@ async def _tool_run_backed_local(self, run_type: str, params: Dict[str, Any]) ->
             result = {**result, "run_id": str(run.run_id)}
         return result
 
-    def _setup_handlers(self):
-        """Register all tool handlers"""
+    def _make_handlers(self):
+        """Build the (list_tools, call_tool) handler pair in their mcp 1.x shapes."""
 
-        @self.server.list_tools()
         async def list_tools() -> List[Tool]:
             """List all available tools based on server mode"""
             logger.info(f"[MCP] list_tools called (mode: {self.mode.value if self.mode else 'server'})")
@@ -1510,7 +1554,6 @@ async def list_tools() -> List[Tool]:
 
             return tools
 
-        @self.server.call_tool()
         async def call_tool(name: str, arguments: Dict[str, Any]) -> List[TextContent]:
             """Handle tool calls"""
             start_time = time.perf_counter()
@@ -1647,9 +1690,12 @@ async def call_tool(name: str, arguments: Dict[str, Any]) -> List[TextContent]:
                     logger.error(f"[MCP] Error executing tool {name} ({duration_ms:.2f}ms): {error_msg}")
                     logger.exception("[MCP] Full traceback:")
 
-                # Re-raise so MCP SDK sets isError=True in the protocol response
+                # Re-raise so the caller reports isError=True: on mcp 1.x the SDK does
+                # it, on 2.0 _handle_call_tool does (see its docstring).
                 raise
 
+        return list_tools, call_tool
+
     async def _tool_lineage_diff(self, arguments: Dict[str, Any]) -> Dict[str, Any]:
         """Get lineage diff between base and current"""
         # Extract filter arguments
diff --git a/tests/mcp_compat.py b/tests/mcp_compat.py
new file mode 100644
index 000000000..9bcaf3c5e
--- /dev/null
+++ b/tests/mcp_compat.py
@@ -0,0 +1,38 @@
+"""Helpers for invoking MCP handlers across mcp 1.x and 2.0.
+
+The two SDK majors disagree on handler registration (decorators vs constructor
+kwargs) and on field naming (``Tool.inputSchema`` vs ``Tool.input_schema``).
+Tests go through these helpers so they read the same on both.
+"""
+
+from dataclasses import dataclass
+from typing import Any, Dict, List, Optional
+
+from mcp.types import CallToolRequestParams, TextContent, Tool
+
+
+@dataclass
+class ToolResult:
+    """Normalised ``tools/call`` result (1.x ``isError`` / 2.0 ``is_error``)."""
+
+    content: List[TextContent]
+    isError: bool
+
+
+async def invoke_call_tool(server, name: str, arguments: Optional[Dict[str, Any]] = None) -> ToolResult:
+    """Call a tool and normalise the result, including the error case."""
+    result = await server._handle_call_tool(None, CallToolRequestParams(name=name, arguments=arguments or {}))
+    is_error = getattr(result, "is_error", None)
+    if is_error is None:
+        is_error = getattr(result, "isError", None)
+    return ToolResult(content=list(result.content), isError=bool(is_error))
+
+
+async def invoke_list_tools(server) -> List[Tool]:
+    """Return the advertised tools."""
+    return (await server._handle_list_tools(None, None)).tools
+
+
+def input_schema(tool: Tool) -> Dict[str, Any]:
+    """Read a tool's JSON schema regardless of SDK field naming."""
+    return getattr(tool, "input_schema", None) or tool.inputSchema
diff --git a/tests/test_mcp_cloud_backend.py b/tests/test_mcp_cloud_backend.py
index 7814efe2b..7089d9c88 100644
--- a/tests/test_mcp_cloud_backend.py
+++ b/tests/test_mcp_cloud_backend.py
@@ -5,8 +5,6 @@
 
 pytest.importorskip("mcp")
 
-from mcp.types import CallToolRequest, CallToolRequestParams  # noqa: E402
-
 from recce.mcp_server import (  # noqa: E402
     CloudBackend,
     InstanceSpawningError,
@@ -14,6 +12,7 @@
     run_mcp_server,
 )
 from recce.util.recce_cloud import RecceCloudException  # noqa: E402
+from tests.mcp_compat import invoke_call_tool  # noqa: E402
 
 
 class MockResponse:
@@ -258,17 +257,11 @@ async def test_recce_mcp_server_delegates_tool_calls_to_backend():
     backend.call_tool.return_value = {"ok": True}
     server = RecceMCPServer(backend=backend)
 
-    handler = server.server.request_handlers[CallToolRequest]
-    request = CallToolRequest(
-        method="tools/call",
-        params=CallToolRequestParams(name="get_server_info", arguments={}),
-    )
-
-    result = await handler(request)
+    result = await invoke_call_tool(server, "get_server_info")
 
     backend.call_tool.assert_awaited_once_with("get_server_info", {})
     # DRC-3758: tool results are now serialized compactly (no indent).
-    assert result.root.content[0].text == '{"ok":true}'
+    assert result.content[0].text == '{"ok":true}'
 
 
 @pytest.mark.asyncio
@@ -422,26 +415,14 @@ async def test_set_backend_invalid_mode_raises():
 async def test_unconfigured_server_blocks_normal_tools_but_allows_set_backend():
     """Tools other than set_backend / get_server_info are gated when unconfigured."""
     server = RecceMCPServer()
-    handler = server.server.request_handlers[CallToolRequest]
-
     # Normal tool blocked
-    blocked = await handler(
-        CallToolRequest(
-            method="tools/call",
-            params=CallToolRequestParams(name="lineage_diff", arguments={}),
-        )
-    )
-    assert blocked.root.isError is True
-    assert "No backend configured" in blocked.root.content[0].text
+    blocked = await invoke_call_tool(server, "lineage_diff")
+    assert blocked.isError is True
+    assert "No backend configured" in blocked.content[0].text
 
     # get_server_info returns mode='none'
-    info = await handler(
-        CallToolRequest(
-            method="tools/call",
-            params=CallToolRequestParams(name="get_server_info", arguments={}),
-        )
-    )
-    assert '"mode":"none"' in info.root.content[0].text
+    info = await invoke_call_tool(server, "get_server_info")
+    assert '"mode":"none"' in info.content[0].text
 
 
 @pytest.mark.asyncio
@@ -898,21 +879,16 @@ async def test_set_backend_api_token_redacted_in_logs(caplog):
     backend.call_tool.return_value = {"ok": True}
     server = RecceMCPServer(api_token=None)
 
-    handler = server.server.request_handlers[CallToolRequest]
-    request = CallToolRequest(
-        method="tools/call",
-        params=CallToolRequestParams(
-            name="set_backend",
-            arguments={"mode": "cloud", "session_id": "sess-123", "api_token": "sk-real-secret"},
-        ),
-    )
-
     with (
         caplog.at_level(logging.INFO, logger="recce.mcp_server"),
         patch("recce.mcp_server.CloudBackend.create", return_value=backend),
         patch.object(server.mcp_logger, "log_tool_call") as mock_log_tool_call,
     ):
-        await handler(request)
+        await invoke_call_tool(
+            server,
+            "set_backend",
+            {"mode": "cloud", "session_id": "sess-123", "api_token": "sk-real-secret"},
+        )
 
     # Stderr/console logs must not contain the raw token
     assert "sk-real-secret" not in caplog.text
diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py
index 6f9d137be..176e4b536 100644
--- a/tests/test_mcp_server.py
+++ b/tests/test_mcp_server.py
@@ -6,12 +6,6 @@
 # Skip all tests in this module if mcp is not available
 pytest.importorskip("mcp")
 
-from mcp.types import (  # noqa: E402
-    CallToolRequest,
-    CallToolRequestParams,
-    ListToolsRequest,
-)
-
 from recce.core import RecceContext  # noqa: E402
 from recce.mcp_server import (  # noqa: E402
     _UNCHECKED_NODE_LIMIT,
@@ -26,6 +20,11 @@
 from recce.tasks.rowcount import RowCountDiffTask  # noqa: E402
 from recce.tasks.top_k import TopKDiffTask  # noqa: E402
 from recce.tasks.valuediff import ValueDiffDetailTask, ValueDiffTask  # noqa: E402
+from tests.mcp_compat import (  # noqa: E402
+    input_schema,
+    invoke_call_tool,
+    invoke_list_tools,
+)
 
 
 @pytest.fixture
@@ -1308,10 +1307,8 @@ async def test_analyze_model_advertised_in_local_mode_only(self):
         a 'Unknown tool' error because RecceMCPCloudBackend doesn't implement it."""
         local_server = RecceMCPServer(MagicMock(spec=RecceContext), backend=None)
         cloud_server = RecceMCPServer(MagicMock(spec=RecceContext), backend=MagicMock())
-        req = ListToolsRequest(method="tools/list", params=None)
-
-        local_tools = (await local_server.server.request_handlers[ListToolsRequest](req)).root.tools
-        cloud_tools = (await cloud_server.server.request_handlers[ListToolsRequest](req)).root.tools
+        local_tools = await invoke_list_tools(local_server)
+        cloud_tools = await invoke_list_tools(cloud_server)
 
         assert any(t.name == "analyze_model" for t in local_tools)
         assert not any(t.name == "analyze_model" for t in cloud_tools)
@@ -1857,18 +1854,14 @@ async def test_non_server_mode_blocks_new_diff_tools(self):
         blocked_tools = ["value_diff", "value_diff_detail", "top_k_diff", "histogram_diff"]
         for tool_name in blocked_tools:
             result = await TestCallToolHandler._invoke_call_tool(server, tool_name, {})
-            assert result.root.isError is True
+            assert result.isError is True
 
     @pytest.mark.asyncio
     async def test_create_check_in_server_mode_tools(self):
         """create_check tool is available in server mode."""
-        from mcp.types import ListToolsRequest
-
         mock_context = MagicMock(spec=RecceContext)
         server = RecceMCPServer(mock_context, mode=RecceServerMode.server)
-        handler = server.server.request_handlers[ListToolsRequest]
-        result = await handler(ListToolsRequest(method="tools/list"))
-        tools = result.root.tools
+        tools = await invoke_list_tools(server)
         tool_names = [t.name for t in tools]
         assert "create_check" in tool_names
 
@@ -1882,7 +1875,7 @@ async def test_create_check_blocked_in_non_server_mode(self):
             "create_check",
             {"type": "row_count_diff", "params": {}, "name": "test"},
         )
-        assert r.root.isError is True
+        assert r.isError is True
 
 
 @pytest.fixture
@@ -2003,12 +1996,8 @@ async def test_query_no_warning_in_single_env(self, mcp_server_single_env):
     @pytest.mark.asyncio
     async def test_diff_tool_descriptions_have_single_env_note(self, mcp_server_single_env):
         """Diff tool descriptions should include single-env note when in single-env mode"""
-        from mcp.types import ListToolsRequest
-
         server, _ = mcp_server_single_env
-        handler = server.server.request_handlers[ListToolsRequest]
-        result = await handler(ListToolsRequest(method="tools/list"))
-        tools = result.root.tools
+        tools = await invoke_list_tools(server)
 
         diff_tool_names = {
             "row_count_diff",
@@ -2028,12 +2017,8 @@ async def test_diff_tool_descriptions_have_single_env_note(self, mcp_server_sing
     @pytest.mark.asyncio
     async def test_diff_tool_descriptions_no_note_in_normal_mode(self, mcp_server):
         """Diff tool descriptions should NOT include single-env note in normal mode"""
-        from mcp.types import ListToolsRequest
-
         server, _ = mcp_server
-        handler = server.server.request_handlers[ListToolsRequest]
-        result = await handler(ListToolsRequest(method="tools/list"))
-        tools = result.root.tools
+        tools = await invoke_list_tools(server)
 
         note_text = "base environment is not configured"
 
@@ -2046,16 +2031,12 @@ async def test_diff_tool_descriptions_no_note_in_normal_mode(self, mcp_server):
     async def test_select_param_descriptions_warn_selector_grammar(self, mcp_server):
         """Every tool exposing a `select` param must warn about dbt selector grammar:
         a comma is intersection, so comma-joining distinct model names silently returns empty."""
-        from mcp.types import ListToolsRequest
-
         server, _ = mcp_server
-        handler = server.server.request_handlers[ListToolsRequest]
-        result = await handler(ListToolsRequest(method="tools/list"))
-        tools = result.root.tools
+        tools = await invoke_list_tools(server)
 
         checked = 0
         for tool in tools:
-            select = tool.inputSchema.get("properties", {}).get("select")
+            select = input_schema(tool).get("properties", {}).get("select")
             if select is None:
                 continue
             checked += 1
@@ -2177,12 +2158,7 @@ class TestCallToolHandler:
     @staticmethod
     async def _invoke_call_tool(server, tool_name, arguments=None):
         """Invoke the registered call_tool handler directly via MCP Server internals."""
-        handler = server.server.request_handlers[CallToolRequest]
-        req = CallToolRequest(
-            method="tools/call",
-            params=CallToolRequestParams(name=tool_name, arguments=arguments or {}),
-        )
-        return await handler(req)
+        return await invoke_call_tool(server, tool_name, arguments)
 
     @pytest.mark.asyncio
     async def test_classified_error_logs_warning(self, mcp_server, caplog):
@@ -2195,7 +2171,7 @@ async def test_classified_error_logs_warning(self, mcp_server, caplog):
         with caplog.at_level(logging.WARNING, logger="recce.mcp_server"):
             result = await self._invoke_call_tool(server, "lineage_diff")
 
-        assert result.root.isError is True
+        assert result.isError is True
         assert "Expected table_not_found error" in caplog.text
 
     @pytest.mark.asyncio
@@ -2209,7 +2185,7 @@ async def test_unclassified_error_logs_error(self, mcp_server, caplog):
         with caplog.at_level(logging.ERROR, logger="recce.mcp_server"):
             result = await self._invoke_call_tool(server, "lineage_diff")
 
-        assert result.root.isError is True
+        assert result.isError is True
         assert "Error executing tool lineage_diff" in caplog.text
 
     @pytest.mark.asyncio
@@ -2235,7 +2211,7 @@ async def test_classified_error_skips_metric_when_sentry_unavailable(self, mcp_s
         with patch("recce.mcp_server.sentry_metrics", None):
             result = await self._invoke_call_tool(server, "lineage_diff")
 
-        assert result.root.isError is True
+        assert result.isError is True
 
     @pytest.mark.asyncio
     async def test_existing_tools_dispatch_via_call_tool(self, mcp_server):
@@ -2250,33 +2226,33 @@ async def test_existing_tools_dispatch_via_call_tool(self, mcp_server):
         }
         mock_context.get_lineage_diff.return_value = mock_lineage_diff
         r = await self._invoke_call_tool(server, "schema_diff", {})
-        assert r.root.isError is not True
+        assert r.isError is not True
 
         # row_count_diff
         with patch.object(RowCountDiffTask, "execute", return_value={"m": {"base": 1, "curr": 1}}):
             r = await self._invoke_call_tool(server, "row_count_diff", {"node_names": ["m"]})
-        assert r.root.isError is not True
+        assert r.isError is not True
 
         # query
         mock_qr = MagicMock()
         mock_qr.model_dump.return_value = {"columns": ["c"], "data": [[1]]}
         with patch.object(QueryTask, "execute", return_value=mock_qr):
             r = await self._invoke_call_tool(server, "query", {"sql_template": "SELECT 1"})
-        assert r.root.isError is not True
+        assert r.isError is not True
 
         # query_diff
         mock_qdr = MagicMock()
         mock_qdr.model_dump.return_value = {"diff": {"added": [], "removed": [], "modified": []}}
         with patch.object(QueryDiffTask, "execute", return_value=mock_qdr):
             r = await self._invoke_call_tool(server, "query_diff", {"sql_template": "SELECT 1"})
-        assert r.root.isError is not True
+        assert r.isError is not True
 
         # profile_diff
         mock_pdr = MagicMock()
         mock_pdr.model_dump.return_value = {"columns": {}}
         with patch.object(ProfileDiffTask, "execute", return_value=mock_pdr):
             r = await self._invoke_call_tool(server, "profile_diff", {"model": "m"})
-        assert r.root.isError is not True
+        assert r.isError is not True
 
         # list_checks
         mock_check_dao = MagicMock()
@@ -2284,7 +2260,7 @@ async def test_existing_tools_dispatch_via_call_tool(self, mcp_server):
         mock_check_dao.status.return_value = {"total": 0, "approved": 0}
         with patch("recce.models.CheckDAO", return_value=mock_check_dao):
             r = await self._invoke_call_tool(server, "list_checks", {})
-        assert r.root.isError is not True
+        assert r.isError is not True
 
         # run_check (successful dispatch via lineage_diff path)
         from uuid import uuid4
@@ -2315,11 +2291,11 @@ async def test_existing_tools_dispatch_via_call_tool(self, mcp_server):
             patch("recce.apis.check_func.export_persistent_state"),
         ):
             r = await self._invoke_call_tool(server, "run_check", {"check_id": str(check_id)})
-        assert r.root.isError is not True
+        assert r.isError is not True
 
         # unknown tool
         r = await self._invoke_call_tool(server, "nonexistent_tool", {})
-        assert r.root.isError is True
+        assert r.isError is True
 
     @pytest.mark.asyncio
     async def test_create_check_dispatches_via_call_tool(self, mcp_server):
@@ -2352,7 +2328,7 @@ async def test_create_check_dispatches_via_call_tool(self, mcp_server):
                     "name": "test",
                 },
             )
-        assert r.root.isError is not True
+        assert r.isError is not True
 
     @pytest.mark.asyncio
     async def test_new_syntax_error_logs_warning(self, mcp_server, caplog):
@@ -2365,7 +2341,7 @@ async def test_new_syntax_error_logs_warning(self, mcp_server, caplog):
         )
         with caplog.at_level(logging.WARNING, logger="recce.mcp_server"):
             result = await self._invoke_call_tool(server, "lineage_diff")
-        assert result.root.isError is True
+        assert result.isError is True
         assert "Expected syntax_error error" in caplog.text
 
     @pytest.mark.asyncio
@@ -2376,7 +2352,7 @@ async def test_large_response_truncates_log(self, mcp_server):
         large_result = {"data": "x" * 2000}
         with patch.object(RowCountDiffTask, "execute", return_value=large_result):
             r = await self._invoke_call_tool(server, "row_count_diff", {"node_names": ["m"]})
-        assert r.root.isError is not True
+        assert r.isError is not True
 
     @pytest.mark.asyncio
     async def test_new_tools_dispatch_via_call_tool(self, mcp_server):
@@ -2388,31 +2364,31 @@ async def test_new_tools_dispatch_via_call_tool(self, mcp_server):
         mock_vd.model_dump.return_value = {"summary": {}, "data": {}}
         with patch.object(ValueDiffTask, "execute", return_value=mock_vd):
             r = await self._invoke_call_tool(server, "value_diff", {"model": "m", "primary_key": "id"})
-        assert r.root.isError is not True
+        assert r.isError is not True
 
         # value_diff_detail
         mock_vdd = MagicMock()
         mock_vdd.model_dump.return_value = {"columns": [], "data": []}
         with patch.object(ValueDiffDetailTask, "execute", return_value=mock_vdd):
             r = await self._invoke_call_tool(server, "value_diff_detail", {"model": "m", "primary_key": "id"})
-        assert r.root.isError is not True
+        assert r.isError is not True
 
         # top_k_diff
         with patch.object(TopKDiffTask, "execute", return_value={"base": {}, "current": {}}):
             r = await self._invoke_call_tool(server, "top_k_diff", {"model": "m", "column_name": "c"})
-        assert r.root.isError is not True
+        assert r.isError is not True
 
         # histogram_diff
         mock_context.build_name_to_unique_id_index.return_value = {"m": "model.p.m"}
         mock_context.get_model.return_value = {"columns": {"c": {"name": "c", "type": "INTEGER"}}}
         with patch.object(HistogramDiffTask, "execute", return_value={"base": {}, "current": {}}):
             r = await self._invoke_call_tool(server, "histogram_diff", {"model": "m", "column_name": "c"})
-        assert r.root.isError is not True
+        assert r.isError is not True
 
         # get_model
         mock_context.get_model.side_effect = [{"columns": {}}, {"columns": {}}]
         r = await self._invoke_call_tool(server, "get_model", {"model_id": "model.p.m"})
-        assert r.root.isError is not True
+        assert r.isError is not True
 
         # get_cll
         mock_context.adapter_type = "dbt"
@@ -2420,7 +2396,7 @@ async def test_new_tools_dispatch_via_call_tool(self, mcp_server):
         mock_cll.model_dump.return_value = {"nodes": {}, "columns": {}, "parent_map": {}, "child_map": {}}
         mock_context.adapter.get_cll.return_value = mock_cll
         r = await self._invoke_call_tool(server, "get_cll", {})
-        assert r.root.isError is not True
+        assert r.isError is not True
 
         # get_server_info
         mock_context.adapter_type = "dbt"
@@ -2428,13 +2404,13 @@ async def test_new_tools_dispatch_via_call_tool(self, mcp_server):
         mock_context.support_tasks.return_value = {}
         mock_context.state_loader = None
         r = await self._invoke_call_tool(server, "get_server_info", {})
-        assert r.root.isError is not True
+        assert r.isError is not True
 
         # select_nodes
         mock_context.adapter_type = "dbt"
         mock_context.adapter.select_nodes.return_value = {"model.p.m"}
         r = await self._invoke_call_tool(server, "select_nodes", {})
-        assert r.root.isError is not True
+        assert r.isError is not True
 
 
 class TestLineageDiffEdgeCases:
@@ -2650,25 +2626,17 @@ class TestImpactAnalysisRegistration:
 
     @pytest.mark.asyncio
     async def test_impact_analysis_in_tool_list(self, mcp_server):
-        from mcp.types import ListToolsRequest
-
         server, mock_context = mcp_server
-        handler = server.server.request_handlers[ListToolsRequest]
-        result = await handler(ListToolsRequest(method="tools/list"))
-        tool_names = [t.name for t in result.root.tools]
+        tool_names = [t.name for t in await invoke_list_tools(server)]
         assert "impact_analysis" in tool_names
 
     @pytest.mark.asyncio
     async def test_impact_analysis_schema_has_select(self, mcp_server):
-        from mcp.types import ListToolsRequest
-
         server, mock_context = mcp_server
-        handler = server.server.request_handlers[ListToolsRequest]
-        result = await handler(ListToolsRequest(method="tools/list"))
-        tool = next(t for t in result.root.tools if t.name == "impact_analysis")
-        assert "select" in tool.inputSchema["properties"]
-        assert "skip_value_diff" in tool.inputSchema["properties"]
-        assert "skip_downstream_value_diff" in tool.inputSchema["properties"]
+        tool = next(t for t in await invoke_list_tools(server) if t.name == "impact_analysis")
+        assert "select" in input_schema(tool)["properties"]
+        assert "skip_value_diff" in input_schema(tool)["properties"]
+        assert "skip_downstream_value_diff" in input_schema(tool)["properties"]
 
 
 class TestImpactAnalysisBehavior:
@@ -2837,15 +2805,10 @@ def setup_impact_mocks(self, mcp_server):
     @staticmethod
     async def _call_impact_analysis(server, **extra_args):
         """Invoke impact_analysis via the MCP call_tool handler."""
-        handler = server.server.request_handlers[CallToolRequest]
-        req = CallToolRequest(
-            method="tools/call",
-            params=CallToolRequestParams(name="impact_analysis", arguments=extra_args),
-        )
-        result = await handler(req)
+        result = await invoke_call_tool(server, "impact_analysis", extra_args)
         import json
 
-        return json.loads(result.root.content[0].text)
+        return json.loads(result.content[0].text)
 
     # ---------------------------------------------------------------------------
     # Tests
@@ -3498,9 +3461,9 @@ async def test_handler_surfaces_failed_run_as_iserror_and_persists(self, server)
             result = await TestCallToolHandler._invoke_call_tool(
                 server, "query_diff", {"sql_template": "SELECT bad_col", "primary_keys": ["id"]}
             )
-        assert result.root.isError is True
+        assert result.isError is True
         # The original message is surfaced so _classify_db_error / the agent can see it.
-        assert "bad_col" in result.root.content[0].text
+        assert "bad_col" in result.content[0].text
         # The FAILED Run is still persisted for citation, not dropped.
         assert len(self._context.runs) == 1
         assert self._context.runs[0].status == RunStatus.FAILED

From 3e470158a91b1bbe965ccc9abf230a7f2c5424ca Mon Sep 17 00:00:00 2001
From: Kent Huang <kent@infuseai.io>
Date: Tue, 11 Aug 2026 15:39:25 +0800
Subject: [PATCH 02/12] test(smoke): add mcp-server mode to the dbt smoke test

`recce mcp-server` speaks stdio, so there is no port to poll: the check
feeds it initialize / notifications/initialized / tools/list and asserts
the server identifies itself and advertises at least one tool. A startup
failure like the mcp 1.x/2.0 handler-registration split shows up here.

SMOKE_SERVER selects the surface ("server" by default, so existing
callers are unchanged); SMOKE_MCP_VERSION pins the SDK version to
install, since mcp is an optional extra that CI's install does not carry.
A new matrix job runs the mcp-server mode against both majors.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Kent Huang <kent@infuseai.io>
---
 .github/workflows/integration-tests.yaml | 30 +++++++++++
 integration_tests/dbt/smoke_test.sh      | 69 +++++++++++++++++++++---
 2 files changed, 93 insertions(+), 6 deletions(-)

diff --git a/.github/workflows/integration-tests.yaml b/.github/workflows/integration-tests.yaml
index 854dfae4c..b839de2d1 100644
--- a/.github/workflows/integration-tests.yaml
+++ b/.github/workflows/integration-tests.yaml
@@ -49,3 +49,33 @@ jobs:
         run: |
           source .venv/bin/activate
           ./integration_tests/dbt/smoke_test.sh
+
+  mcp-smoke-test:
+    if: github.actor != 'dependabot[bot]'
+    runs-on: ubuntu-latest
+    strategy:
+      # `mcp` is an optional extra pinned to >=1.23,<3, and the two majors
+      # register tool handlers differently. Both have to boot.
+      matrix:
+        mcp-version: ["1.28.1", "2.0.0"]
+    steps:
+      - uses: actions/checkout@v4
+
+      - name: Install uv
+        uses: astral-sh/setup-uv@v4
+        with:
+          version: "latest"
+
+      - name: Install Recce and dbt
+        run: |
+          uv venv
+          uv sync --no-dev --python 3.12
+          uv pip install dbt-core dbt-duckdb
+
+      - name: Run smoke test - mcp server
+        env:
+          SMOKE_SERVER: mcp-server
+          SMOKE_MCP_VERSION: ${{ matrix.mcp-version }}
+        run: |
+          source .venv/bin/activate
+          ./integration_tests/dbt/smoke_test.sh
diff --git a/integration_tests/dbt/smoke_test.sh b/integration_tests/dbt/smoke_test.sh
index 31c12f1f4..6ef35c051 100755
--- a/integration_tests/dbt/smoke_test.sh
+++ b/integration_tests/dbt/smoke_test.sh
@@ -5,6 +5,29 @@ SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 cd "$SCRIPT_DIR"
 pwd
 
+# Which server surface to smoke test: "server" (default) or "mcp-server".
+SMOKE_SERVER="${SMOKE_SERVER:-server}"
+# Only used when SMOKE_SERVER=mcp-server. The mcp SDK majors register tool
+# handlers differently, so the version under test has to be explicit.
+SMOKE_MCP_VERSION="${SMOKE_MCP_VERSION:-2.0.0}"
+
+case "$SMOKE_SERVER" in
+    server) ;;
+    mcp-server)
+        # `mcp` is an optional extra, so CI's install does not carry it.
+        echo "Installing mcp==$SMOKE_MCP_VERSION"
+        if command -v uv > /dev/null; then
+            uv pip install "mcp==$SMOKE_MCP_VERSION"
+        else
+            python -m pip install "mcp==$SMOKE_MCP_VERSION"
+        fi
+        ;;
+    *)
+        echo "Unknown SMOKE_SERVER '$SMOKE_SERVER' (expected 'server' or 'mcp-server')."
+        exit 1
+        ;;
+esac
+
 # Prepare env
 git restore models/customers.sql
 dbt --version
@@ -104,10 +127,44 @@ function check_server_status() {
     echo "Server stopped."
 }
 
-echo "Starting the server..."
-recce server &
-check_server_status false
+# Recce MCP Server
+# Stdio, not HTTP: there is no port to poll, so speak the protocol instead. The
+# startup path is what breaks (tool registration differs between mcp 1.x and
+# 2.0), so the handshake plus a non-empty tool list is the whole check.
+function check_mcp_server_status() {
+    echo "Starting the MCP server..."
+    local stderr_log output server_name tool_count
+    stderr_log=$(mktemp)
+
+    if ! output=$( {
+        echo '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"smoke","version":"0"}}}'
+        echo '{"jsonrpc":"2.0","method":"notifications/initialized"}'
+        echo '{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}'
+    } | timeout 60 recce mcp-server 2>"$stderr_log" ); then
+        echo "The MCP server failed to start:"
+        cat "$stderr_log"
+        exit 1
+    fi
+
+    server_name=$(jq -r 'select(.id == 1) | .result.serverInfo.name' <<< "$output")
+    tool_count=$(jq -r 'select(.id == 2) | .result.tools | length' <<< "$output")
+    assert_string_value "$server_name" "recce"
+    if [ "${tool_count:-0}" -lt 1 ]; then
+        echo "The MCP server started but advertised no tools."
+        cat "$stderr_log"
+        exit 1
+    fi
+    echo "MCP server is up and advertised $tool_count tools."
+}
+
+if [ "$SMOKE_SERVER" = "mcp-server" ]; then
+    check_mcp_server_status
+else
+    echo "Starting the server..."
+    recce server &
+    check_server_status false
 
-echo "Starting the server (review mode)..."
-recce server --review recce_state.json &
-check_server_status true
+    echo "Starting the server (review mode)..."
+    recce server --review recce_state.json &
+    check_server_status true
+fi

From 30d49a4aef3aaf8d252c07a04b02d83876bc3f14 Mon Sep 17 00:00:00 2001
From: Kent Huang <kent@infuseai.io>
Date: Tue, 11 Aug 2026 15:57:18 +0800
Subject: [PATCH 03/12] fix(smoke): address PR review on the mcp-server smoke
 test

- Launch `recce mcp-server` outside check_mcp_server_status, mirroring
  how check_server_status is called. The handshake is written to a file
  and fed on stdin, so the server can be backgrounded like `recce server &`
  and the check just waits on its pid.
- Take major.minor mcp versions (`1.29`, `2.0`) and install with `~=`
  instead of `==`, so a new patch release is picked up automatically.
- Add an explicit `permissions: contents: read` block to the workflow
  (CodeQL: workflow does not limit GITHUB_TOKEN permissions).
- Drop a stray tooling marker from the MCP_V2 comment.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Kent Huang <kent@infuseai.io>
---
 .github/workflows/integration-tests.yaml |  5 +-
 integration_tests/dbt/smoke_test.sh      | 58 ++++++++++++++----------
 recce/mcp_server.py                      |  4 +-
 3 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/.github/workflows/integration-tests.yaml b/.github/workflows/integration-tests.yaml
index b839de2d1..dabfdd492 100644
--- a/.github/workflows/integration-tests.yaml
+++ b/.github/workflows/integration-tests.yaml
@@ -14,6 +14,9 @@ on:
       - "js/**"
       - "recce/data/**"
 
+permissions:
+  contents: read
+
 jobs:
   smoke-test:
     if: github.actor != 'dependabot[bot]'
@@ -57,7 +60,7 @@ jobs:
       # `mcp` is an optional extra pinned to >=1.23,<3, and the two majors
       # register tool handlers differently. Both have to boot.
       matrix:
-        mcp-version: ["1.28.1", "2.0.0"]
+        mcp-version: ["1.29", "2.0"]
     steps:
       - uses: actions/checkout@v4
 
diff --git a/integration_tests/dbt/smoke_test.sh b/integration_tests/dbt/smoke_test.sh
index 6ef35c051..596332437 100755
--- a/integration_tests/dbt/smoke_test.sh
+++ b/integration_tests/dbt/smoke_test.sh
@@ -7,19 +7,20 @@ pwd
 
 # Which server surface to smoke test: "server" (default) or "mcp-server".
 SMOKE_SERVER="${SMOKE_SERVER:-server}"
-# Only used when SMOKE_SERVER=mcp-server. The mcp SDK majors register tool
-# handlers differently, so the version under test has to be explicit.
-SMOKE_MCP_VERSION="${SMOKE_MCP_VERSION:-2.0.0}"
+# Only used when SMOKE_SERVER=mcp-server. Major.minor, e.g. "1.29" or "2.0":
+# the mcp SDK majors register tool handlers differently, so the version under
+# test has to be explicit, but `~=` still picks up the latest patch release.
+SMOKE_MCP_VERSION="${SMOKE_MCP_VERSION:-2.0}"
 
 case "$SMOKE_SERVER" in
     server) ;;
     mcp-server)
         # `mcp` is an optional extra, so CI's install does not carry it.
-        echo "Installing mcp==$SMOKE_MCP_VERSION"
+        echo "Installing mcp~=$SMOKE_MCP_VERSION"
         if command -v uv > /dev/null; then
-            uv pip install "mcp==$SMOKE_MCP_VERSION"
+            uv pip install "mcp~=$SMOKE_MCP_VERSION"
         else
-            python -m pip install "mcp==$SMOKE_MCP_VERSION"
+            python -m pip install "mcp~=$SMOKE_MCP_VERSION"
         fi
         ;;
     *)
@@ -128,37 +129,46 @@ function check_server_status() {
 }
 
 # Recce MCP Server
-# Stdio, not HTTP: there is no port to poll, so speak the protocol instead. The
-# startup path is what breaks (tool registration differs between mcp 1.x and
-# 2.0), so the handshake plus a non-empty tool list is the whole check.
+# Stdio, not HTTP: there is no port to poll, so the server is started with the
+# handshake on stdin and its replies land in a file. The startup path is what
+# breaks (tool registration differs between mcp 1.x and 2.0), so the handshake
+# plus a non-empty tool list is the whole check.
+MCP_REQUESTS=$(mktemp)
+MCP_RESPONSES=$(mktemp)
+MCP_STDERR=$(mktemp)
+cat > "$MCP_REQUESTS" << 'EOF'
+{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"smoke","version":"0"}}}
+{"jsonrpc":"2.0","method":"notifications/initialized"}
+{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}
+EOF
+
+# Takes the pid of the backgrounded `recce mcp-server`. The server exits once it
+# reaches the end of the request file, so waiting on it is the readiness signal.
 function check_mcp_server_status() {
-    echo "Starting the MCP server..."
-    local stderr_log output server_name tool_count
-    stderr_log=$(mktemp)
-
-    if ! output=$( {
-        echo '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"smoke","version":"0"}}}'
-        echo '{"jsonrpc":"2.0","method":"notifications/initialized"}'
-        echo '{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}'
-    } | timeout 60 recce mcp-server 2>"$stderr_log" ); then
-        echo "The MCP server failed to start:"
-        cat "$stderr_log"
+    local mcp_pid="$1"
+    echo "Waiting for the MCP server to respond..."
+    if ! wait "$mcp_pid"; then
+        echo "Failed to start the MCP server."
+        cat "$MCP_STDERR"
         exit 1
     fi
 
-    server_name=$(jq -r 'select(.id == 1) | .result.serverInfo.name' <<< "$output")
-    tool_count=$(jq -r 'select(.id == 2) | .result.tools | length' <<< "$output")
+    local server_name tool_count
+    server_name=$(jq -r 'select(.id == 1) | .result.serverInfo.name' "$MCP_RESPONSES")
+    tool_count=$(jq -r 'select(.id == 2) | .result.tools | length' "$MCP_RESPONSES")
     assert_string_value "$server_name" "recce"
     if [ "${tool_count:-0}" -lt 1 ]; then
         echo "The MCP server started but advertised no tools."
-        cat "$stderr_log"
+        cat "$MCP_STDERR"
         exit 1
     fi
     echo "MCP server is up and advertised $tool_count tools."
 }
 
 if [ "$SMOKE_SERVER" = "mcp-server" ]; then
-    check_mcp_server_status
+    echo "Starting the MCP server..."
+    timeout 60 recce mcp-server < "$MCP_REQUESTS" > "$MCP_RESPONSES" 2> "$MCP_STDERR" &
+    check_mcp_server_status $!
 else
     echo "Starting the server..."
     recce server &
diff --git a/recce/mcp_server.py b/recce/mcp_server.py
index d2ad852e9..3dc9f824b 100644
--- a/recce/mcp_server.py
+++ b/recce/mcp_server.py
@@ -44,8 +44,8 @@
 logger = logging.getLogger(__name__)
 
 # mcp 2.0 dropped the `@server.list_tools()` / `@server.call_tool()` decorators in
-# favour of constructor handlers with a `(ctx, params) -> Result` signature.
-# ponytail: one flag, two registration paths; drop the 1.x branch when the floor is mcp>=2.
+# favour of constructor handlers with a `(ctx, params) -> Result` signature. This
+# flag selects the registration path; the 1.x branch can go once the floor is mcp>=2.
 MCP_V2 = not hasattr(Server, "list_tools")
 
 try:

From bad523114aaa126b57a3ab6b97aa4f02ed276eda Mon Sep 17 00:00:00 2001
From: Kent Huang <kent@infuseai.io>
Date: Tue, 11 Aug 2026 16:08:17 +0800
Subject: [PATCH 04/12] fix(smoke): drive the mcp check over http instead of
 stdin

`recce mcp-server` is launched in the background as an HTTP/SSE server on
an MCP port, the way `recce server` is, and every request now goes over
that port from inside check_mcp_server_status: poll /health, open the
GET /sse response stream, POST initialize / notifications/initialized /
tools/list to the session endpoint the stream hands out, then assert the
server identifies itself and advertises tools. The function stops the
server on the way out, matching check_server_status.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Kent Huang <kent@infuseai.io>
---
 integration_tests/dbt/smoke_test.sh | 76 +++++++++++++++++++----------
 1 file changed, 51 insertions(+), 25 deletions(-)

diff --git a/integration_tests/dbt/smoke_test.sh b/integration_tests/dbt/smoke_test.sh
index 596332437..2ce34004a 100755
--- a/integration_tests/dbt/smoke_test.sh
+++ b/integration_tests/dbt/smoke_test.sh
@@ -129,46 +129,72 @@ function check_server_status() {
 }
 
 # Recce MCP Server
-# Stdio, not HTTP: there is no port to poll, so the server is started with the
-# handshake on stdin and its replies land in a file. The startup path is what
-# breaks (tool registration differs between mcp 1.x and 2.0), so the handshake
-# plus a non-empty tool list is the whole check.
-MCP_REQUESTS=$(mktemp)
-MCP_RESPONSES=$(mktemp)
-MCP_STDERR=$(mktemp)
-cat > "$MCP_REQUESTS" << 'EOF'
-{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"smoke","version":"0"}}}
-{"jsonrpc":"2.0","method":"notifications/initialized"}
-{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}
-EOF
-
-# Takes the pid of the backgrounded `recce mcp-server`. The server exits once it
-# reaches the end of the request file, so waiting on it is the readiness signal.
+# The MCP server talks HTTP/SSE: responses arrive on the GET /sse stream, and
+# requests are POSTed to the session endpoint that stream hands out in its first
+# event. Liveness alone is not enough — tool registration differs between mcp
+# 1.x and 2.0, so the handshake plus a non-empty tool list is the real check.
+MCP_PORT=8765
+
 function check_mcp_server_status() {
-    local mcp_pid="$1"
+    local base="http://localhost:$MCP_PORT"
+    local stream stream_pid endpoint responses server_name tool_count
+    stream=$(mktemp)
+
     echo "Waiting for the MCP server to respond..."
-    if ! wait "$mcp_pid"; then
-        echo "Failed to start the MCP server."
-        cat "$MCP_STDERR"
+    if ! timeout 60 bash -c "until curl -sf $base/health > /dev/null; do
+    echo \"MCP server not ready yet...\"
+    sleep 2
+    done"; then
+        echo "Failed to start the MCP server within the time limit."
+        exit 1
+    fi
+
+    # The response stream has to be open before any request is sent.
+    curl -sN "$base/sse" > "$stream" &
+    stream_pid=$!
+    if ! timeout 20 bash -c "until grep -q '^data: /' '$stream'; do sleep 0.5; done"; then
+        echo "The MCP server did not hand out a session endpoint."
+        exit 1
+    fi
+    # SSE lines are CRLF-terminated; a trailing CR makes the POST url malformed.
+    endpoint=$(awk '/^data: \//{sub(/^data: /,""); sub(/\r$/,""); print; exit}' "$stream")
+
+    for request in \
+        '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"smoke","version":"0"}}}' \
+        '{"jsonrpc":"2.0","method":"notifications/initialized"}' \
+        '{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}'; do
+        if ! curl -sf -o /dev/null -X POST "$base$endpoint" -H 'Content-Type: application/json' -d "$request"; then
+            echo "The MCP server rejected a request: $request"
+            exit 1
+        fi
+    done
+
+    if ! timeout 20 bash -c "until grep -q '\"id\":2' '$stream'; do sleep 0.5; done"; then
+        echo "The MCP server did not answer tools/list."
         exit 1
     fi
 
-    local server_name tool_count
-    server_name=$(jq -r 'select(.id == 1) | .result.serverInfo.name' "$MCP_RESPONSES")
-    tool_count=$(jq -r 'select(.id == 2) | .result.tools | length' "$MCP_RESPONSES")
+    responses=$(awk '/^data: \{/{sub(/^data: /,""); print}' "$stream")
+    kill "$stream_pid" 2>/dev/null || true
+    server_name=$(jq -r 'select(.id == 1) | .result.serverInfo.name' <<< "$responses")
+    tool_count=$(jq -r 'select(.id == 2) | .result.tools | length' <<< "$responses")
     assert_string_value "$server_name" "recce"
     if [ "${tool_count:-0}" -lt 1 ]; then
         echo "The MCP server started but advertised no tools."
-        cat "$MCP_STDERR"
         exit 1
     fi
     echo "MCP server is up and advertised $tool_count tools."
+
+    echo "Stopping the MCP server..."
+    kill $(jobs -p) 2>/dev/null || true
+    wait || true
+    echo "MCP server stopped."
 }
 
 if [ "$SMOKE_SERVER" = "mcp-server" ]; then
     echo "Starting the MCP server..."
-    timeout 60 recce mcp-server < "$MCP_REQUESTS" > "$MCP_RESPONSES" 2> "$MCP_STDERR" &
-    check_mcp_server_status $!
+    recce mcp-server --sse --port "$MCP_PORT" &
+    check_mcp_server_status
 else
     echo "Starting the server..."
     recce server &

From c5c3395d7c48632864a54bae4f037bb639e1d900 Mon Sep 17 00:00:00 2001
From: Kent Huang <kent@infuseai.io>
Date: Tue, 11 Aug 2026 16:17:48 +0800
Subject: [PATCH 05/12] test(mcp): cover both handler registration paths
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`_build_server` picks its branch from MCP_V2, so whichever SDK major is
installed leaves the other branch unexecuted — the 2.0 constructor kwargs
and the 1.x decorators cannot both be reached natively. Patch the flag and
assert each path wires the handlers it should.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Kent Huang <kent@infuseai.io>
---
 tests/test_mcp_server.py | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py
index 176e4b536..ecf416202 100644
--- a/tests/test_mcp_server.py
+++ b/tests/test_mcp_server.py
@@ -3467,3 +3467,33 @@ async def test_handler_surfaces_failed_run_as_iserror_and_persists(self, server)
         # The FAILED Run is still persisted for citation, not dropped.
         assert len(self._context.runs) == 1
         assert self._context.runs[0].status == RunStatus.FAILED
+
+
+class TestHandlerRegistration:
+    """Both mcp SDK registration paths, whichever version is installed.
+
+    `MCP_V2` is patched so each branch of `_build_server` is exercised on any
+    install: the constructor kwargs are 2.0-only and the decorators are 1.x-only,
+    so neither path can be reached natively by the other version's SDK.
+    """
+
+    def test_mcp2_registers_handlers_via_constructor(self):
+        with patch("recce.mcp_server.MCP_V2", True), patch("recce.mcp_server.Server") as mock_server:
+            server = RecceMCPServer(MagicMock(spec=RecceContext))
+
+        kwargs = mock_server.call_args.kwargs
+        assert kwargs["on_list_tools"] == server._handle_list_tools
+        assert kwargs["on_call_tool"] == server._handle_call_tool
+        # The decorators no longer exist on 2.0, so they must not be touched.
+        mock_server.return_value.list_tools.assert_not_called()
+        mock_server.return_value.call_tool.assert_not_called()
+
+    def test_mcp1_registers_handlers_via_decorators(self):
+        with patch("recce.mcp_server.MCP_V2", False), patch("recce.mcp_server.Server") as mock_server:
+            server = RecceMCPServer(MagicMock(spec=RecceContext))
+
+        instance = mock_server.return_value
+        # The 1.x SDK consumes the handlers in their native shapes, undecorated.
+        instance.list_tools.return_value.assert_called_once_with(server._list_tools)
+        instance.call_tool.return_value.assert_called_once_with(server._call_tool)
+        assert "on_list_tools" not in mock_server.call_args.kwargs

From c7f7987bc96b84ad2d441291668790efbada2897 Mon Sep 17 00:00:00 2001
From: Kent <iamcxa@gmail.com>
Date: Tue, 11 Aug 2026 18:08:58 +0800
Subject: [PATCH 06/12] fix(mcp): validate tool arguments on mcp 2.0

mcp 1.x validates `tools/call` arguments against the tool's `inputSchema`
inside `Server.call_tool(validate_input=True)` and returns
`Input validation error: ...`. mcp 2.0's low-level server dropped that
entirely -- `jsonschema` survives only in the client, for output schemas --
so the 2.0 adapter forwarded raw arguments straight to the tool.

That fails silently rather than loudly. `impact_analysis` declares
`skip_value_diff` as a boolean; the string `"false"` is truthy, so on 2.0
the value comparison is skipped and the agent gets a result that looks
complete. Missing required fields and array params given a bare string
diverge the same way.

`_handle_call_tool` now validates first, with the same message 1.x emits.
Schemas are cached and refreshed on a miss, mirroring the SDK's own
`_tool_cache`: `set_backend` can change the advertised surface at runtime,
and rebuilding the list per call would forge a `Returning N tools` log line
on every `tools/call`.

Verified on mcp 1.29.0 and 2.0.0 -- 259 passed on both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Kent <iamcxa@gmail.com>
---
 recce/mcp_server.py      | 48 ++++++++++++++++++++++++++++++++-
 tests/test_mcp_server.py | 57 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 104 insertions(+), 1 deletion(-)

diff --git a/recce/mcp_server.py b/recce/mcp_server.py
index 3dc9f824b..c4865afb1 100644
--- a/recce/mcp_server.py
+++ b/recce/mcp_server.py
@@ -16,6 +16,7 @@
 from typing import Any, Dict, List, Optional
 from urllib.parse import quote
 
+import jsonschema
 import requests
 from mcp.server import Server
 from mcp.server.stdio import stdio_server
@@ -48,6 +49,20 @@
 # flag selects the registration path; the 1.x branch can go once the floor is mcp>=2.
 MCP_V2 = not hasattr(Server, "list_tools")
 
+
+def _tool_input_schema(tool: Tool) -> Dict[str, Any]:
+    """Read a tool's JSON schema regardless of SDK field naming.
+
+    `Tool.inputSchema` on mcp 1.x, `Tool.input_schema` on 2.0. Tested for `None`
+    rather than falsiness so an empty schema does not fall through to the attribute
+    the other major does not have.
+    """
+    schema = getattr(tool, "input_schema", None)
+    if schema is None:
+        schema = tool.inputSchema
+    return schema
+
+
 try:
     from sentry_sdk import metrics as sentry_metrics
 except ImportError:  # pragma: no cover
@@ -666,6 +681,7 @@ def __init__(
         self.api_token = api_token
         self._backend_lock = asyncio.Lock()
         self._local_cache_key: Optional[tuple] = None
+        self._tool_schema_cache: Dict[str, Dict[str, Any]] = {}
         self.mcp_logger = MCPLogger(debug=debug, log_file=log_file)
         self.server = self._build_server()
 
@@ -709,6 +725,19 @@ async def _handle_list_tools(self, ctx, params) -> ListToolsResult:
         """`tools/list` in the mcp 2.0 handler signature (also used by tests on 1.x)."""
         return ListToolsResult(tools=await self._list_tools())
 
+    async def _get_tool_input_schema(self, name: str) -> Optional[Dict[str, Any]]:
+        """Look up a tool's JSON schema, refreshing the cache on a miss.
+
+        Mirrors the `_tool_cache` the mcp 1.x SDK keeps: `set_backend` can change the
+        advertised surface at runtime, so a miss means "re-read the list", not "unknown
+        tool". The cache is what keeps validation quiet — `_list_tools` logs its result
+        and writes an MCPLogger entry, so rebuilding it per call would forge a
+        `Returning N tools` line on every `tools/call`.
+        """
+        if name not in self._tool_schema_cache:
+            self._tool_schema_cache = {tool.name: _tool_input_schema(tool) for tool in await self._list_tools()}
+        return self._tool_schema_cache.get(name)
+
     async def _handle_call_tool(self, ctx, params) -> CallToolResult:
         """`tools/call` in the mcp 2.0 handler signature (also used by tests on 1.x).
 
@@ -716,9 +745,26 @@ async def _handle_call_tool(self, ctx, params) -> CallToolResult:
         JSON-RPC protocol error instead, which an agent reads as a transport
         failure rather than a tool failure. Return the tool error explicitly to
         keep the response identical across both versions.
+
+        Argument validation is here for the same reason. 1.x runs it inside
+        `Server.call_tool(validate_input=True)`; 2.0's low-level server dropped it, and
+        an unvalidated argument fails silently rather than loudly — `"false"` is a
+        truthy string, so a skip flag arrives flipped and the work it guards is quietly
+        not done. The message matches what 1.x emits so the contract reads the same.
         """
+        arguments = params.arguments or {}
+        schema = await self._get_tool_input_schema(params.name)
+        if schema is not None:
+            try:
+                jsonschema.validate(instance=arguments, schema=schema)
+            except jsonschema.ValidationError as e:
+                return CallToolResult(
+                    content=[TextContent(type="text", text=f"Input validation error: {e.message}")],
+                    isError=True,
+                )
+
         try:
-            content = await self._call_tool(params.name, params.arguments or {})
+            content = await self._call_tool(params.name, arguments)
         except Exception as e:
             return CallToolResult(content=[TextContent(type="text", text=str(e))], isError=True)
         return CallToolResult(content=content)
diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py
index ecf416202..91725e8f9 100644
--- a/tests/test_mcp_server.py
+++ b/tests/test_mcp_server.py
@@ -3497,3 +3497,60 @@ def test_mcp1_registers_handlers_via_decorators(self):
         instance.list_tools.return_value.assert_called_once_with(server._list_tools)
         instance.call_tool.return_value.assert_called_once_with(server._call_tool)
         assert "on_list_tools" not in mock_server.call_args.kwargs
+
+
+class TestCallToolInputValidation:
+    """`tools/call` argument validation, which only one SDK major performs for us.
+
+    mcp 1.x validates arguments against `inputSchema` inside
+    `Server.call_tool(validate_input=True)`; mcp 2.0's low-level server dropped that
+    entirely. Without an equivalent in the adapter a wrongly-typed argument reaches the
+    tool and is silently coerced — `"false"` is a truthy string, so a skip flag flips on
+    and the comparison it guards is quietly not run.
+    """
+
+    @pytest.mark.asyncio
+    async def test_wrong_type_is_rejected(self, mcp_server):
+        server, _ = mcp_server
+        result = await invoke_call_tool(server, "impact_analysis", {"skip_value_diff": "false"})
+        assert result.isError is True
+        assert "Input validation error" in result.content[0].text
+
+    @pytest.mark.asyncio
+    async def test_missing_required_argument_is_rejected(self, mcp_server):
+        server, _ = mcp_server
+        result = await invoke_call_tool(server, "value_diff", {"model": "customers"})
+        assert result.isError is True
+        assert "Input validation error" in result.content[0].text
+
+    @pytest.mark.asyncio
+    async def test_well_typed_arguments_are_not_blocked(self, mcp_server):
+        """A correctly shaped call must reach the tool, whatever the tool then does."""
+        server, _ = mcp_server
+        result = await invoke_call_tool(server, "impact_analysis", {"skip_value_diff": True})
+        assert "Input validation error" not in result.content[0].text
+
+    @pytest.mark.asyncio
+    async def test_unknown_tool_is_not_reported_as_a_validation_error(self, mcp_server):
+        """Having no schema to validate against is not the same as failing validation."""
+        server, _ = mcp_server
+        result = await invoke_call_tool(server, "nonexistent_tool", {})
+        assert result.isError is True
+        assert "Input validation error" not in result.content[0].text
+
+    @pytest.mark.asyncio
+    async def test_schema_lookup_does_not_re_announce_the_tool_list(self, mcp_server, caplog):
+        """Validation must not make every tool call look like a `tools/list`.
+
+        `_list_tools` logs its result and writes an MCPLogger entry, so rebuilding the
+        list per call would forge a `Returning N tools` line on every `tools/call`.
+        """
+        import logging
+
+        server, _ = mcp_server
+        with caplog.at_level(logging.INFO, logger="recce.mcp_server"):
+            await invoke_call_tool(server, "value_diff", {"model": "customers"})
+            caplog.clear()
+            await invoke_call_tool(server, "value_diff", {"model": "customers"})
+
+        assert "Returning" not in caplog.text

From c0c0302d4b9040a7e07980f930e75a7616737e77 Mon Sep 17 00:00:00 2001
From: Kent <iamcxa@gmail.com>
Date: Tue, 11 Aug 2026 18:09:10 +0800
Subject: [PATCH 07/12] test(mcp): route the e2e protocol tests through the
 compat helpers

`tests/test_mcp_e2e.py` was the one test module left reading `result.isError`
off a real SDK `CallToolResult`. That attribute exists only on mcp 1.x, so
the module failed 5/60 on 2.0 with

    AttributeError: 'CallToolResult' object has no attribute 'isError'

No CI job could see it: the MCP suite runs exactly once, in the tox
`dbtlatest` env, whose `mcp>=1.0.0` resolves to a 1.x release.

Reading `.is_error` instead would just move the breakage to 1.x, so the
normalisation `invoke_call_tool` already did is lifted into a free
`is_error()` -- it also has to take results built by a real `ClientSession`,
not only ones this module builds.

`input_schema()` now tests for `None` rather than falsiness, so an empty
schema cannot fall through to the attribute the other major lacks.

Verified on mcp 1.29.0 and 2.0.0 -- 259 passed on both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Kent <iamcxa@gmail.com>
---
 tests/mcp_compat.py   | 28 ++++++++++++++++++++++------
 tests/test_mcp_e2e.py | 15 ++++++++-------
 2 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/tests/mcp_compat.py b/tests/mcp_compat.py
index 9bcaf3c5e..8a3a8247f 100644
--- a/tests/mcp_compat.py
+++ b/tests/mcp_compat.py
@@ -19,13 +19,22 @@ class ToolResult:
     isError: bool
 
 
+def is_error(result) -> bool:
+    """Read the error flag off a ``CallToolResult`` regardless of SDK field naming.
+
+    Also takes results built by a real ``ClientSession``, which is why this is a free
+    function rather than something only ``invoke_call_tool`` uses.
+    """
+    value = getattr(result, "is_error", None)
+    if value is None:
+        value = getattr(result, "isError", None)
+    return bool(value)
+
+
 async def invoke_call_tool(server, name: str, arguments: Optional[Dict[str, Any]] = None) -> ToolResult:
     """Call a tool and normalise the result, including the error case."""
     result = await server._handle_call_tool(None, CallToolRequestParams(name=name, arguments=arguments or {}))
-    is_error = getattr(result, "is_error", None)
-    if is_error is None:
-        is_error = getattr(result, "isError", None)
-    return ToolResult(content=list(result.content), isError=bool(is_error))
+    return ToolResult(content=list(result.content), isError=is_error(result))
 
 
 async def invoke_list_tools(server) -> List[Tool]:
@@ -34,5 +43,12 @@ async def invoke_list_tools(server) -> List[Tool]:
 
 
 def input_schema(tool: Tool) -> Dict[str, Any]:
-    """Read a tool's JSON schema regardless of SDK field naming."""
-    return getattr(tool, "input_schema", None) or tool.inputSchema
+    """Read a tool's JSON schema regardless of SDK field naming.
+
+    Tested for ``None`` rather than falsiness: an empty schema would otherwise fall
+    through to the attribute the other major does not have.
+    """
+    schema = getattr(tool, "input_schema", None)
+    if schema is None:
+        schema = tool.inputSchema
+    return schema
diff --git a/tests/test_mcp_e2e.py b/tests/test_mcp_e2e.py
index ddd6195ce..7c3675097 100644
--- a/tests/test_mcp_e2e.py
+++ b/tests/test_mcp_e2e.py
@@ -23,6 +23,7 @@
 from recce.core import set_default_context  # noqa: E402
 from recce.mcp_server import RecceMCPServer  # noqa: E402
 from tests.adapter.dbt_adapter.dbt_test_helper import DbtTestHelper  # noqa: E402
+from tests.mcp_compat import is_error  # noqa: E402
 
 
 @asynccontextmanager
@@ -1262,7 +1263,7 @@ async def test_call_set_backend_via_protocol_flips_unconfigured_to_cloud(self):
             async with create_mcp_client(server) as client:
                 # Before swap: normal tools blocked.
                 blocked = await client.call_tool("lineage_diff", {})
-                assert blocked.isError
+                assert is_error(blocked)
                 assert "No backend configured" in blocked.content[0].text
 
                 # Flip via protocol-level set_backend.
@@ -1270,7 +1271,7 @@ async def test_call_set_backend_via_protocol_flips_unconfigured_to_cloud(self):
                     "set_backend",
                     {"mode": "cloud", "session_id": "sess-123"},
                 )
-                assert not swap.isError
+                assert not is_error(swap)
                 swap_data = json.loads(swap.content[0].text)
                 assert swap_data == {
                     "mode": "cloud",
@@ -1280,7 +1281,7 @@ async def test_call_set_backend_via_protocol_flips_unconfigured_to_cloud(self):
 
                 # After swap: get_server_info delegates to the cloud backend.
                 info = await client.call_tool("get_server_info", {})
-                assert not info.isError
+                assert not is_error(info)
                 info_data = json.loads(info.content[0].text)
                 assert info_data["mode"] == "cloud"
                 assert info_data["session_id"] == "sess-123"
@@ -1290,7 +1291,7 @@ async def test_call_row_count_diff_via_protocol(self, mcp_e2e_with_data):
         server, _ = mcp_e2e_with_data
         async with create_mcp_client(server) as client:
             result = await client.call_tool("row_count_diff", {"node_names": ["customers"]})
-            assert not result.isError
+            assert not is_error(result)
             data = json.loads(result.content[0].text)
             assert data["customers"]["base"] == 2
             assert data["customers"]["curr"] == 3
@@ -1300,7 +1301,7 @@ async def test_call_lineage_diff_via_protocol(self, mcp_e2e_with_data):
         server, _ = mcp_e2e_with_data
         async with create_mcp_client(server) as client:
             result = await client.call_tool("lineage_diff", {})
-            assert not result.isError
+            assert not is_error(result)
             data = json.loads(result.content[0].text)
             assert "nodes" in data
             assert "edges" in data
@@ -1314,7 +1315,7 @@ async def test_call_query_via_protocol(self, mcp_e2e_with_data):
                 "query",
                 {"sql_template": f"SELECT count(*) as cnt FROM {schema}.customers"},
             )
-            assert not result.isError
+            assert not is_error(result)
             data = json.loads(result.content[0].text)
             assert data["data"][0][0] == 3
 
@@ -1323,7 +1324,7 @@ async def test_call_list_checks_via_protocol(self, mcp_e2e_with_data):
         server, _ = mcp_e2e_with_data
         async with create_mcp_client(server) as client:
             result = await client.call_tool("list_checks", {})
-            assert not result.isError
+            assert not is_error(result)
             data = json.loads(result.content[0].text)
             assert data["total"] == 0
             assert data["checks"] == []

From 122ffde479c2c6764cd86ad3524a018503554cb1 Mon Sep 17 00:00:00 2001
From: Kent <iamcxa@gmail.com>
Date: Tue, 11 Aug 2026 18:11:50 +0800
Subject: [PATCH 08/12] test(smoke): call a tool, and clean up when the check
 fails
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The check stopped at `initialize` + `tools/list`, which is the part the two
SDK majors agree on. What actually differs is the error contract, and
nothing exercised it:

- id 3 calls `get_server_info` and asserts the call succeeds.
- id 4 sends `{"select":123}` to `lineage_diff`, whose schema says string,
  and asserts `isError` plus `Input validation error`. 1.x rejects that in
  the SDK; on 2.0 the adapter has to.

Verified against a real `recce mcp-server --sse` on mcp 1.29.0 and 2.0.0
(20 tools, both assertions pass), and with the argument made valid, where
the id 4 assertion correctly fails — so it is not vacuous.

Two smaller things in the same check:

- `tool_count -lt 1` passes with 19 of 20 tools silently unregistered,
  which is the exact failure this job exists to catch. Assert `lineage_diff`
  is named.
- Every `exit 1` fired with the server, and later the SSE reader, still
  running and holding the CI step's stdout. An EXIT trap covers both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Kent <iamcxa@gmail.com>
---
 integration_tests/dbt/smoke_test.sh | 38 ++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/integration_tests/dbt/smoke_test.sh b/integration_tests/dbt/smoke_test.sh
index 2ce34004a..a2855e914 100755
--- a/integration_tests/dbt/smoke_test.sh
+++ b/integration_tests/dbt/smoke_test.sh
@@ -9,7 +9,8 @@ pwd
 SMOKE_SERVER="${SMOKE_SERVER:-server}"
 # Only used when SMOKE_SERVER=mcp-server. Major.minor, e.g. "1.29" or "2.0":
 # the mcp SDK majors register tool handlers differently, so the version under
-# test has to be explicit, but `~=` still picks up the latest patch release.
+# test has to be explicit. `~=` keeps it a floor, not a pin — `~=1.29` is
+# `>=1.29,<2.0`, so later patch *and* minor releases are picked up.
 SMOKE_MCP_VERSION="${SMOKE_MCP_VERSION:-2.0}"
 
 case "$SMOKE_SERVER" in
@@ -159,18 +160,25 @@ function check_mcp_server_status() {
     # SSE lines are CRLF-terminated; a trailing CR makes the POST url malformed.
     endpoint=$(awk '/^data: \//{sub(/^data: /,""); sub(/\r$/,""); print; exit}' "$stream")
 
+    # ids 3 and 4 are the error contract, which is where the two SDK majors actually
+    # diverge: 1.x turns a raised exception and a schema violation into isError, 2.0
+    # would turn the former into a transport error and skip the latter entirely.
+    # `tools/list` alone cannot see either, so call a tool both ways.
     for request in \
         '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"smoke","version":"0"}}}' \
         '{"jsonrpc":"2.0","method":"notifications/initialized"}' \
-        '{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}'; do
+        '{"jsonrpc":"2.0","id":2,"method":"tools/list","params":{}}' \
+        '{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"get_server_info","arguments":{}}}' \
+        '{"jsonrpc":"2.0","id":4,"method":"tools/call","params":{"name":"lineage_diff","arguments":{"select":123}}}'; do
         if ! curl -sf -o /dev/null -X POST "$base$endpoint" -H 'Content-Type: application/json' -d "$request"; then
             echo "The MCP server rejected a request: $request"
             exit 1
         fi
     done
 
-    if ! timeout 20 bash -c "until grep -q '\"id\":2' '$stream'; do sleep 0.5; done"; then
-        echo "The MCP server did not answer tools/list."
+    # Requests are answered in order on one session, so id 4 arriving means all of them have.
+    if ! timeout 20 bash -c "until grep -q '\"id\":4' '$stream'; do sleep 0.5; done"; then
+        echo "The MCP server did not answer every request."
         exit 1
     fi
 
@@ -179,12 +187,30 @@ function check_mcp_server_status() {
     server_name=$(jq -r 'select(.id == 1) | .result.serverInfo.name' <<< "$responses")
     tool_count=$(jq -r 'select(.id == 2) | .result.tools | length' <<< "$responses")
     assert_string_value "$server_name" "recce"
+    # A count alone is a weak gate — server mode advertises 20 tools, so `>= 1` still
+    # passes with 19 of them silently unregistered. Name one that must be there.
+    if ! jq -e 'select(.id == 2) | [.result.tools[].name] | index("lineage_diff")' > /dev/null <<< "$responses"; then
+        echo "The MCP server did not advertise lineage_diff."
+        exit 1
+    fi
     if [ "${tool_count:-0}" -lt 1 ]; then
         echo "The MCP server started but advertised no tools."
         exit 1
     fi
     echo "MCP server is up and advertised $tool_count tools."
 
+    if ! jq -e 'select(.id == 3) | .result.isError != true' > /dev/null <<< "$responses"; then
+        echo "The MCP server failed a get_server_info tool call."
+        exit 1
+    fi
+    # 123 violates lineage_diff's `select: string` schema. mcp 1.x rejects this in the
+    # SDK; on 2.0 the adapter has to, or the argument reaches the tool coerced.
+    if ! jq -e 'select(.id == 4) | .result.isError == true and (.result.content[0].text | test("Input validation error"))' > /dev/null <<< "$responses"; then
+        echo "The MCP server did not reject a schema-invalid tool argument."
+        exit 1
+    fi
+    echo "MCP server tool calls behave correctly on success and on invalid input."
+
     echo "Stopping the MCP server..."
     kill $(jobs -p) 2>/dev/null || true
     wait || true
@@ -193,6 +219,10 @@ function check_mcp_server_status() {
 
 if [ "$SMOKE_SERVER" = "mcp-server" ]; then
     echo "Starting the MCP server..."
+    # Every `exit 1` inside the check fires with the server already backgrounded, and
+    # after the SSE reader starts, with that too. Both inherit this step's stdout, so
+    # without a trap a failed smoke test keeps the CI step open with nothing left to say.
+    trap 'kill $(jobs -p) 2>/dev/null || true' EXIT
     recce mcp-server --sse --port "$MCP_PORT" &
     check_mcp_server_status
 else

From 74e3313672c0c7153b42897fd2112accb2750514 Mon Sep 17 00:00:00 2001
From: Kent <iamcxa@gmail.com>
Date: Tue, 11 Aug 2026 18:13:20 +0800
Subject: [PATCH 09/12] chore(deps): sync uv.lock with the widened mcp window

The lock still recorded the extra as `~=1.23`, so it disagreed with
pyproject the moment the pin moved and `uv lock --check` failed. Nothing in
CI uses `--locked` or `--frozen`, which is why it stayed green: every
`uv sync` silently re-resolved and rewrote the lock in the working tree.

Regenerating only rewrites the recorded specifier (plus two marker
refinements from a newer resolver); the resolved mcp version is unchanged,
since 1.28.1 still satisfies the new window. Moving that is a separate
decision.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Kent <iamcxa@gmail.com>
---
 uv.lock | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/uv.lock b/uv.lock
index 1e17a2682..b1fcae8f2 100644
--- a/uv.lock
+++ b/uv.lock
@@ -807,7 +807,7 @@ name = "exceptiongroup"
 version = "1.3.0"
 source = { registry = "https://pypi.org/simple" }
 dependencies = [
-    { name = "typing-extensions" },
+    { name = "typing-extensions", marker = "python_full_version < '3.11'" },
 ]
 sdist = { url = "https://files.pythonhosted.org/packages/0b/9f/a65090624ecf468cdca03533906e7c69ed7588582240cfe7cc9e770b50eb/exceptiongroup-1.3.0.tar.gz", hash = "sha256:b241f5885f560bc56a59ee63ca4c6a8bfa46ae4ad651af316d4e81817bb9fd88", size = 29749, upload-time = "2025-05-10T17:42:51.123Z" }
 wheels = [
@@ -2558,7 +2558,7 @@ requires-dist = [
     { name = "isort", marker = "extra == 'dev'", specifier = ">=6.0.1" },
     { name = "itsdangerous" },
     { name = "jinja2" },
-    { name = "mcp", marker = "extra == 'mcp'", specifier = "~=1.23" },
+    { name = "mcp", marker = "extra == 'mcp'", specifier = ">=1.23,<3" },
     { name = "openpyxl", specifier = ">=3.1.0" },
     { name = "packaging" },
     { name = "pandas", marker = "extra == 'dev'" },
@@ -2893,8 +2893,8 @@ name = "secretstorage"
 version = "3.4.1"
 source = { registry = "https://pypi.org/simple" }
 dependencies = [
-    { name = "cryptography" },
-    { name = "jeepney" },
+    { name = "cryptography", marker = "python_full_version < '3.12' or sys_platform != 'win32'" },
+    { name = "jeepney", marker = "python_full_version < '3.12' or sys_platform != 'win32'" },
 ]
 sdist = { url = "https://files.pythonhosted.org/packages/32/8a/ed6747b1cc723c81f526d4c12c1b1d43d07190e1e8258dbf934392fc850e/secretstorage-3.4.1.tar.gz", hash = "sha256:a799acf5be9fb93db609ebaa4ab6e8f1f3ed5ae640e0fa732bfea59e9c3b50e8", size = 19871, upload-time = "2025-11-11T11:30:23.798Z" }
 wheels = [

From 23258ffeb36f06bc46a1cb938c1cee80786dd863 Mon Sep 17 00:00:00 2001
From: Kent <iamcxa@gmail.com>
Date: Tue, 11 Aug 2026 18:13:20 +0800
Subject: [PATCH 10/12] ci(mcp): run the MCP tests against both SDK majors

The MCP suite ran exactly once in CI. `Test Python Versions` uses tox envs
with no mcp dep at all, so all three MCP modules were skipped at collection
(`collected 1334 items / 3 skipped`); only `dbtlatest` carried mcp, pinned
`mcp>=1.0.0` -- under the project's own floor, unbounded above -- which
resolved to 1.27.1. Nothing ever ran pytest against 2.x, which is how a
module that fails 5/60 on mcp 2.0 shipped green.

`mcp-smoke-test` already installs both majors, so it is the cheap place to
close this: the smoke step proves the server boots, and the new step proves
the handlers behave. tox now takes mcp through the extra, so that env
inherits the window pyproject declares instead of restating it wrongly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Kent <iamcxa@gmail.com>
---
 .github/workflows/integration-tests.yaml | 10 ++++++++++
 tox.ini                                  |  6 ++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/integration-tests.yaml b/.github/workflows/integration-tests.yaml
index dabfdd492..bcbdab19d 100644
--- a/.github/workflows/integration-tests.yaml
+++ b/.github/workflows/integration-tests.yaml
@@ -82,3 +82,13 @@ jobs:
         run: |
           source .venv/bin/activate
           ./integration_tests/dbt/smoke_test.sh
+
+      # The step above is the only place either major gets installed: the tox envs
+      # that run pytest resolve one mcp release, so on their own they leave the other
+      # major's compat path untested. The smoke test proves the server boots; these
+      # prove the handlers behave.
+      - name: Run MCP tests
+        run: |
+          source .venv/bin/activate
+          uv pip install pytest pytest-asyncio pandas duckdb
+          python -m pytest tests/test_mcp_server.py tests/test_mcp_cloud_backend.py tests/test_mcp_e2e.py -q
diff --git a/tox.ini b/tox.ini
index 5701f6b96..2fba34d60 100644
--- a/tox.ini
+++ b/tox.ini
@@ -17,8 +17,10 @@ deps =
     dbt1.8: dbt-duckdb==1.8.*
     dbt1.9: dbt-duckdb==1.9.*
     dbtlatest: dbt-duckdb
-    # only test mcp for the latest dbt version
-    dbtlatest: mcp>=1.0.0
+# only test mcp for the latest dbt version, and through the extra so the env
+# inherits the window pyproject declares instead of drifting from it.
+extras =
+    dbtlatest: mcp
 commands =
     pytest --cov --cov-append --cov-report=xml {posargs:./tests}
 

From 6bec82b40a9731e49530130c1bc8b38d372d393b Mon Sep 17 00:00:00 2001
From: Kent <iamcxa@gmail.com>
Date: Tue, 11 Aug 2026 18:19:57 +0800
Subject: [PATCH 11/12] docs(mcp): record what differs between the two SDK
 majors

The skill still read `Handler must raise for SDK to set isError=True` as a
flat rule. That is true on mcp 1.x and false on 2.0, where a raised
exception becomes a protocol error and the adapter has to convert it -- so
following the rule as written on 2.0 changes what an agent sees.

Also records the two things this PR made load-bearing and neither obvious
nor greppable: `inputSchema` is enforced (2.0 does no validation of its
own, so `type` and `required` stop being documentation), and which CI job
covers which mcp version, since the tox envs resolve only one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Kent <iamcxa@gmail.com>
---
 .claude/skills/recce-mcp-dev/SKILL.md | 31 ++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/.claude/skills/recce-mcp-dev/SKILL.md b/.claude/skills/recce-mcp-dev/SKILL.md
index a38b2f9d5..9f99f610a 100644
--- a/.claude/skills/recce-mcp-dev/SKILL.md
+++ b/.claude/skills/recce-mcp-dev/SKILL.md
@@ -11,11 +11,32 @@ description: Use when modifying recce/mcp_server.py, MCP tool handlers, error cl
 
 Entry point `run_mcp_server()` pops `single_env` before passing kwargs to `load_context()`.
 
+**Two SDK majors are supported** (`mcp>=1.23,<3`), and `_build_server()` picks the
+registration path from the `MCP_V2` flag: decorators on 1.x, `on_list_tools` /
+`on_call_tool` constructor kwargs on 2.0. The handler bodies keep their 1.x shapes
+(`List[Tool]` / `List[TextContent]`, errors raised); `_handle_list_tools` /
+`_handle_call_tool` adapt them for 2.0. Tests drive the `_handle_*` adapters on both
+majors via `tests/mcp_compat.py` — never `server.server.request_handlers[...]`, which
+does not exist on 2.0.
+
 ## Key Patterns
 
 **Error classification** — Shared indicator lists defined in `recce/tasks/rowcount.py`. Priority order (`PERMISSION_DENIED` > `TABLE_NOT_FOUND` > `SYNTAX_ERROR`) enforced by `_classify_db_error()` in `mcp_server.py` and `_query_row_count()` in `rowcount.py`. Classified → `logger.warning()` + `sentry_metrics.count()` (when sentry_sdk available). Unclassified → `logger.error()` + traceback.
 
-**MCP SDK quirk** — Handler must **raise** for SDK to set `isError=True`.
+**MCP SDK quirk — version-dependent, do not generalise.** On mcp 1.x the handler must
+**raise** for the SDK to set `isError=True`. On 2.0 a raised exception becomes a JSON-RPC
+*protocol* error instead, which an agent reads as a transport failure rather than a tool
+failure — `_handle_call_tool` catches and returns `CallToolResult(isError=True)` so the
+response is the same on both. Inner handlers still raise; only the adapter converts.
+
+**Input validation is not free on 2.0.** mcp 1.x validated `tools/call` arguments against
+the tool's `inputSchema` inside `Server.call_tool(validate_input=True)`; 2.0's low-level
+server dropped that entirely (`jsonschema` survives only client-side, for output schemas).
+`_handle_call_tool` validates explicitly, with 1.x's `Input validation error: ...` wording.
+This matters because the failure is silent: `"false"` is a truthy string, so a boolean skip
+flag arrives flipped and the work it guards is quietly not done. Any new tool gets this for
+free — but only as far as its declared schema goes, so `"type"` and `"required"` in
+`inputSchema` are load-bearing, not documentation.
 
 **Single-env** — `_maybe_add_single_env_warning()` adds `_warning` to diff results. Descriptions get conditional note.
 
@@ -78,6 +99,14 @@ Origin: PR #1342 review (DRC-3307).
 | Integration | `tests/test_mcp_e2e.py` | `DbtTestHelper` + DuckDB (fixed data) | CI (`pytest`) | MCP protocol works end-to-end via anyio memory streams |
 | Smoke (E2E) | `/recce-mcp-e2e` skill | User's real dbt project + real database | Manual | The 8 tools that harness covers return valid results against real data |
 
+**Which mcp version each layer runs against is itself a coverage question.** The tox envs
+resolve one mcp release, so on their own they leave the other major untested — that is how
+a module failing 5/60 on mcp 2.0 once shipped CI-green. The `mcp-smoke-test` job in
+`.github/workflows/integration-tests.yaml` installs each major in turn and runs both the
+shell smoke check and the three MCP pytest modules. Anything that touches the `MCP_V2`
+branches, the `_handle_*` adapters, or `tests/mcp_compat.py` has to be checked there, not
+only in the default pytest run.
+
 **Tool count — mode-dependent, verify per mode.** `list_tools` registers **at
 most 20** tools, and how many it actually returns depends on the server mode:
 

From 0350e2a7647a6e21c7e81a030e4302e467f0064f Mon Sep 17 00:00:00 2001
From: Kent Huang <kent@infuseai.io>
Date: Tue, 11 Aug 2026 22:06:12 +0800
Subject: [PATCH 12/12] fix(mcp): build the schema cache once, and assert id 3
 has a result
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two review findings on the validation work:

- `_get_tool_input_schema` refreshed on a miss, and an unknown tool name
  misses every time — so every bad call rebuilt all 20 tool definitions
  and forged a `Returning N tools` line plus an MCPLogger `list_tools`
  entry. Measured 3 rebuilds for 3 unknown calls. Build once instead and
  have `_tool_set_backend` clear the cache, since it is the only thing
  that moves the advertised surface after `__init__`. The existing test
  passed because it only called a name already in the cache; it now calls
  an unknown one twice, and fails against the old code.

- The smoke test's id-3 assertion read `.result.isError != true`, which is
  true when there is no `result` at all — a JSON-RPC error response, i.e.
  exactly the escaped-exception regression that call exists to catch.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Kent Huang <kent@infuseai.io>
---
 integration_tests/dbt/smoke_test.sh |  5 ++++-
 recce/mcp_server.py                 | 18 +++++++++++-------
 tests/test_mcp_server.py            | 18 ++++++++++++++++++
 3 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/integration_tests/dbt/smoke_test.sh b/integration_tests/dbt/smoke_test.sh
index a2855e914..c87986c77 100755
--- a/integration_tests/dbt/smoke_test.sh
+++ b/integration_tests/dbt/smoke_test.sh
@@ -199,7 +199,10 @@ function check_mcp_server_status() {
     fi
     echo "MCP server is up and advertised $tool_count tools."
 
-    if ! jq -e 'select(.id == 3) | .result.isError != true' > /dev/null <<< "$responses"; then
+    # `.result != null` first: on a JSON-RPC error response there is no `result` at all,
+    # and `null.isError != true` is true — which is exactly the escaped-exception
+    # regression this call is here to catch.
+    if ! jq -e 'select(.id == 3) | .result != null and .result.isError != true' > /dev/null <<< "$responses"; then
         echo "The MCP server failed a get_server_info tool call."
         exit 1
     fi
diff --git a/recce/mcp_server.py b/recce/mcp_server.py
index c4865afb1..690a9786a 100644
--- a/recce/mcp_server.py
+++ b/recce/mcp_server.py
@@ -726,15 +726,15 @@ async def _handle_list_tools(self, ctx, params) -> ListToolsResult:
         return ListToolsResult(tools=await self._list_tools())
 
     async def _get_tool_input_schema(self, name: str) -> Optional[Dict[str, Any]]:
-        """Look up a tool's JSON schema, refreshing the cache on a miss.
+        """Look up a tool's JSON schema, building the cache on first use.
 
-        Mirrors the `_tool_cache` the mcp 1.x SDK keeps: `set_backend` can change the
-        advertised surface at runtime, so a miss means "re-read the list", not "unknown
-        tool". The cache is what keeps validation quiet — `_list_tools` logs its result
-        and writes an MCPLogger entry, so rebuilding it per call would forge a
-        `Returning N tools` line on every `tools/call`.
+        A miss must not mean "re-read the list": an unknown name misses every time, so
+        that would rebuild on every bad call and forge a `Returning N tools` line each
+        time — `_list_tools` logs its result and writes an MCPLogger entry. `set_backend`
+        is the only thing that moves the advertised surface after `__init__`, and it
+        clears this cache itself, so building once is enough.
         """
-        if name not in self._tool_schema_cache:
+        if not self._tool_schema_cache:
             self._tool_schema_cache = {tool.name: _tool_input_schema(tool) for tool in await self._list_tools()}
         return self._tool_schema_cache.get(name)
 
@@ -2563,6 +2563,10 @@ async def _tool_set_backend(self, arguments: Dict[str, Any]) -> Dict[str, Any]:
             raise ValueError(f"Invalid mode '{mode}'. Use 'local' or 'cloud'.")
 
         async with self._backend_lock:
+            # Both branches below change what _list_tools advertises (backend, context,
+            # single_env), so the schemas validation reads have to be re-derived.
+            self._tool_schema_cache = {}
+
             if mode == "cloud":
                 session_id = arguments.get("session_id")
                 if not session_id:
diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py
index 91725e8f9..1b0ac93b0 100644
--- a/tests/test_mcp_server.py
+++ b/tests/test_mcp_server.py
@@ -3544,6 +3544,9 @@ async def test_schema_lookup_does_not_re_announce_the_tool_list(self, mcp_server
 
         `_list_tools` logs its result and writes an MCPLogger entry, so rebuilding the
         list per call would forge a `Returning N tools` line on every `tools/call`.
+        An unknown name is the case that matters: it misses the cache every time, so
+        "refresh on a miss" would re-announce the list on every bad call — and a
+        hallucinated tool name is the normal way an agent produces one.
         """
         import logging
 
@@ -3552,5 +3555,20 @@ async def test_schema_lookup_does_not_re_announce_the_tool_list(self, mcp_server
             await invoke_call_tool(server, "value_diff", {"model": "customers"})
             caplog.clear()
             await invoke_call_tool(server, "value_diff", {"model": "customers"})
+            await invoke_call_tool(server, "nonexistent_tool", {})
+            await invoke_call_tool(server, "nonexistent_tool", {})
 
         assert "Returning" not in caplog.text
+
+    @pytest.mark.asyncio
+    async def test_set_backend_invalidates_the_schema_cache(self, mcp_server):
+        """The cache is built once, so the only thing that moves the advertised surface
+        has to clear it — otherwise validation keeps checking against the old schemas."""
+        server, _ = mcp_server
+        await invoke_call_tool(server, "value_diff", {"model": "customers"})
+        assert server._tool_schema_cache
+
+        with patch("recce.mcp_server.CloudBackend.create", return_value=AsyncMock()):
+            await server._tool_set_backend({"mode": "cloud", "session_id": "sess-1", "api_token": "tok"})
+
+        assert server._tool_schema_cache == {}