Suppress OAuth prompts freezing MCP background discovery

Fixes on record

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

• #53294 fix(mcp): suppress interactive OAuth stdin prompts during background discovery (#35927)

  merged to main

  files: hermes_cli/mcp_startup.py, tests/hermes_cli/test_mcp_startup.py, tests/tools/test_mcp_oauth.py, tools/mcp_oauth.py, tui_gateway/entry.py

  Show the diff

  • hermes_cli/mcp_startup.py

    @@ -3,6 +3,7 @@
     from __future__ import annotations
     
     import threading
    +from contextlib import nullcontext
     from typing import Optional
     
     _mcp_discovery_lock = threading.Lock()
    @@ -36,9 +37,7 @@ def start_background_mcp_discovery(*, logger, thread_name: str) -> None:
     
             def _discover() -> None:
                 try:
    -                from tools.mcp_tool import discover_mcp_tools
    -
    -                discover_mcp_tools()
    +                _discover_mcp_tools_without_interactive_oauth()
                 except Exception:
                     logger.debug("Background MCP tool discovery failed", exc_info=True)
     
    @@ -72,6 +71,19 @@ def _resolve_discovery_timeout(explicit: "float | None") -> float:
             return 1.5
     
     
    +def _discover_mcp_tools_without_interactive_oauth() -> None:
    +    """Run MCP discovery without letting OAuth read from the user's stdin."""
    +    try:
    +        from tools.mcp_oauth import suppress_interactive_oauth
    +    except Exception:
    +        suppress_interactive_oauth = nullcontext
    +
    +    with suppress_interactive_oauth():
    +        from tools.mcp_tool import discover_mcp_tools
    +
    +        discover_mcp_tools()
    +
    +
     def wait_for_mcp_discovery(timeout: "float | None" = None) -> None:
         """Wait for background MCP discovery before the first tool snapshot.
     

  • tests/hermes_cli/test_mcp_startup.py

    @@ -3,6 +3,7 @@
     from __future__ import annotations
     
     from argparse import Namespace
    +from contextlib import nullcontext
     import sys
     import threading
     import time
    @@ -70,6 +71,16 @@ def _blocking_discover():
             "agent.shell_hooks",
             types.SimpleNamespace(register_from_config=lambda *_a, **_k: None),
         )
    +    # Stub mcp_oauth so the background thread doesn't pay the real (cold,
    +    # ~0.75s) ``tools.mcp_oauth`` import before calling discovery. This test
    +    # asserts the *backgrounding contract* (main thread returns fast, discovery
    +    # runs off-thread), not OAuth suppression — the unrelated import latency
    +    # would otherwise blow the polling deadline on a loaded CI runner.
    +    monkeypatch.setitem(
    +        sys.modules,
    +        "tools.mcp_oauth",
    +        types.SimpleNamespace(suppress_interactive_oauth=lambda: nullcontext()),
    +    )
         monkeypatch.setitem(
             sys.modules,
             "tools.mcp_tool",
    @@ -81,6 +92,9 @@ def _blocking_discover():
             main_mod._prepare_agent_startup(_agent_args())
             elapsed = time.monotonic() - start
             assert elapsed < 0.2
    +        deadline = time.monotonic() + 3.0
    +        while calls["mcp"] == 0 and time.monotonic() < deadline:
    +            time.sleep(0.01)
             assert calls["mcp"] == 1
             assert mcp_startup._mcp_discovery_thread is not None
             assert mcp_startup._mcp_discovery_thread.is_alive()
    @@ -88,6 +102,50 @@ def _blocking_discover():
             stop.set()
     
     
    +def test_background_mcp_discovery_suppresses_interactive_oauth(monkeypatch):
    +    state = {"active": False, "during_discover": None}
    +
    +    class SuppressInteractiveOAuth:
    +        def __enter__(self):
    +            state["active"] = True
    +
    +        def __exit__(self, *_exc):
    +            state["active"] = False
    +
    +    def _discover():
    +        state["during_discover"] = state["active"]
    +
    +    monkeypatch.setitem(
    +        sys.modules,
    +        "hermes_cli.config",
    +        types.SimpleNamespace(
    +            read_raw_config=lambda: {"mcp_servers": {"demo": {"url": "https://mcp.example.test/mcp"}}},
    +        ),
    +    )
    +    monkeypatch.setitem(
    +        sys.modules,
    +        "tools.mcp_oauth",
    +        types.SimpleNamespace(
    +            suppress_interactive_oauth=lambda: SuppressInteractiveOAuth(),
    +        ),
    +    )
    +    monkeypatch.setitem(
    +        sys.modules,
    +        "tools.mcp_tool",
    +        types.SimpleNamespace(discover_mcp_tools=_discover),
    +    )
    +
    +    mcp_startup.start_background_mcp_discovery(
    +        logger=types.SimpleNamespace(debug=lambda *_a, **_k: None),
    +        thread_name="test-mcp-discovery",
    +    )
    +    assert mcp_startup._mcp_discovery_thread is not None
    +    mcp_startup._mcp_discovery_thread.join(timeout=1.0)
    +
    +    assert state["during_discover"] is True
    +    assert state["active"] is False
    +
    +
     def test_prepare_agent_startup_skips_mcp_bootstrap_for_tui_chat(monkeypatch):
         calls = {"mcp": 0}
     

  • tests/tools/test_mcp_oauth.py

    @@ -466,6 +466,63 @@ def test_false_when_stdin_has_no_isatty(self, monkeypatch):
             monkeypatch.setattr("tools.mcp_oauth.sys.stdin", mock_stdin)
             assert _is_interactive() is False
     
    +    def test_suppress_interactive_oauth_disables_stdin_prompts(self, monkeypatch):
    +        import tools.mcp_oauth as mod
    +
    +        mock_stdin = MagicMock()
    +        mock_stdin.isatty.return_value = True
    +        monkeypatch.setattr("tools.mcp_oauth.sys.stdin", mock_stdin)
    +
    +        assert _is_interactive() is True
    +        with mod.suppress_interactive_oauth():
    +            assert _is_interactive() is False
    +        assert _is_interactive() is True
    +
    +    def test_suppression_propagates_across_run_coroutine_threadsafe(self, monkeypatch):
    +        """#35927 core: suppression set on the discovery thread MUST reach the
    +        coroutine asyncio runs on a *different* (event-loop) thread — that is
    +        where the OAuth callback / _is_interactive() actually executes via
    +        run_coroutine_threadsafe. A threading.local would NOT propagate here
    +        (the original fix's defect); a ContextVar does."""
    +        import asyncio
    +        import threading
    +        import tools.mcp_oauth as mod
    +
    +        mock_stdin = MagicMock()
    +        mock_stdin.isatty.return_value = True
    +        monkeypatch.setattr("tools.mcp_oauth.sys.stdin", mock_stdin)
    +
    +        loop = asyncio.new_event_loop()
    +        loop_thread = threading.Thread(target=loop.run_forever, daemon=True)
    +        loop_thread.start()
    +        result = {}
    +        try:
    +            async def _probe_on_loop_thread():
    +                # runs on the loop thread, NOT the one that set suppression
    +                return (threading.current_thread() is not discovery_thread,
    +                        _is_interactive())
    +
    +            discovery_thread = None
    +
    +            def _discovery():
    +                nonlocal discovery_thread
    +                discovery_thread = threading.current_thread()
    +                with mod.suppress_interactive_oauth():
    +                    fut = asyncio.run_coroutine_threadsafe(
    +                        _probe_on_loop_thread(), loop
    +                    )
    +                    result["cross_thread"], result["interactive"] = fut.result(timeout=5)
    +
    +            dt = threading.Thread(target=_discovery)
    +            dt.start()
    +            dt.join()
    +        finally:
    +            loop.call_soon_threadsafe(loop.stop)
    +
    +        assert result["cross_thread"] is True, "probe must run on the loop thread"
    +        # The whole point: suppression must hold on the loop thread.
    +        assert result["interactive"] is False
    +
     
     class TestWaitForCallbackNoBlocking:
         """_wait_for_callback() must never call input() — it raises instead."""
    @@ -753,6 +810,26 @@ async def instant_sleep(_):
             err = capsys.readouterr().err
             assert "paste the redirect URL" not in err
     
    +    def test_paste_prompt_NOT_shown_when_interactivity_suppressed(self, monkeypatch, capsys):
    +        """Background MCP discovery must not race the CLI/TUI stdin reader."""
    +        import tools.mcp_oauth as mod
    +
    +        mod._oauth_port = _find_free_port()
    +        mock_stdin = MagicMock()
    +        mock_stdin.isatty.return_value = True
    +        monkeypatch.setattr(mod.sys, "stdin", mock_stdin)
    +
    +        async def instant_sleep(_):
    +            pass
    +
    +        with patch.object(mod.asyncio, "sleep", instant_sleep):
    +            with mod.suppress_interactive_oauth():
    +                with pytest.raises(OAuthNonInteractiveError):
    +                    asyncio.run(_wait_for_callback())
    +        err = capsys.readouterr().err
    +        assert "paste the redirect URL" not in err
    +        mock_stdin.readline.assert_not_called()
    +
     
     class TestPasteCallbackSkipToken:
         """User can type `skip` (or similar) at the paste prompt to bail out."""

  • tools/mcp_oauth.py

    @@ -33,6 +33,7 @@
     """
     
     import asyncio
    +import contextvars
     import json
     import logging
     import os
    @@ -44,6 +45,7 @@
     import threading
     import time
     import webbrowser
    +from contextlib import contextmanager
     from http.server import BaseHTTPRequestHandler, HTTPServer
     from pathlib import Path
     from typing import Any
    @@ -92,6 +94,15 @@ class OAuthNonInteractiveError(RuntimeError):
     # Port used by the most recent build_oauth_auth() call.  Exposed so that
     # tests can verify the callback server and the redirect_uri share a port.
     _oauth_port: int | None = None
    +# Interactivity gate for OAuth stdin prompts. A ContextVar (NOT threading.local)
    +# is required: background MCP discovery sets this on the discovery thread, but
    +# the actual connect+OAuth runs on the dedicated `mcp-event-loop` thread via
    +# run_coroutine_threadsafe. asyncio copies the *calling context* into the
    +# scheduled coroutine, so a ContextVar propagates across that boundary while a
    +# threading.local would not — see #35927. Default True (interactive allowed).
    +_oauth_interactive_enabled: "contextvars.ContextVar[bool]" = contextvars.ContextVar(
    +    "_oauth_interactive_enabled", default=True
    +)
     
     
     # Skip tokens accepted at the paste prompt — exit OAuth without auth.
    @@ -137,12 +148,30 @@ def _find_free_port() -> int:
     
     def _is_interactive() -> bool:
         """Return True if we can reasonably expect to interact with a user."""
    +    if not _oauth_interactive_enabled.get():
    +        return False
         try:
             return sys.stdin.isatty()
         except (AttributeError, ValueError):
             return False
     
     
    +@contextmanager
    +def suppress_interactive_oauth():
    +    """Disable stdin-based OAuth prompts for the current execution context.
    +
    +    Uses a ContextVar so the suppression propagates from a background-discovery
    +    thread onto the coroutine scheduled (via run_coroutine_threadsafe) on the
    +    dedicated MCP event-loop thread — where the OAuth callback actually runs
    +    (#35927). A threading.local would not cross that thread boundary.
    +    """
    +    token = [redacted:secret-assignment]
    +    try:
    +        yield
    +    finally:
    +        _oauth_interactive_enabled.reset(token)
    +
    +
     def _can_open_browser() -> bool:
         """Return True if opening a browser is likely to work."""
         # Explicit SSH session → no local display

  • tui_gateway/entry.py

    @@ -293,8 +293,11 @@ def main():
         if _has_mcp_servers:
             def _discover_mcp_background() -> None:
                 try:
    -                from tools.mcp_tool import discover_mcp_tools
    -                discover_mcp_tools()
    +                from hermes_cli.mcp_startup import (
    +                    _discover_mcp_tools_without_interactive_oauth,
    +                )
    +
    +                _discover_mcp_tools_without_interactive_oauth()
                 except Exception:
                     logger.warning(
                         "Background MCP tool discovery failed", exc_info=True