From a348368019bbbaf4620fcde74390aab064959164 Mon Sep 17 00:00:00 2001 From: Michael Musser Date: Wed, 1 Jul 2026 20:09:57 -0400 Subject: [PATCH] fix: honor configured connect_timeout on MCP OAuth login path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _reauth_oauth_server (hermes mcp login / reauth) called _probe_single_server without a timeout, so it always used the 30s probe default — far too short for a human browser OAuth round-trip (open → sign in → consent → loopback redirect). The server-level connect_timeout in config.yaml was silently ignored, so login timed out at ~40s no matter what the user configured. Pass the server's configured connect_timeout through, with a 180s floor for the interactive login path. Update the two TestMcpLogin probe mocks for the new kwarg and assert the login path propagates a >=180s timeout. --- hermes_cli/mcp_config.py | 13 ++++++++++++- tests/hermes_cli/test_mcp_config.py | 12 ++++++++++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/hermes_cli/mcp_config.py b/hermes_cli/mcp_config.py index 7c3052b18b5..5304935b8e9 100644 --- a/hermes_cli/mcp_config.py +++ b/hermes_cli/mcp_config.py @@ -781,8 +781,19 @@ def _reauth_oauth_server(name: str, server_config: dict) -> bool: _info(f"Starting OAuth flow for '{name}'...") # Probe triggers the OAuth flow (browser redirect + callback capture). + # Honor the server's configured connect_timeout so a human has enough + # time to complete the browser sign-in; the 30s default is too tight for + # an interactive OAuth round-trip. Give at least 180s for the login path. try: - tools = _probe_single_server(name, server_config) + _login_connect_timeout = server_config.get("connect_timeout") + try: + _login_connect_timeout = float(_login_connect_timeout) + except (TypeError, ValueError): + _login_connect_timeout = 0.0 + _login_connect_timeout = max(_login_connect_timeout, 180.0) + tools = _probe_single_server( + name, server_config, connect_timeout=_login_connect_timeout + ) # A clean probe is NOT proof of authentication. Some MCP servers # (notably Google's official Drive server) serve initialize + # tools/list WITHOUT auth, so the probe lists tools even when the diff --git a/tests/hermes_cli/test_mcp_config.py b/tests/hermes_cli/test_mcp_config.py index bc9f1cbfc3b..8b2f464335f 100644 --- a/tests/hermes_cli/test_mcp_config.py +++ b/tests/hermes_cli/test_mcp_config.py @@ -868,7 +868,9 @@ class TestMcpLogin: # Probe returns tools even though auth never completed. monkeypatch.setattr( "hermes_cli.mcp_config._probe_single_server", - lambda name, cfg: [("search_files", "d"), ("read_file_content", "d")], + lambda name, cfg, connect_timeout=30: [ + ("search_files", "d"), ("read_file_content", "d"), + ], ) # No token file is created → _oauth_tokens_present() returns False. from hermes_cli.mcp_config import cmd_mcp_login @@ -890,7 +892,10 @@ class TestMcpLogin: # cmd_mcp_login wipes tokens before probing, then the real OAuth flow # writes a fresh token during the probe. Simulate that: the mocked # probe drops a token file, mirroring a successful authorization. - def mock_probe(name, cfg): + seen = {} + + def mock_probe(name, cfg, connect_timeout=30): + seen["connect_timeout"] = connect_timeout token_dir.mkdir(exist_ok=True) (token_dir / "realserver.json").write_text('{"access_token": "x"}') return [("a", "d"), ("b", "d"), ("c", "d")] @@ -906,6 +911,9 @@ class TestMcpLogin: assert "Authenticated — 3 tool(s) available" in out assert "no OAuth token" not in out + # The login path must grant a human enough time to finish the browser + # OAuth round-trip — far longer than the 30s probe default. + assert seen["connect_timeout"] >= 180 # ---------------------------------------------------------------------------