From 9ccbb9b2f87de38450a9e442a5e3cb3a3c025de6 Mon Sep 17 00:00:00 2001
From: glorydeus-cyber <264702366+glorydeus-cyber@users.noreply.github.com>
Date: Thu, 30 Jul 2026 01:10:06 -0400
Subject: [PATCH] fix(mcp): keep tool errors out of server breaker

---
 tests/tools/test_mcp_circuit_breaker.py | 39 +++++++++++++++++++++++++
 tools/mcp_tool.py                       | 14 ++++-----
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/tests/tools/test_mcp_circuit_breaker.py b/tests/tools/test_mcp_circuit_breaker.py
index 357589d06621d..4d9603b6597e9 100644
--- a/tests/tools/test_mcp_circuit_breaker.py
+++ b/tests/tools/test_mcp_circuit_breaker.py
@@ -101,6 +101,45 @@ def _cleanup(mcp_tool_module, name: str) -> None:
 # ---------------------------------------------------------------------------
 
 
+def test_tool_level_errors_do_not_trip_server_breaker(monkeypatch, tmp_path):
+    """A completed MCP round-trip with ``isError=True`` proves transport health.
+
+    Validation/domain errors belong to the request or tool, not to the server
+    connection. Repeating them must preserve the actionable tool error without
+    opening the server-level circuit breaker.
+    """
+    monkeypatch.setenv("HERMES_HOME", str(tmp_path))
+
+    from tools import mcp_tool
+    from tools.mcp_tool import _make_tool_handler
+
+    call_count = {"n": 0}
+
+    async def _call_tool_validation_error(*a, **kw):
+        call_count["n"] += 1
+        result = MagicMock()
+        result.isError = True
+        block = MagicMock()
+        block.text = "REQUEST_INVALID: type 'solution' is not allowed"
+        result.content = [block]
+        result.structuredContent = None
+        return result
+
+    _install_stub_server(mcp_tool, "srv-semantic", _call_tool_validation_error)
+    mcp_tool._ensure_mcp_loop()
+
+    try:
+        handler = _make_tool_handler("srv-semantic", "note_create", 10.0)
+        for _ in range(mcp_tool._CIRCUIT_BREAKER_THRESHOLD + 1):
+            parsed = json.loads(handler({"type": "solution"}))
+            assert "REQUEST_INVALID" in parsed.get("error", ""), parsed
+
+        assert call_count["n"] == mcp_tool._CIRCUIT_BREAKER_THRESHOLD + 1
+        assert mcp_tool._server_error_counts.get("srv-semantic", 0) == 0
+    finally:
+        _cleanup(mcp_tool, "srv-semantic")
+
+
 def test_circuit_breaker_half_opens_after_cooldown(monkeypatch, tmp_path):
     """After a tripped breaker's cooldown elapses, the *next* call must
     actually execute against the session (half-open probe). When the
diff --git a/tools/mcp_tool.py b/tools/mcp_tool.py
index fe5ab9cfdfa42..5819707538c14 100644
--- a/tools/mcp_tool.py
+++ b/tools/mcp_tool.py
@@ -4894,15 +4894,11 @@ def _call_once():
 
         try:
             result = _call_once()
-            # Check if the MCP tool itself returned an error
-            try:
-                parsed = json.loads(result)
-                if "error" in parsed:
-                    _bump_server_error(server_name)
-                else:
-                    _reset_server_error(server_name)  # success — reset
-            except (json.JSONDecodeError, TypeError):
-                _reset_server_error(server_name)  # non-JSON = success
+            # Any CallToolResult — including ``isError=True`` — proves that the
+            # MCP transport completed a healthy RPC round-trip. Tool-level
+            # validation/domain errors belong to the request or tool and must
+            # not open the server-level circuit breaker.
+            _reset_server_error(server_name)
             return result
         except InterruptedError:
             return _interrupted_call_result()