fix(mcp): use per-provider closures and allow_reuse_address for OAuth

[Code-suphub]

Summary

Fixes #44588
Fixes #44590

Two related OAuth reliability fixes:

1. Cross-server state pollution (#44588)

_redirect_handler was a module-level function reading the global _oauth_port. When multiple MCP servers run OAuth concurrently, the later call overwrites the port, causing the wrong authorization_url to be returned.

Fix: Replace _redirect_handler with _make_redirect_handler(port) — a closure factory that closes over the per-provider resolved port. Updated both mcp_oauth.py and mcp_oauth_manager.py call sites.

2. OSError: Address already in use on repeated OAuth flows (#44590)

_wait_for_callback() creates a new HTTPServer per flow without allow_reuse_address. After the first flow, the socket stays in TIME_WAIT and the next flow's bind() fails.

Fix: Set server.allow_reuse_address = True on the ephemeral callback HTTPServer.

Changes

File	Change
tools/mcp_oauth.py	Replace _redirect_handler with _make_redirect_handler(port) closure; add allow_reuse_address = True
tools/mcp_oauth_manager.py	Import and use _make_redirect_handler instead of _redirect_handler
tests/tools/test_mcp_oauth.py	Update tests to use the new factory API

Testing

python -m pytest tests/tools/test_mcp_oauth.py -v -k "redirect"

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)
• ✅ Tests (adding or improving test coverage)

Checklist

Code

• I have read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope): description)
• I searched for existing PRs to make sure this is not a duplicate
• My PR contains only changes related to this fix (no unrelated commits)
• I have considered cross-platform impact (Windows, macOS, Linux) per the compatibility guide

Documentation

• I have updated relevant documentation or marked as N/A
• I have updated cli-config.yaml.example if I added/changed config keys or marked as N/A

[liuhao1024]

Dead code after return in _make_redirect_handler

The closure refactor moves the browser-opening logic into the inner _redirect_handler, but the old standalone browser block was left in place after return _redirect_handler — making it unreachable dead code:

def _make_redirect_handler(port: int):
    async def _redirect_handler(authorization_url: str) -> None:
        # ... SSH hint + browser logic now lives here ...
        if _can_open_browser():
            try:
                opened = webbrowser.open(authorization_url)
                ...
            except Exception:
                ...
        else:
            print("(Headless environment detected ...")

    return _redirect_handler

    # DEAD CODE — unreachable after the return above
    if _can_open_browser():
        try:
            opened = webbrowser.open(authorization_url)
            ...

This does not affect correctness (the closure handles browser opening), but it is ~10 lines of unreachable code that should be removed.

Everything else looks clean — the closure-based port isolation, allow_reuse_address = True, and the mcp_oauth_manager.py integration are all correct.

[Code-suphub]

Good catch! The dead code has been removed in the latest push. The return _redirect_handler is now the last statement in the factory function — no unreachable browser block remaining.

Thanks for the thorough review!

[alt-glitch]

Duplicate of #5345 (earliest open) — same per-provider closure approach for the MCP OAuth _oauth_port cross-server pollution (#44588). #44607 / #44685 take the same approach. The allow_reuse_address half (#44590) is also covered by #44611.

[tonydwb]

Clean fix. Converts _redirect_handler to _make_redirect_handler closure to avoid module-level _oauth_port state that caused cross-server pollution when multiple MCP servers run OAuth concurrently. Fixes #44588. Test updated accordingly. No issues found.

[teknium1]

Thanks for tackling two real MCP OAuth reliability failures. The current main branch still has shared callback-port state, but this patch needs adaptation before it can safely fix it.

Problems

• tools/mcp_oauth.py:496 sets allow_reuse_address after HTTPServer(...) has already bound. Python's TCPServer.__init__ calls server_bind() during construction, and server_bind() checks the flag before binding, so this does not enable reuse for the attempted bind.
• The new redirect closure does not isolate callbacks: the PR still passes _wait_for_callback at tools/mcp_oauth.py:791 and tools/mcp_oauth_manager.py:447. That handler reads module-global _oauth_port; current main still assigns that global in tools/mcp_oauth.py:828-832.
• The test changes only exercise SSH redirect text. They do not cover manager-built providers with distinct ports or a real fixed-port rebind.

Suggested changes

• Use an HTTPServer subclass with allow_reuse_address = True set before construction.
• Pass per-provider callback and redirect closures through MCPOAuthManager._build_provider.
• Add manager-path concurrency and real socket-rebind regression tests.

This is an automated hermes-sweeper review.

[teknium1]

HTTPServer.__init__ already calls server_bind() before this assignment, so SO_REUSEADDR is not enabled for this bind. Use a dedicated HTTPServer subclass with allow_reuse_address = True as a class attribute.

[teknium1]

This closure isolates only the redirect display path; the provider still receives global-dependent _wait_for_callback below. A concurrent provider can overwrite _oauth_port before this flow binds its callback listener.

[teknium1]

Merged on main via PR #65622 with your two commits cherry-picked and your authorship preserved (13e19a9, f4c7caa) — thanks @Code-suphub! One correctness note: allow_reuse_address had to be set BEFORE binding (the constructor had already bound, so setting it after was a no-op) — fixed during salvage via bind_and_activate=False. The TOCTOU half from #22161 was folded in on top. Closing since the branch itself was too stale to merge directly.