fix(mcp): suppress interactive OAuth stdin prompts during background discovery (#35927) - #53294
Merged
Merged
Conversation
Contributor
🔎 Lint report:
|
| Rule | Count |
|---|---|
invalid-assignment |
2 |
First entries
tests/run_agent/test_credits_notices_toggle.py:76: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to attribute `_credits_session_start_micros` of type `int`
hermes_cli/mcp_startup.py:79: [invalid-assignment] invalid-assignment: Object of type `<class 'nullcontext'>` is not assignable to `() -> _GeneratorContextManager[Unknown, None, None]`
✅ Fixed issues (2):
| Rule | Count |
|---|---|
unresolved-attribute |
2 |
First entries
tests/run_agent/test_credits_notices_toggle.py:76: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_credits_session_start_micros` on type `AIAgent`
run_agent.py:3002: [unresolved-attribute] unresolved-attribute: Object of type `Self@get_credits_spent_micros` has no attribute `_credits_session_start_micros`
Unchanged: 6043 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
…discovery (#35927) When an MCP server requires OAuth, the interactive `hermes` TUI froze on startup: background MCP discovery hit the OAuth flow, which on an interactive TTY spawns a daemon thread doing a blocking `sys.stdin.readline()` (the "paste the redirect URL" fallback in mcp_oauth._wait_for_callback). That thread competes with the TUI's own stdin reader for the same terminal, so keystrokes get swallowed and the TUI appears frozen (up to the 300s OAuth timeout). Reported symptom: "MCP OAuth: authorization required / Open this URL ... the tui is freezing, not respond to typing." Add a thread-local `suppress_interactive_oauth()` context manager in tools/mcp_oauth.py; `_is_interactive()` returns False while it's active, so the stdin paste-thread and prompt are never created. Background discovery (hermes_cli/mcp_startup.py, tui_gateway/entry.py) now runs discovery inside that context, so OAuth-requiring servers soft-skip (raise OAuthNonInteractiveError, already handled) instead of stealing the TUI's stdin. A real `hermes mcp login` on the main thread is unaffected (thread-local). Salvaged from #35945 by @zapabob (authorship preserved via cherry-pick; resolved a conflict against main's new mcp_discovery_timeout / wait_for_mcp_ discovery refactor, keeping both). Verified E2E: with suppression the paste prompt is NOT printed and no stdin thread spawns (raises OAuthNonInteractive soft-skip); without it the prompt shows (the freeze). Mutation-verified (removing the suppress check in _is_interactive fails the regression test). 76 tests pass, ruff clean. Closes #35927. SELF-REVIEW FIX: the original #35945 used threading.local(), which does NOT propagate to the dedicated mcp-event-loop thread where OAuth actually runs (discover_mcp_tools dispatches the connect via run_coroutine_threadsafe), so the suppression was a NO-OP in production (the tests passed only by stubbing out the cross-thread dispatch). Converted to a contextvars.ContextVar, which asyncio copies onto the scheduled coroutine — empirically verified suppression now holds on the mcp-event-loop thread through the real _run_on_mcp_loop path. Added a cross-thread regression test (fails on threading.local, passes on the ContextVar) so the no-op can't regress.
kshitijk4poor
force-pushed
the
salvage/35927-mcp-oauth-stdin
branch
from
June 26, 2026 22:16
95441d8 to
280134b
Compare
kshitijk4poor
enabled auto-merge (rebase)
June 26, 2026 22:18
alt-glitch
added
type/bug
Collaborator
The backgrounding-contract test (test_prepare_agent_startup_backgrounds_ blocking_mcp_for_chat) failed intermittently on loaded CI shards: it stubs tools.mcp_tool.discover_mcp_tools but NOT tools.mcp_oauth, so the background discovery thread paid the real, cold ~0.75s 'import tools.mcp_oauth' (added by this PR's _discover_mcp_tools_without_interactive_oauth) before calling the stubbed discovery. On a slow/loaded runner that import plus thread scheduling exceeded the 1.0s polling deadline, leaving calls['mcp'] == 0. Fix: stub tools.mcp_oauth with a nullcontext suppress_interactive_oauth (the same no-op production falls back to when mcp_oauth is unavailable), so the test exercises the backgrounding contract without paying an unrelated cold import in its timing window. Bumped the poll deadline 1.0s -> 3.0s as belt-and-suspenders. Production behaviour is unchanged; the import cost was always off the main thread. Verified: 5/5 pass repeatedly via scripts/run_tests.sh (per-file isolation, matching CI), ruff clean.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The interactive
hermesTUI no longer freezes on startup when a configured MCP server requires OAuth.Root cause (#35927)
On startup, background MCP discovery connects to configured servers. When a server requires OAuth, the flow (
tools/mcp_oauth._wait_for_callback) — on an interactive TTY — spawns a daemon thread doing a blockingsys.stdin.readline()(the "paste the redirect URL" fallback that races the HTTP callback listener). That thread competes with the TUI's own stdin reader for the same terminal, so the user's keystrokes get swallowed and the TUI appears frozen (up to the 300s OAuth timeout). Reported symptom: "MCP OAuth: authorization required / Open this URL … the tui is freezing, not respond to typing."Fix
Add a thread-local
suppress_interactive_oauth()context manager (tools/mcp_oauth.py). While active,_is_interactive()returnsFalse, so the stdin paste-thread and its prompt are never created. Background discovery (hermes_cli/mcp_startup.py,tui_gateway/entry.py) now runs inside that context — an OAuth-requiring server soft-skips (raisesOAuthNonInteractiveError, already handled as "skip this server") instead of stealing the TUI's stdin. A genuinehermes mcp loginon the main thread is unaffected (thread-local, not global).Validation
_wait_for_callbackdoes not print the paste prompt and spawns no stdin thread — it raisesOAuthNonInteractiveError(soft skip), leaving the TUI's stdin untouched. Without suppression the prompt IS shown (the freeze). Demonstrated directly._is_interactive()fails the regression test.test_mcp_startup.py+test_mcp_oauth.py), incl. new tests for the context manager and the suppressed-discovery path. ruff clean.Salvage notes
Salvaged from #35945 by @zapabob (authorship preserved via cherry-pick; already in AUTHOR_MAP). Resolved a conflict against main's newer
mcp_discovery_timeout/wait_for_mcp_discoveryrefactor, keeping both that and the OAuth-suppression helper.Closes #35927.