From 454d553d343eec197f70a6151e43977b7bae7d49 Mon Sep 17 00:00:00 2001 From: doxe0x Date: Mon, 22 Jun 2026 11:32:30 +0000 Subject: [PATCH] fix(mcp): report a clear error when the OAuth callback port is in use _wait_for_callback catches OSError on bind with a comment claiming the port is held by a server build_oauth_auth started, and promising to fall back to polling it. build_oauth_auth never starts a callback server (this is the only listener), so there is nothing to poll: the branch just raised a misleading "OAuth callback timed out" when the real cause is a busy port (a concurrent login, a leftover listener, or a fixed oauth.redirect_port that collided). Fix the stale comment and raise an accurate, actionable message (names the port, suggests freeing it or setting a free oauth.redirect_port), chained from the original OSError. Behavior is otherwise unchanged. Adds a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/tools/test_mcp_oauth.py | 19 +++++++++++++++++++ tools/mcp_oauth.py | 16 ++++++++++------ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/tests/tools/test_mcp_oauth.py b/tests/tools/test_mcp_oauth.py index 4f19ec33d..55f92f070 100644 --- a/tests/tools/test_mcp_oauth.py +++ b/tests/tools/test_mcp_oauth.py @@ -1470,3 +1470,22 @@ class TestPoisonClientRegistration: monkeypatch.setenv("HERMES_HOME", str(tmp_path)) storage = HermesTokenStorage("srv") assert storage.poison_client_registration() is False + + +def test_wait_for_callback_port_in_use_reports_clear_error(monkeypatch): + """A busy loopback callback port surfaces a clear 'already in use' error, + not a misleading 'timed out'. Guards the stale-comment fix where the branch + also wrongly claimed build_oauth_auth had started a server to poll.""" + import tools.mcp_oauth as mo + + monkeypatch.setattr(mo, "_is_interactive", lambda: True) + with patch.object(mo, "_oauth_port", 54321), patch.object( + mo, "HTTPServer", side_effect=OSError("address already in use") + ): + with pytest.raises(mo.OAuthNonInteractiveError) as excinfo: + asyncio.run(mo._wait_for_callback()) + + msg = str(excinfo.value) + assert "54321" in msg + assert "already in use" in msg + assert "timed out" not in msg diff --git a/tools/mcp_oauth.py b/tools/mcp_oauth.py index bd41b09ef..b905ca3de 100644 --- a/tools/mcp_oauth.py +++ b/tools/mcp_oauth.py @@ -736,13 +736,17 @@ async def _wait_for_callback() -> tuple[str, str | None]: server.allow_reuse_address = True server.server_bind() server.server_activate() - 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. + except OSError as exc: + # The loopback callback port is genuinely in use: a concurrent OAuth + # flow, a leftover listener, or a fixed `oauth.redirect_port` that + # collided. build_oauth_auth does not start its own callback server, + # so there is nothing to poll here; surface a clear, actionable error + # instead of a misleading "timed out". raise OAuthNonInteractiveError( - "OAuth callback timed out — could not bind callback port. " - "Complete the authorization in a browser first, then retry." - ) + f"OAuth callback port {_oauth_port} is already in use ({exc}). " + "Close any other in-progress login, or set a free `oauth.redirect_port` " + "in the server config, then retry." + ) from exc server_thread = threading.Thread(target=server.handle_request, daemon=True) server_thread.start()