fix(mcp): keep anticipated tool failures readable under mcp 2.1

The mcp 2.1 SDK narrowed which exceptions reach the client: only a
ToolError keeps its message, and everything else is treated as a crash
and reported as a bare "Error executing tool <name>". The server raised
ValueError/RuntimeError for failures it fully anticipated, so all of
them were masked -- an agent asking for a mistyped REQ_TYPO got
"Error executing tool get_requirement" and no way to see the typo.

Raise ToolError at the eight anticipated-failure sites instead, which
also drops them from ERROR-with-traceback to INFO in the server log.

Reverting only the source change fails eight tests, but main caught
this with one: the others asserted is_error without asserting that the
reason survives. They now assert the message too, so a future SDK bump
cannot re-mask them silently.

Refs reqstool/.github#111

Signed-off-by: Jimisola Laursen <jimisola@jimisola.com>

Author: jimisola

src/reqstool/mcp/server.py

@@ -6,6 +6,7 @@
 
 from reqstool_python_decorators.decorators.decorators import Requirements
 
+from reqstool.common.exceptions import SnapshotReloadError
 from reqstool.common.project_session import ProjectSession
 from reqstool.common.enrichment.enricher import BUILT_IN_PRESETS, enrich_text
 from reqstool.common.queries.details import (
@@ -32,6 +33,11 @@ def start_server(  # noqa: C901
 ) -> None:
     try:
         from mcp.server.mcpserver import MCPServer
+
+        # SDK 2.1: only a ToolError's message reaches the model. Any other exception is
+        # treated as a crash and reported as a bare "Error executing tool <name>", so
+        # every anticipated failure below raises ToolError to keep its text.
+        from mcp.server.mcpserver.exceptions import ToolError
     except ImportError as exc:
         raise ImportError("MCP server requires extra dependencies: pip install 'mcp>=2.0'") from exc
 
@@ -51,10 +57,13 @@ def _repo() -> RequirementsRepository:
         Every tool must resolve the repository through this. Binding it once at startup is
         what let long-lived servers serve a snapshot from before the last build (#437).
         """
-        session.ensure_fresh()
+        try:
+            session.ensure_fresh()
+        except SnapshotReloadError as exc:
+            raise ToolError(str(exc)) from exc
         repo = session.repo
         if repo is None:
-            raise RuntimeError(f"reqstool project is not loaded: {session.error}")
+            raise ToolError(f"reqstool project is not loaded: {session.error}")
         return repo
 
     @Requirements("MCP_0008")
@@ -87,7 +96,7 @@ async def get_requirement(id: str) -> dict:
         """Get full details for a requirement by ID (e.g. REQ_010)."""
         result = get_requirement_details(id, _repo(), session.urn_source_paths)
         if result is None:
-            raise ValueError(f"Requirement {id!r} not found")
+            raise ToolError(f"Requirement {id!r} not found")
         return result
 
     @mcp.tool()
@@ -109,7 +118,7 @@ async def get_svc(id: str) -> dict:
         """Get full details for an SVC by ID (e.g. SVC_010)."""
         result = get_svc_details(id, _repo(), session.urn_source_paths)
         if result is None:
-            raise ValueError(f"SVC {id!r} not found")
+            raise ToolError(f"SVC {id!r} not found")
         return result
 
     @mcp.tool()
@@ -122,7 +131,7 @@ async def get_mvr(id: str) -> dict:
         """Get full details for an MVR by ID."""
         result = get_mvr_details(id, _repo(), session.urn_source_paths)
         if result is None:
-            raise ValueError(f"MVR {id!r} not found")
+            raise ToolError(f"MVR {id!r} not found")
         return result
 
     @mcp.tool()
@@ -144,7 +153,7 @@ async def refresh() -> dict:
         unconditionally — after a build, for instance — or to confirm what is being served."""
         session.build()
         if not session.ready:
-            raise RuntimeError(f"Failed to reload reqstool project: {session.error}")
+            raise ToolError(f"Failed to reload reqstool project: {session.error}")
         return _snapshot_info()
 
     @mcp.tool()
@@ -154,7 +163,7 @@ async def get_requirement_status(id: str, include_post_build: bool = False) -> d
         `status --with-post-tests` (scopes to post-build-phase SVCs too)."""
         result = _get_requirement_status(id, _repo(), include_post_build=include_post_build)
         if result is None:
-            raise ValueError(f"Requirement {id!r} not found")
+            raise ToolError(f"Requirement {id!r} not found")
         return result
 
     @mcp.tool()
@@ -184,7 +193,7 @@ async def get_urn_details(urn: str) -> dict:
         """Get details for a URN: variant, title, location, file paths, and entity counts."""
         result = _get_urn_details(urn, _repo(), session.urn_source_paths)
         if result is None:
-            raise ValueError(f"URN {urn!r} not found")
+            raise ToolError(f"URN {urn!r} not found")
         return result
 
     @mcp.tool()
@@ -198,7 +207,7 @@ async def enrich_document(content: str, preset: str) -> str:
                  openspec:proposal, openspec:tasks
         """
         if preset not in BUILT_IN_PRESETS:
-            raise ValueError(f"Unknown preset {preset!r}. Valid: {sorted(BUILT_IN_PRESETS)}")
+            raise ToolError(f"Unknown preset {preset!r}. Valid: {sorted(BUILT_IN_PRESETS)}")
         config = BUILT_IN_PRESETS[preset]
         repo = _repo()
         return enrich_text(content, repo.get_all_requirements(), repo.get_all_svcs(), repo.get_all_mvrs(), config)

tests/integration/reqstool/mcp/test_mcp_integration.py

@@ -77,6 +77,9 @@ async def test_get_requirement_known(mcp_session):
 async def test_get_requirement_not_found(mcp_session):
     result = await mcp_session.call_tool("get_requirement", {"id": "REQ_NONEXISTENT"})
     assert result.is_error
+    # The reason has to reach the model, not just the failure: an SDK that masks it
+    # leaves a bare "Error executing tool get_requirement" and no way to spot a typo.
+    assert "REQ_NONEXISTENT" in str(result.content)
 
 
 # ---------------------------------------------------------------------------
@@ -114,6 +117,7 @@ async def test_get_svc_known(mcp_session):
 async def test_get_svc_not_found(mcp_session):
     result = await mcp_session.call_tool("get_svc", {"id": "SVC_NONEXISTENT"})
     assert result.is_error
+    assert "SVC_NONEXISTENT" in str(result.content)
 
 
 # ---------------------------------------------------------------------------
@@ -133,6 +137,7 @@ async def test_list_mvrs(mcp_session):
 async def test_get_mvr_not_found(mcp_session):
     result = await mcp_session.call_tool("get_mvr", {"id": "MVR_NONEXISTENT"})
     assert result.is_error
+    assert "MVR_NONEXISTENT" in str(result.content)
 
 
 # ---------------------------------------------------------------------------
@@ -167,6 +172,7 @@ async def test_get_requirement_status(mcp_session):
 async def test_get_requirement_status_not_found(mcp_session):
     result = await mcp_session.call_tool("get_requirement_status", {"id": "REQ_NONEXISTENT"})
     assert result.is_error
+    assert "REQ_NONEXISTENT" in str(result.content)
 
 
 @pytest.mark.parametrize("include_post_build", [False, True])
@@ -176,6 +182,7 @@ async def test_get_requirement_status_not_found_with_include_post_build(mcp_sess
         "get_requirement_status", {"id": "REQ_NONEXISTENT", "include_post_build": include_post_build}
     )
     assert result.is_error
+    assert "REQ_NONEXISTENT" in str(result.content)
 
 
 async def test_get_requirement_status_missing_automated_test_not_met(mcp_session):

tests/unit/reqstool/mcp/test_server_freshness.py

@@ -12,9 +12,9 @@
 
 import mcp.server.mcpserver
 import pytest
+from mcp.server.mcpserver.exceptions import ToolError
 from reqstool_python_decorators.decorators.decorators import SVCs
 
-from reqstool.common.exceptions import SnapshotReloadError
 from reqstool.locations.local_location import LocalLocation
 from reqstool.mcp import server as mcp_server
 
@@ -111,9 +111,11 @@ def test_a_tool_errors_when_the_changed_project_cannot_be_reloaded(project_copy)
     async def scenario(tools):
         (project_copy / "requirements.yml").write_text(": this is not: [ valid yaml")
 
-        with pytest.raises(SnapshotReloadError, match="sources changed but reloading them failed"):
+        # ToolError, not the underlying SnapshotReloadError: only a ToolError's message
+        # survives the SDK, so asserting the type is what proves the reason reaches the model.
+        with pytest.raises(ToolError, match="sources changed but reloading them failed"):
             await tools["get_status"]()
-        with pytest.raises(SnapshotReloadError):
+        with pytest.raises(ToolError):
             await tools["list_requirements"]()
         return True