From 7dab93b06e2bb3757dc18229169efcee1b5b47a3 Mon Sep 17 00:00:00 2001 From: David Metcalfe Date: Fri, 7 Aug 2026 12:27:12 -0700 Subject: [PATCH] fix(mcp): reject fabricated DCR for providers without RFC 7591 registration (#78190) Google's hosted Gmail/Drive MCP servers advertise no registration_endpoint in their ASM, so the mcp SDK's create_client_registration_request falls back to guessing POST {server_origin}/register. Those servers answer with an opaque 404 (OAuthRegistrationError: Registration failed: 404) and the gateway burns the reconnect ladder on an unrecoverable failure while the CLI looks healthy because tools/list is unauthenticated on Google servers. - Guard: HermesMCPOAuthProvider intercepts the fabricated registration POST in the bidirectional auth_flow bridge and raises an actionable OAuthRegistrationError (create an OAuth client, add oauth.client_id/ client_secret under mcp_servers..oauth in config.yaml) instead of sending the request. Fires only when the URL is the exact SDK fallback AND the ASM lacks a registration_endpoint, so servers advertising a real RFC 7591 endpoint pass through untouched. - Surfacing: needs_reauth tool errors now include the registration guidance, and the generic 'MCP call failed' path unwraps TaskGroup exception groups so the root cause reaches the model instead of 'unhandled errors in a TaskGroup (1 sub-exception)'. - humanize_oauth_registration_error: 404 registration failures get the same pre-registered-client guidance as 403 (CLI login path). Tests: full discovery dance (401 -> PRM -> ASM without registration_endpoint -> fabricated POST) drives the bridge and asserts the guard raises before any network send; humanize 404 + passthrough cases; _auth_error_detail and group unwrap surfacing. Rebuilt on current main, where mcp_tool.py was decomposed and mcp 2.0 rides on httpx2: the surfacing helpers live in tools/mcp_tool_errors.py and feed the shared _dispatch / _NEEDS_REAUTH_MSG paths in tools/mcp_tool_handlers.py; the guard compares with the request's own URL type and only fires once ASM was actually discovered (an undiscovered ASM keeps main's discovery-context error, #113771), and closes the inner SDK generator before raising so context.lock is released immediately. --- .../tools/test_mcp_failure_classification.py | 64 +++- tests/tools/test_mcp_oauth.py | 31 ++ tests/tools/test_mcp_oauth_bidirectional.py | 274 ++++++++++++++++++ tools/mcp_oauth.py | 11 +- tools/mcp_oauth_manager.py | 38 +++ tools/mcp_tool_errors.py | 22 +- tools/mcp_tool_handlers.py | 10 +- 7 files changed, 443 insertions(+), 7 deletions(-) diff --git a/tests/tools/test_mcp_failure_classification.py b/tests/tools/test_mcp_failure_classification.py index 2bd8454c53..b3d87bc03a 100644 --- a/tests/tools/test_mcp_failure_classification.py +++ b/tests/tools/test_mcp_failure_classification.py @@ -14,7 +14,8 @@ import logging import pytest from tools.mcp_tool_errors import ( - InvalidMcpUrlError, NonMcpEndpointError, _classify_mcp_failure, _unwrap_exception_group) + InvalidMcpUrlError, NonMcpEndpointError, _auth_error_detail, _classify_mcp_failure, _mcp_call_failed_message, + _unwrap_exception_group) from tools.mcp_tool import MCPServerTask @@ -66,6 +67,67 @@ class TestClassifyMcpFailure: assert _classify_mcp_failure(g) == "permanent" +# ── _auth_error_detail / _mcp_call_failed_message ──────────────────────────── + +class TestAuthErrorSurfacing: + def test_registration_detail_surfaces(self): + """The actionable registration guidance raised by the DCR guard must + reach the needs_reauth tool error (GH#78190).""" + pytest.importorskip("mcp.client.auth") + from mcp.client.auth import OAuthRegistrationError + + exc = OAuthRegistrationError( + "MCP OAuth 'gmail': this provider does not support automatic " + "client registration ... add it under config.yaml " + "mcp_servers..oauth (client_id, client_secret), then run " + "`hermes mcp login gmail`." + ) + detail = _auth_error_detail(exc) + assert detail.startswith(" ") + assert "config.yaml" in detail + assert "client_id" in detail + + def test_registration_detail_unwraps_group(self): + """The transport raises the auth error inside a TaskGroup wrapper — + the detail must still be found.""" + pytest.importorskip("mcp.client.auth") + from mcp.client.auth import OAuthRegistrationError + + exc = OAuthRegistrationError("config.yaml guidance") + assert "config.yaml" in _auth_error_detail(_group(exc)) + + def test_non_registration_no_detail(self): + assert _auth_error_detail(ValueError("nope")) == "" + assert _auth_error_detail(_group(TimeoutError("t"))) == "" + + def test_mcp_call_failed_message_unwraps_group(self): + """The generic error path must surface the group root cause instead + of the opaque TaskGroup wrapper text.""" + pytest.importorskip("mcp.client.auth") + from mcp.client.auth import OAuthRegistrationError + + exc = OAuthRegistrationError("registration guidance text") + msg = _mcp_call_failed_message(_group(exc)) + assert "OAuthRegistrationError" in msg + assert "registration guidance text" in msg + assert "TaskGroup" not in msg + + def test_needs_reauth_tool_error_carries_guidance(self, monkeypatch): + """Through the real recovery handler: an unrecoverable registration + failure returns needs_reauth WITH the guard's guidance.""" + pytest.importorskip("mcp.client.auth") + import json + from mcp.client.auth import OAuthRegistrationError + from tools import mcp_tool_handlers as handlers + + monkeypatch.setattr(handlers._loop, "_run_on_mcp_loop", lambda *_a, **_k: False) + out = json.loads(handlers._handle_auth_error_and_retry( + "gmail", OAuthRegistrationError("Create an OAuth client (config.yaml)."), + lambda: None, "call_tool")) + assert out["needs_reauth"] is True + assert "requires re-authentication. Create an OAuth client (config.yaml). Run" in out["error"] + + # ── Keepalive failure log surfaces the root cause ──────────────────────────── @pytest.mark.no_isolate diff --git a/tests/tools/test_mcp_oauth.py b/tests/tools/test_mcp_oauth.py index 610e434fa0..0597cf742c 100644 --- a/tests/tools/test_mcp_oauth.py +++ b/tests/tools/test_mcp_oauth.py @@ -1104,3 +1104,34 @@ def test_humanize_non_registration_403_passthrough(): ) is None ) + + +def test_humanize_404_registration_error_guidance(): + """A 404 on the registration endpoint surfaces pre-registered-client + guidance instead of the raw SDK traceback (GH#78190 — Google's hosted + Gmail/Drive MCP servers answer the SDK's guessed /register with 404).""" + from tools.mcp_oauth import humanize_oauth_registration_error + + msg = humanize_oauth_registration_error( + "gmail", + "Registration failed: 404 Error 404 (Not Found)!!1", + server_url="https://gmailmcp.googleapis.com/mcp/v1", + ) + assert msg is not None + assert "does not support automatic client registration" in msg + assert "client_id" in msg + assert "hermes mcp login gmail" in msg + + +def test_humanize_404_non_registration_passthrough(): + """A 404 that has nothing to do with registration stays raw.""" + from tools.mcp_oauth import humanize_oauth_registration_error + + assert ( + humanize_oauth_registration_error( + "srv", + RuntimeError("HTTP 404: no such tool"), + server_url="https://example.com/mcp", + ) + is None + ) diff --git a/tests/tools/test_mcp_oauth_bidirectional.py b/tests/tools/test_mcp_oauth_bidirectional.py index 6ee6703610..3b8481c000 100644 --- a/tests/tools/test_mcp_oauth_bidirectional.py +++ b/tests/tools/test_mcp_oauth_bidirectional.py @@ -234,3 +234,277 @@ async def _noop_callback() -> tuple[str, str | None]: raise AssertionError( "callback handler should not be invoked in bidirectional-generator tests" ) + + +@pytest.mark.asyncio +async def test_fabricated_dcr_registration_rejected_before_network(tmp_path, monkeypatch): + """The bridge must raise instead of sending the SDK's *fabricated* DCR POST. + + Google's hosted Gmail/Drive MCP servers advertise no RFC 7591 + ``registration_endpoint`` in their ASM, so the SDK falls back to guessing + ``POST {server_origin}/register`` (``create_client_registration_request``, + mcp/client/auth/utils.py) — a URL those servers answer with an opaque 404 + (GH#78190). The guard in ``HermesMCPOAuthProvider`` must intercept that + guessed request and raise an actionable ``OAuthRegistrationError`` before + any network call, instead of letting the gateway burn the reconnect + ladder on an unrecoverable registration failure. + + Drive the full discovery dance (401 -> PRM -> ASM without + registration_endpoint -> fabricated registration POST) through the + generator manually, exactly like the other tests in this file. + """ + from tools.mcp_tool import sdk_httpx + httpx = sdk_httpx() + from mcp.client.auth import OAuthRegistrationError + from mcp.shared.auth import OAuthClientMetadata + from pydantic import AnyUrl + + from tools.mcp_oauth import HermesTokenStorage + from tools.mcp_oauth_manager import _HERMES_PROVIDER_CLS, reset_manager_for_tests + + assert _HERMES_PROVIDER_CLS is not None, "SDK OAuth types must be available" + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + reset_manager_for_tests() + + # Empty storage: no tokens, no client_info — the SDK's 401 branch must + # take the registration path (this is the cold gateway state for a + # provider whose cached client is gone). + storage = HermesTokenStorage("srv") + + metadata = OAuthClientMetadata( + redirect_uris=[AnyUrl("http://127.0.0.1:12345/callback")], + client_name="Hermes Agent", + ) + provider = _HERMES_PROVIDER_CLS( + server_name="srv", + server_url="https://example.com/mcp", + client_metadata=metadata, + storage=storage, + redirect_handler=_noop_redirect, + callback_handler=_noop_callback, + ) + + req = httpx.Request("POST", "https://example.com/mcp") + flow = provider.async_auth_flow(req) + + # 1. Initial outbound MCP request. + outbound = await flow.__anext__() + assert outbound is not None + + # 2. 401 with a resource_metadata pointer -> SDK starts PRM discovery. + fake_401 = httpx.Response( + 401, + request=outbound, + headers={ + "www-authenticate": ( + 'Bearer resource_metadata="https://example.com/.well-known/' + 'oauth-protected-resource"' + ) + }, + ) + prm_request = await flow.asend(fake_401) + assert isinstance(prm_request, httpx.Request) + assert prm_request.method == "GET" + + # 3. Serve PRM pointing at a fake authorization server. + prm_response = httpx.Response( + 200, + request=prm_request, + headers={"Content-Type": "application/json"}, + json={ + "authorization_servers": ["https://auth.example.com"], + "resource": "https://example.com/mcp", + "bearer_methods_supported": ["header"], + }, + ) + asm_request = await flow.asend(prm_response) + assert isinstance(asm_request, httpx.Request) + assert asm_request.method == "GET" + + # 4. Serve ASM with NO registration_endpoint (the Google Gmail/Drive + # MCP class of provider). + asm_response = httpx.Response( + 200, + request=asm_request, + headers={"Content-Type": "application/json"}, + json={ + "issuer": "https://auth.example.com", + "authorization_endpoint": "https://auth.example.com/authorize", + "token_endpoint": "https://auth.example.com/token", + "response_types_supported": ["code"], + "code_challenge_methods_supported": ["S256"], + }, + ) + + # 5. The SDK now wants to POST the fabricated registration URL — the + # bridge must raise the actionable error instead of yielding it. + with pytest.raises(OAuthRegistrationError) as excinfo: + await flow.asend(asm_response) + text = str(excinfo.value) + assert "does not support automatic client registration" in text + assert "https://example.com/register" in text + assert "config.yaml" in text + assert "hermes mcp login srv" in text + await flow.aclose() + + +@pytest.mark.asyncio +async def test_advertised_registration_endpoint_passes_through(tmp_path, monkeypatch): + """A server that *advertises* a registration endpoint must NOT be blocked. + + Real-world case: Linear's MCP ASM advertises + ``registration_endpoint: https://mcp.linear.app/register`` — the same + origin+path the SDK would fabricate. The guard's AND condition (exact + fabricated URL **and** no advertised registration_endpoint) must let + such servers through; otherwise every legitimate DCR flow breaks. + """ + from tools.mcp_tool import sdk_httpx + httpx = sdk_httpx() + from mcp.shared.auth import OAuthClientMetadata + from pydantic import AnyUrl + + from tools.mcp_oauth import HermesTokenStorage + from tools.mcp_oauth_manager import _HERMES_PROVIDER_CLS, reset_manager_for_tests + + assert _HERMES_PROVIDER_CLS is not None, "SDK OAuth types must be available" + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + reset_manager_for_tests() + + storage = HermesTokenStorage("srv") + metadata = OAuthClientMetadata( + redirect_uris=[AnyUrl("http://127.0.0.1:12345/callback")], + client_name="Hermes Agent", + ) + provider = _HERMES_PROVIDER_CLS( + server_name="srv", + server_url="https://example.com/mcp", + client_metadata=metadata, + storage=storage, + redirect_handler=_noop_redirect, + callback_handler=_noop_callback, + ) + + req = httpx.Request("POST", "https://example.com/mcp") + flow = provider.async_auth_flow(req) + + outbound = await flow.__anext__() + fake_401 = httpx.Response( + 401, + request=outbound, + headers={ + "www-authenticate": ( + 'Bearer resource_metadata="https://example.com/.well-known/' + 'oauth-protected-resource"' + ) + }, + ) + prm_request = await flow.asend(fake_401) + prm_response = httpx.Response( + 200, + request=prm_request, + headers={"Content-Type": "application/json"}, + json={ + "authorization_servers": ["https://auth.example.com"], + "resource": "https://example.com/mcp", + "bearer_methods_supported": ["header"], + }, + ) + asm_request = await flow.asend(prm_response) + + # ASM DOES advertise a registration endpoint — at origin+/register, + # exactly the URL the SDK would otherwise fabricate (Linear's ASM does + # this: https://mcp.linear.app/register). + asm_response = httpx.Response( + 200, + request=asm_request, + headers={"Content-Type": "application/json"}, + json={ + "issuer": "https://auth.example.com", + "authorization_endpoint": "https://auth.example.com/authorize", + "token_endpoint": "https://auth.example.com/token", + "registration_endpoint": "https://example.com/register", + "response_types_supported": ["code"], + "code_challenge_methods_supported": ["S256"], + }, + ) + + # The registration POST must be yielded (not blocked): the SDK uses the + # advertised endpoint and the guard must pass it through. + registration_request = await flow.asend(asm_response) + assert isinstance(registration_request, httpx.Request) + assert registration_request.method == "POST" + assert str(registration_request.url) == "https://example.com/register" + + # No OAuthRegistrationError may have been raised along the way. (The + # dance ends here: a 200 registration response would hand control to the + # browser-redirect flow, which is outside this test's scope.) + await flow.aclose() + + +@pytest.mark.asyncio +async def test_guard_ignores_non_fabricated_post_paths(tmp_path, monkeypatch): + """A POST to a *different* path must never be blocked by the guard. + + The guard is exact-URL matched: only the SDK's fabricated + ``{origin}/register`` POST is rejected when the ASM lacks a + registration_endpoint. A server-side registration at ``/foo/register`` + or any other path passes through untouched. Undiscovered metadata (a + blocked ASM fetch) never trips the guard: the SDK's guess may be a real + endpoint, and the discovery-context error explains a failure there. + """ + from tools.mcp_tool import sdk_httpx + httpx = sdk_httpx() + from mcp.shared.auth import OAuthClientMetadata, OAuthMetadata + from pydantic import AnyUrl + + from tools.mcp_oauth import HermesTokenStorage + from tools.mcp_oauth_manager import _HERMES_PROVIDER_CLS, reset_manager_for_tests + + assert _HERMES_PROVIDER_CLS is not None, "SDK OAuth types must be available" + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + reset_manager_for_tests() + + provider = _HERMES_PROVIDER_CLS( + server_name="srv", + server_url="https://example.com/mcp", + client_metadata=OAuthClientMetadata( + redirect_uris=[AnyUrl("http://127.0.0.1:12345/callback")], + client_name="Hermes Agent", + ), + storage=HermesTokenStorage("srv"), + redirect_handler=_noop_redirect, + callback_handler=_noop_callback, + ) + + # No ASM discovered: even the exact guessed URL passes through. + assert provider.context.oauth_metadata is None + await provider._maybe_reject_fabricated_registration( + httpx.Request("POST", "https://example.com/register") + ) # must not raise + + # ASM discovered without registration_endpoint: only the exact + # origin+/register POST is refused; everything else passes. + provider.context.oauth_metadata = OAuthMetadata( + issuer="https://auth.example.com", + authorization_endpoint="https://auth.example.com/authorize", + token_endpoint="https://auth.example.com/token", + ) + for method, url in ( + ("POST", "https://example.com/foo/register"), + ("POST", "https://example.com/register/"), + ("POST", "https://example.com/registers"), + ("POST", "https://other.example.com/register"), + ("POST", "https://example.com:8443/register"), + ("GET", "https://example.com/register"), + ): + await provider._maybe_reject_fabricated_registration( + httpx.Request(method, url) + ) # must not raise + from mcp.client.auth import OAuthRegistrationError + with pytest.raises(OAuthRegistrationError): + await provider._maybe_reject_fabricated_registration( + httpx.Request("POST", "https://example.com:443/register") + ) diff --git a/tools/mcp_oauth.py b/tools/mcp_oauth.py index 1a05f37f8f..4e58be3f1c 100644 --- a/tools/mcp_oauth.py +++ b/tools/mcp_oauth.py @@ -1195,7 +1195,7 @@ def _maybe_preregister_client(storage: "HermesTokenStorage", cfg: dict, client_m def humanize_oauth_registration_error( server_name: str, exc: BaseException | str, *, server_url: str | None = None) -> str | None: - """Turn a DCR 403/Forbidden into a useful next step; None for anything else so the caller keeps the + """Turn a DCR 403/Forbidden or 404 into a useful next step; None for anything else so the caller keeps the original text. Figma gates DCR on exact ``client_name`` (auto-set to ``Claude Code``), so this fires when the user overrode it or an older Hermes is running.""" msg = str(exc) @@ -1203,6 +1203,15 @@ def humanize_oauth_registration_error( from tools.mcp_oauth_provider import _DISCOVERY_CONTEXT_LEAD if msg.startswith(_DISCOVERY_CONTEXT_LEAD): # the 403 there is the metadata fetch, not a DCR refusal return None + # A 404 on registration (Google's hosted Gmail/Drive MCP servers answer the SDK's guessed /register with + # one) is the same permanent "no RFC 7591 DCR" class as a 403 refusal (#78190). + if ("404" in msg and ("not found" in lowered or "/register" in lowered) + and any(k in lowered for k in ("regist", "dcr", "dynamic client"))): + return ( + f"'{server_name}' does not support automatic client registration — the registration request returned " + "404 (typical for Google's hosted Gmail/Drive MCP servers and other providers without RFC 7591 dynamic " + "client registration). Create an OAuth client for this provider and add it under config.yaml as " + f"`oauth: {{client_id: ..., client_secret: ...}}`, then re-run:\n hermes mcp login {server_name}") looks_like_registration = ("403" in msg or "forbidden" in lowered) and ( any(k in lowered for k in ("regist", "dcr", "dynamic client")) or lowered.strip() in {"forbidden", "403 forbidden", "http 403: forbidden"} diff --git a/tools/mcp_oauth_manager.py b/tools/mcp_oauth_manager.py index 9f3a2a8339..c9aa2c7b49 100644 --- a/tools/mcp_oauth_manager.py +++ b/tools/mcp_oauth_manager.py @@ -206,6 +206,38 @@ class HermesMCPOAuthProvider(HermesProviderMixin, *_SDK_BASES): except Exception as exc: # pragma: no cover — must not throw self._log_nonfatal("invalid_client detection", exc) + async def _maybe_reject_fabricated_registration(self, outgoing: Any) -> None: + """Refuse to send the SDK's *fabricated* dynamic client registration (#78190). + + When the discovered authorization-server metadata (RFC 8414) has no ``registration_endpoint``, + ``create_client_registration_request`` (mcp/client/auth/utils.py) guesses ``POST {origin}/register``. + Providers without RFC 7591 DCR — Google's hosted Gmail/Drive MCP servers among them — answer with an + opaque 404 and the gateway burns its reconnect ladder on an unrecoverable failure. Their supported + path is a pre-registered client (``oauth.client_id`` / ``client_secret``), so raise that guidance + instead of sending the request. + + Fires only when the metadata WAS discovered and lacks the endpoint, and the request is a POST to the + exact guessed URL. Undiscovered metadata (a WAF-blocked fetch) keeps the SDK's guess, so a real + ``/register`` still works and the discovery-context error (#113771) still explains a failure. + """ + meta = getattr(self.context, "oauth_metadata", None) + if meta is None or getattr(meta, "registration_endpoint", None) or getattr(outgoing, "method", None) != "POST": + return + from urllib.parse import urljoin + url = outgoing.url + # Same URL type as the request (httpx or httpx2), so default-port handling matches on both sides. + expected = type(url)(urljoin(self.context.get_authorization_base_url(self.context.server_url), "/register")) + if (url.scheme, url.host, url.port, url.path) != (expected.scheme, expected.host, expected.port, expected.path): + return + from mcp.client.auth.oauth2 import OAuthRegistrationError + name = self._hermes_server_name + raise OAuthRegistrationError( + f"MCP OAuth '{name}': this provider does not support automatic client registration (its authorization " + f"server advertises no registration_endpoint), so the SDK's fallback would POST {expected} and fail " + "(Google's hosted Gmail/Drive MCP servers and similar providers return 404 for it). Create an OAuth " + "client for this provider and add it under config.yaml mcp_servers..oauth (client_id, " + f"client_secret), then run `hermes mcp login {name}`.") + async def async_auth_flow(self, request): # type: ignore[override] try: # pre-flow hook: reload from disk if it changed (non-fatal on error) await get_manager().invalidate_if_disk_changed(self._hermes_server_name, hermes_home=self._hermes_home) @@ -234,6 +266,12 @@ class HermesMCPOAuthProvider(HermesProviderMixin, *_SDK_BASES): sent_access_token = tokens.access_token if tokens is not None else None self.context.lock.release() resource_lock_released = True + else: + try: # refuse the SDK's guessed DCR POST before it hits the network + await self._maybe_reject_fabricated_registration(outgoing) + except Exception: + await inner.aclose() # release the SDK's context.lock now, not at GC + raise incoming = yield outgoing if resource_lock_released: await self.context.lock.acquire() diff --git a/tools/mcp_tool_errors.py b/tools/mcp_tool_errors.py index 0003d5a4a7..757cde7316 100644 --- a/tools/mcp_tool_errors.py +++ b/tools/mcp_tool_errors.py @@ -11,7 +11,7 @@ import os import re from typing import Any, List, Optional from urllib.parse import urlparse -from tools.mcp_tool_common import _sanitize_error, _core +from tools.mcp_tool_common import _exc_str, _sanitize_error, _core from tools.mcp_tool_node_abi import NodeAbiMismatchError logger = logging.getLogger("tools.mcp_tool") @@ -154,6 +154,26 @@ def _contains_only_cancellation(exc: BaseException) -> bool: return isinstance(exc, asyncio.CancelledError) +def _auth_error_detail(exc: BaseException) -> str: + """`` `` when *exc* (or its group root) is the SDK's ``OAuthRegistrationError``, else ``""``. The DCR + guard in ``HermesMCPOAuthProvider`` raises one naming the next step (pre-register an OAuth client), and the + generic needs-reauth text alone would hide it from the model (#78190).""" + try: + from mcp.client.auth import OAuthRegistrationError + except ImportError: # pragma: no cover — SDK required in CI + return "" + root = _unwrap_exception_group(exc) + text = str(root).strip() if isinstance(root, OAuthRegistrationError) else "" + return f" {_sanitize_error(text)}" if text else "" + + +def _mcp_call_failed_message(exc: BaseException) -> str: + """Generic tool-error text naming the group root cause: the transport's TaskGroup wrapper ``str()`` is + "unhandled errors in a TaskGroup (1 sub-exception)", which tells the model nothing.""" + root = _unwrap_exception_group(exc) + return _sanitize_error(f"MCP call failed: {type(root).__name__}: {_exc_str(root)}") + + def _classify_mcp_failure(exc: BaseException) -> str: """``'permanent'`` (``run()`` parks instead of burning the retry ladder: auth 401/403, NonMcpEndpointError, InvalidMcpUrlError, missing stdio command, native addon built for another diff --git a/tools/mcp_tool_handlers.py b/tools/mcp_tool_handlers.py index 54f81aa737..5eaf591791 100644 --- a/tools/mcp_tool_handlers.py +++ b/tools/mcp_tool_handlers.py @@ -21,7 +21,8 @@ from tools.mcp_tool_content import ( _MCP_HARD_RESULT_CAP_CHARS, _cache_mcp_audio_block, _cache_mcp_image_block, _mcp_result_with_native_images, _render_mcp_dropped_block_notice, _render_mcp_resource_block, _strip_reserved_meta_keys, _truncate_mcp_text_result) -from tools.mcp_tool_errors import _is_auth_error, _is_session_expired_error +from tools.mcp_tool_errors import ( + _auth_error_detail, _is_auth_error, _is_session_expired_error, _mcp_call_failed_message) logger = logging.getLogger("tools.mcp_tool") _MISSING = object() @@ -29,7 +30,7 @@ _MISSING = object() declaration.on_change = invalidate_check_fn_cache _NEEDS_REAUTH_MSG = ( - "MCP server '{s}' requires re-authentication. Run `hermes mcp login {s}` (or delete the tokens file under " + "MCP server '{s}' requires re-authentication.{detail} Run `hermes mcp login {s}` (or delete the tokens file under " "~/.hermes/mcp-tokens/ and restart). Do NOT retry this tool — ask the user to re-authenticate.") _STDIO_NO_RESPAWN_MSG = ( "MCP server '{s}' stdio subprocess had exited (this is not a timeout — the call never reached the server). A " @@ -210,7 +211,8 @@ def _handle_auth_error_and_retry(server_name: str, exc: BaseException, retry_cal result = _retry_once(server_name, retry_call, op_description, "auth recovery") if result is not None: return result - return _strike(server_name, _NEEDS_REAUTH_MSG.format(s=server_name), needs_reauth=True, server=server_name) + return _strike(server_name, _NEEDS_REAUTH_MSG.format(s=server_name, detail=_auth_error_detail(exc)), + needs_reauth=True, server=server_name) def _handle_session_expired_and_retry(server_name: str, exc: BaseException, retry_call, op_description: str, @@ -345,7 +347,7 @@ def _dispatch(server_name: str, server: Any, op: str, call, tool_timeout: float, if recovered is not None: return recovered on_final_failure(exc) - return tool_error(_sanitize_error(f"MCP call failed: {type(exc).__name__}: {_exc_str(exc)}")) + return tool_error(_mcp_call_failed_message(exc)) @asynccontextmanager