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

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,13 +92,60 @@ 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()
     finally:
         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 = _oauth_interactive_enabled.set(False)
+    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