-
Notifications
You must be signed in to change notification settings - Fork 52.6k
fix(mcp-oauth): graceful fallback when OAuth callback port is already bound #55741
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weβll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -650,19 +650,18 @@ async def _wait_for_callback() -> tuple[str, str | None]: | |
| # We just need to poll for the result. | ||
| handler_cls, result = _make_callback_handler() | ||
|
|
||
| # Start a temporary server on the known port | ||
| # Start a temporary server on the known port. The MCP SDK may have | ||
| # already started its own callback server on the same port β if so, | ||
| # we just poll for the result without creating a second one. | ||
| server = None | ||
| try: | ||
| server = HTTPServer(("127.0.0.1", _oauth_port), handler_cls) | ||
| server_thread = threading.Thread(target=server.handle_request, daemon=True) | ||
| server_thread.start() | ||
| except OSError: | ||
| # Port already in use β the server from build_oauth_auth is running. | ||
| # Fall back to polling the server started by build_oauth_auth. | ||
| raise OAuthNonInteractiveError( | ||
| "OAuth callback timed out β could not bind callback port. " | ||
| "Complete the authorization in a browser first, then retry." | ||
| ) | ||
|
|
||
| server_thread = threading.Thread(target=server.handle_request, daemon=True) | ||
| server_thread.start() | ||
| # Port already in use β the server from the MCP SDK is running. | ||
| # Fall back to polling: the SDK's server will handle the callback. | ||
| pass | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
|
|
||
| # Optional paste-fallback thread: only on interactive TTYs. Reads one | ||
| # line from stdin and writes the parsed code/state into the shared | ||
|
|
@@ -692,7 +691,8 @@ async def _wait_for_callback() -> tuple[str, str | None]: | |
| await asyncio.sleep(poll_interval) | ||
| elapsed += poll_interval | ||
| finally: | ||
| server.server_close() | ||
| if server is not None: | ||
| server.server_close() | ||
|
|
||
| if result["error"] == _USER_SKIPPED_SENTINEL: | ||
| raise OAuthNonInteractiveError("user_skipped") | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is
cmd_mcp_add, not thehermes mcp loginpath described by the PR. Current main already grants interactive login at least 315 seconds in_reauth_oauth_server(hermes_cli/mcp_config.py:794-801); extending every add-time probe to 600 seconds changes unrelated discovery failures.