MCP OAuth probe timeout too short or ignored

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #59219 fix(mcp): honor per-server connect_timeout in CLI/GUI probes (salvage #34276, #56699, #54494)

  merged to main

  files: hermes_cli/mcp_config.py, hermes_cli/subcommands/mcp.py, hermes_cli/web_server.py, scripts/release.py, tests/hermes_cli/test_mcp_add_command_dest.py, tests/hermes_cli/test_mcp_config.py

  Show the diff

  • hermes_cli/mcp_config.py

    @@ -247,7 +247,7 @@ def _resolve_mcp_server_config(config: dict) -> dict:
     
     
     def _probe_single_server(
    -    name: str, config: dict, connect_timeout: float = 30, *, details: Optional[dict] = None
    +    name: str, config: dict, connect_timeout: Optional[float] = None, *, details: Optional[dict] = None
     ) -> List[Tuple[str, str]]:
         """Temporarily connect to one MCP server, list its tools, disconnect.
     
    @@ -271,9 +271,14 @@ def _probe_single_server(
         )
     
         config = _resolve_mcp_server_config(config)
    +    if connect_timeout is None:
    +        raw_timeout = config.get("connect_timeout", 30)
    +        try:
    +            connect_timeout = max(1.0, float(raw_timeout))
    +        except (TypeError, ValueError):
    +            connect_timeout = 30.0
     
         _ensure_mcp_loop()
    -
         tools_found: List[Tuple[str, str]] = []
     
         async def _probe():
    @@ -392,6 +397,7 @@ def cmd_mcp_add(args):
         auth_type = getattr(args, "auth", None)
         preset_name = getattr(args, "preset", None)
         raw_env = getattr(args, "env", None)
    +    raw_connect_timeout = getattr(args, "connect_timeout", None)
     
         server_config: Dict[str, Any] = {}
         try:
    @@ -437,6 +443,8 @@ def cmd_mcp_add(args):
                 server_config["args"] = cmd_args
             if explicit_env:
                 server_config["env"] = explicit_env
    +    if raw_connect_timeout is not None:
    +        server_config["connect_timeout"] = raw_connect_timeout
     
         issues = validate_mcp_server_entry(name, server_config)
         if issues:
    @@ -776,8 +784,21 @@ 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. Floor at 315s — the OAuth callback
    +    # window (300s in mcp_oauth) plus headroom — matching the GUI re-auth
    +    # path in web_server.py so CLI and dashboard behave identically.
         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, 315.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

  • hermes_cli/subcommands/mcp.py

    @@ -60,6 +60,11 @@ def build_mcp_parser(subparsers, *, cmd_mcp: Callable) -> None:
         )
         mcp_add_p.add_argument("--auth", choices=["oauth", "header"], help="Auth method")
         mcp_add_p.add_argument("--preset", help="Known MCP preset name")
    +    mcp_add_p.add_argument(
    +        "--connect-timeout",
    +        type=float,
    +        help="Timeout in seconds for initial connection and tool discovery",
    +    )
         mcp_add_p.add_argument(
             "--env",
             nargs="*",

  • hermes_cli/web_server.py

    @@ -9152,8 +9152,15 @@ def _run():
                     # The default 30s connect timeout would kill the flow while the
                     # user is still on the consent screen — give the browser
                     # round-trip the full callback window (300s in mcp_oauth) plus
    -                # headroom so the connect wrapper can't pre-empt it.
    -                tools = _probe_single_server(name, cfg, connect_timeout=315)
    +                # headroom so the connect wrapper can't pre-empt it. Honor a
    +                # larger configured connect_timeout when the user set one.
    +                try:
    +                    _cfg_timeout = float(cfg.get("connect_timeout", 0))
    +                except (TypeError, ValueError):
    +                    _cfg_timeout = 0.0
    +                tools = _probe_single_server(
    +                    name, cfg, connect_timeout=max(_cfg_timeout, 315)
    +                )
                 except Exception:
                     storage.restore(backup)
                     raise

  • scripts/release.py

    @@ -1204,6 +1204,8 @@
         "[redacted:email]": "r266-tech",
         "[redacted:email]": "r266-tech",  # PR #55780 salvage (dead-target not_found blast radius)
         "[redacted:email]": "BlackishGreen33",
    +    "[redacted:email]": "sberan",  # PR #54494 salvage (--connect-timeout flag on hermes mcp add)
    +    "[redacted:email]": "labsobsidian",  # PR #56699 salvage (MCP OAuth login connect_timeout floor)
         "[redacted:email]": "SaulJWu",
         "[redacted:email]": "hcshen0111",
         "[redacted:email]": "Cygra",

  • tests/hermes_cli/test_mcp_add_command_dest.py

    @@ -41,6 +41,7 @@ def _build_parser():
         mcp_add.add_argument("name")
         mcp_add.add_argument("--url")
         mcp_add.add_argument("--command", dest="mcp_command")
    +    mcp_add.add_argument("--connect-timeout", type=float)
         mcp_add.add_argument("--args", nargs=argparse.REMAINDER, default=[])
     
         return parser
    @@ -87,6 +88,25 @@ def test_bare_mcp_add_does_not_clobber_command(self):
             assert args.mcp_command is None
             assert args.url is None
     
    +    def test_connect_timeout_flag_sets_probe_timeout(self):
    +        """`--connect-timeout` exposes the per-server discovery timeout."""
    +        parser = _build_parser()
    +        args = parser.parse_args(
    +            [
    +                "mcp",
    +                "add",
    +                "slow",
    +                "--url",
    +                "https://example.com/mcp",
    +                "--connect-timeout",
    +                "180",
    +            ]
    +        )
    +
    +        assert args.command == "mcp"
    +        assert args.mcp_action == "add"
    +        assert args.connect_timeout == 180
    +
         def test_args_passthrough_keeps_nested_option_flags(self):
             """`--args` must keep command flags like Docker MCP's --profile."""
             parser = _build_parser()

  • tests/hermes_cli/test_mcp_config.py

    @@ -445,6 +445,44 @@ def mock_probe(name, config, **kw):
             assert "Connected" in out
             assert "Tools discovered: 2" in out
     
    +    def test_probe_uses_configured_connect_timeout(self, monkeypatch):
    +        """OAuth-capable probes must not hard-code a short 30s timeout."""
    +        import asyncio
    +        from hermes_cli import mcp_config
    +        import tools.mcp_tool as mcp_tool
    +
    +        captured = {}
    +
    +        class FakeServer:
    +            _tools = []
    +
    +            async def shutdown(self):
    +                captured["shutdown"] = True
    +
    +        async def fake_connect(name, config):
    +            return FakeServer()
    +
    +        def fake_run_on_mcp_loop(coro, timeout):
    +            captured["outer_timeout"] = timeout
    +            return asyncio.run(coro)
    +
    +        async def fake_wait_for(awaitable, timeout):
    +            captured["inner_timeout"] = timeout
    +            return await awaitable
    +
    +        monkeypatch.setattr(mcp_tool, "_ensure_mcp_loop", lambda: None)
    +        monkeypatch.setattr(mcp_tool, "_stop_mcp_loop_if_idle", lambda: None)
    +        monkeypatch.setattr(mcp_tool, "_connect_server", fake_connect)
    +        monkeypatch.setattr(mcp_tool, "_run_on_mcp_loop", fake_run_on_mcp_loop)
    +        monkeypatch.setattr(mcp_config.asyncio, "wait_for", fake_wait_for)
    +
    +        assert mcp_config._probe_single_server(
    +            "supabase", {"connect_timeout": 300}
    +        ) == []
    +        assert captured["inner_timeout"] == 300.0
    +        assert captured["outer_timeout"] == 310.0
    +        assert captured["shutdown"] is True
    +
     
     # ---------------------------------------------------------------------------
     # Tests: env var interpolation
    @@ -830,7 +868,9 @@ def test_login_false_success_no_token(self, tmp_path, capsys, monkeypatch):
             # 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
    @@ -852,7 +892,10 @@ def test_login_genuine_success_with_token(self, tmp_path, capsys, monkeypatch):
             # 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")]
    @@ -868,6 +911,9 @@ def mock_probe(name, cfg):
     
             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
     
     
     # ---------------------------------------------------------------------------