diff --git a/hermes_cli/mcp_config.py b/hermes_cli/mcp_config.py index ba9d2ca2ff..3c1a1ae2b9 100644 --- a/hermes_cli/mcp_config.py +++ b/hermes_cli/mcp_config.py @@ -446,7 +446,14 @@ def _probe_single_server( tools_found: List[Tuple[str, str]] = [] async def _probe(): - server = await asyncio.wait_for(_connect_server(name, config), timeout=connect_timeout) + try: + server = await asyncio.wait_for(_connect_server(name, config), timeout=connect_timeout) + except asyncio.TimeoutError: + # str(TimeoutError()) is '' — printed verbatim it was a blank "Authentication failed:". + raise TimeoutError( + f"Connecting to MCP server '{name}' timed out after {float(connect_timeout):.0f}s " + "(bounded by connect_timeout; an OAuth login also by oauth.timeout)" + ) from None try: for t in server._tools: desc = getattr(t, "description", "") or "" @@ -848,25 +855,22 @@ def _reauth_oauth_server(name: str, server_config: dict, *, flow: str | None = N print() _info(f"Starting OAuth flow for '{name}'...") - # The probe triggers the OAuth flow (browser redirect + callback capture). Honor the configured - # connect_timeout, floored at 315s (the 300s OAuth callback window + headroom) — matching the GUI - # re-auth path in web_server.py. force_interactive_oauth: `hermes mcp login` is explicitly - # user-initiated even when stdin isn't a TTY (desktop / agent-spawned terminals), where - # _is_interactive() alone would refuse to open a browser. + # The probe triggers the OAuth flow (browser redirect + callback capture). Its bound must outlast + # the oauth.timeout callback window (plus headroom for the token exchange), or a user who raised + # oauth.timeout still gets cut off at the old fixed floor — matching the GUI re-auth path in + # web_server_mcp.py and tui_gateway/mcp_oauth_sessions.py. force_interactive_oauth: `hermes mcp + # login` is explicitly user-initiated even when stdin isn't a TTY (desktop / agent-spawned + # terminals), where _is_interactive() alone would refuse to open a browser. try: - from tools.mcp_oauth import force_interactive_oauth + from tools.mcp_oauth import force_interactive_oauth, login_connect_timeout - try: - _login_connect_timeout = float(server_config.get("connect_timeout")) - except (TypeError, ValueError): - _login_connect_timeout = 0.0 if selected_flow == "device": from tools.mcp_oauth_device import login_device asyncio.run(login_device(name, url, oauth_cfg)) probe_config = {**server_config, "oauth": {**oauth_cfg, "flow": selected_flow}} with force_interactive_oauth(): tools = _probe_single_server( - name, probe_config, connect_timeout=max(_login_connect_timeout, 315.0) + name, probe_config, connect_timeout=login_connect_timeout(probe_config) ) # A clean probe is NOT proof of authentication: some servers (e.g. Google Drive) serve # initialize + tools/list without auth, so the flow may have failed (e.g. DCR 400 for diff --git a/hermes_cli/web_server_mcp.py b/hermes_cli/web_server_mcp.py index 02a5bf43d9..32f75a3b60 100644 --- a/hermes_cli/web_server_mcp.py +++ b/hermes_cli/web_server_mcp.py @@ -119,7 +119,7 @@ def _run_dashboard_mcp_oauth(flow, cfg: dict) -> None: from agent.secret_scope import build_profile_secret_scope, reset_secret_scope, set_secret_scope from hermes_constants import reset_hermes_home_override, set_hermes_home_override from tools.mcp_dashboard_oauth import dashboard_oauth_flow - from tools.mcp_oauth import HermesTokenStorage, force_interactive_oauth + from tools.mcp_oauth import HermesTokenStorage, force_interactive_oauth, login_connect_timeout from tools.mcp_oauth_manager import get_manager home_token = set_hermes_home_override(flow.hermes_home) @@ -134,9 +134,7 @@ def _run_dashboard_mcp_oauth(flow, cfg: dict) -> None: try: previous_entry = manager.remove(flow.server_name, hermes_home=flow.hermes_home) tools = _probe_single_server( - flow.server_name, - cfg, - connect_timeout=max(float(cfg.get("connect_timeout", 0) or 0), 315), + flow.server_name, cfg, connect_timeout=login_connect_timeout(cfg) ) if not _oauth_tokens_present(flow.server_name): raise RuntimeError( diff --git a/tests/hermes_cli/test_mcp_login_window.py b/tests/hermes_cli/test_mcp_login_window.py new file mode 100644 index 0000000000..76272f1d6f --- /dev/null +++ b/tests/hermes_cli/test_mcp_login_window.py @@ -0,0 +1,50 @@ +"""An OAuth login waits as long as ``oauth.timeout`` says, and a probe timeout names itself (#116278). + +``hermes mcp login`` (and the dashboard / Desktop re-auth twins) bounded the login probe at a fixed +315 s floor, so ``oauth: {timeout: 3600}`` changed nothing; the expiry then surfaced as a bare +``asyncio.TimeoutError`` whose ``str()`` is empty — a blank ``✗ Authentication failed:`` line. +""" + +import asyncio +import contextlib +import io + +import pytest + + +def test_login_probe_window_follows_oauth_timeout(monkeypatch, tmp_path): + """Production entry point ``_reauth_oauth_server``: with ``oauth.timeout: 3600`` the probe's + connect bound is the callback window plus exchange headroom, not the old 315 s floor.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + import hermes_cli.mcp_config as mc + + seen = {} + + def _recording_probe(name, config, connect_timeout=None, **kw): + seen["connect_timeout"] = connect_timeout + raise RuntimeError("probe body replaced by recorder") + + monkeypatch.setattr(mc, "_probe_single_server", _recording_probe) + with contextlib.redirect_stdout(io.StringIO()): + mc._reauth_oauth_server( + "traveler", + {"url": "https://mcp.example.test/mcp", "auth": "oauth", + "oauth": {"timeout": 3600, "redirect_port": 27891}}, + ) + assert seen["connect_timeout"] == 3615.0 + + +def test_probe_timeout_names_server_and_knobs(monkeypatch): + """A probe that outlives its bound raises a TimeoutError whose message names the server and the + governing settings — never the empty ``str(asyncio.TimeoutError())``.""" + import hermes_cli.mcp_config as mc + + async def _hang(name, config): + await asyncio.sleep(3600) + + monkeypatch.setattr("tools.mcp_tool_discovery._connect_server", _hang) + with pytest.raises(TimeoutError) as info: + mc._probe_single_server("hangsrv", {"url": "https://mcp.example.test/mcp"}, connect_timeout=0.2) + message = str(info.value) + assert "hangsrv" in message and "timed out" in message + assert "connect_timeout" in message and "oauth.timeout" in message diff --git a/tools/mcp_oauth.py b/tools/mcp_oauth.py index 4a4d578020..923848d1f9 100644 --- a/tools/mcp_oauth.py +++ b/tools/mcp_oauth.py @@ -1019,6 +1019,21 @@ def token_request_user_agent(cfg: dict) -> str | None: return ua.strip() if isinstance(ua, str) and ua.strip() else None +def login_connect_timeout(config: dict) -> float: + """Connect bound for an interactive OAuth login probe (CLI ``hermes mcp login``, dashboard and + Desktop re-auth): the server's ``connect_timeout`` or its ``oauth.timeout`` callback window + (default 300 s) plus 15 s headroom for the token exchange, whichever is longer. A fixed 315 s + floor here made raising ``oauth.timeout`` alone a no-op — the probe timed out first (#116278).""" + def _seconds(value, default: float) -> float: + try: + return max(0.0, float(value)) + except (TypeError, ValueError): + return default + oauth_cfg = config.get("oauth") or {} + return max(_seconds(config.get("connect_timeout"), 0.0), + _seconds(oauth_cfg.get("timeout"), 300.0) + 15.0) + + def _configure_callback_port(cfg: dict, storage: "HermesTokenStorage | None" = None) -> int: """Resolve the callback port into ``cfg['_resolved_port']`` (0 = non-loopback URI). Precedence: dashboard flow / cached https redirect URI → CIMD pinned port (sets ``cfg['_cimd_url']``) → diff --git a/tui_gateway/mcp_oauth_sessions.py b/tui_gateway/mcp_oauth_sessions.py index 9b4ebdaf3b..de2bec209a 100644 --- a/tui_gateway/mcp_oauth_sessions.py +++ b/tui_gateway/mcp_oauth_sessions.py @@ -85,7 +85,7 @@ def _probe_with_rollback( server_name: str, cfg: dict, hermes_home: str, flow, reconnect_live: bool) -> None: """Run the OAuth probe; on ANY failure restore the prior token file + manager entry.""" from hermes_cli.mcp_config import _oauth_tokens_present, _probe_single_server, _save_mcp_server - from tools.mcp_oauth import HermesTokenStorage + from tools.mcp_oauth import HermesTokenStorage, login_connect_timeout from tools.mcp_oauth_manager import get_manager manager = get_manager() storage = HermesTokenStorage(server_name) @@ -93,8 +93,7 @@ def _probe_with_rollback( previous_entry = None try: previous_entry = manager.remove(server_name, hermes_home=hermes_home) - timeout = max(float(cfg.get("connect_timeout", 0) or 0), 315) - tools = _probe_single_server(server_name, cfg, connect_timeout=timeout) + tools = _probe_single_server(server_name, cfg, connect_timeout=login_connect_timeout(cfg)) if not _oauth_tokens_present(server_name): raise RuntimeError( "The server responded, but no OAuth token was obtained — " diff --git a/website/docs/user-guide/features/mcp.md b/website/docs/user-guide/features/mcp.md index 1b84bd943a..9e58f8ca3d 100644 --- a/website/docs/user-guide/features/mcp.md +++ b/website/docs/user-guide/features/mcp.md @@ -391,6 +391,8 @@ Then run `hermes mcp login googledrive` — with the pre-registered client, Herm **Pitfall — config auto-reload race.** When you edit `~/.hermes/config.yaml` from inside a running Hermes session, the CLI auto-reloads MCP connections with a 30s timeout. That's not enough for an interactive OAuth flow. Add the entry, then run `hermes mcp login ` from a fresh terminal — it waits the full 5 minutes for you to complete auth. +**Need longer than 5 minutes to approve?** Set `oauth.timeout` on the server entry (seconds). `hermes mcp login`, the dashboard and Desktop re-auth all wait `oauth.timeout` + 15 s (or the entry's `connect_timeout`, whichever is longer); a login that still runs out of time reports `Connecting to MCP server '' timed out after Ns` naming both knobs instead of a blank failure line. + ## mTLS / client certificates Remote HTTP MCP servers that require mutual TLS (client-certificate authentication) are supported via `client_cert` / `client_key`. Hermes passes the resolved certificate to the underlying HTTP client for the TLS handshake.