fix(mcp): replace global _oauth_port with per-provider closures

[HrushiYadav]

Summary

_oauth_port is a module-level global that gets overwritten each time build_oauth_auth() or MCPOAuthManager._build_provider() is called. When multiple MCP servers need OAuth, the last call wins and earlier providers read the wrong port from the global, causing their callback servers to bind the wrong port or cross-pollute each other's flows.

Changes

• Add a port keyword argument to _redirect_handler and _wait_for_callback. Both fall back to the module-level global when port is not provided, preserving backward compatibility.
• In build_oauth_auth and MCPOAuthManager._build_provider, capture the resolved port in closures (scoped_redirect, scoped_callback) so each provider carries its own port instead of reading the shared global.
• The legacy _oauth_port global is still set by _configure_callback_port for any external consumers, but the two primary construction paths no longer depend on it.
• Add a regression test (TestOAuthPortIsolation) that builds two providers back-to-back and verifies each closure passes the correct port to _redirect_handler.

Test plan

• New test test_concurrent_providers_use_different_ports passes
• All existing test_mcp_oauth.py tests pass (13 passed, 1 skipped on Windows)
• Existing SSH hint tests (TestRedirectHandlerSshHint) pass with the new port kwarg

Closes #44588

[liuhao1024]

Code Review — Clean ✅

Reviewed the diff (3 files, +85/-19 lines). Findings:

1. Per-provider port isolation is correct: The scoped_redirect and scoped_callback closures in build_oauth_auth capture resolved_port at creation time, preventing the global _oauth_port race when multiple MCP servers are configured concurrently.

2. Backward-compatible fallback: Both _redirect_handler and _wait_for_callback default port to None and fall back to _oauth_port, so existing callers that don't pass port still work.

3. _configure_callback_port return value: Verified the function now returns the resolved port (previously it only set the global). The return is consumed by resolved_port = _configure_callback_port(cfg) in both mcp_oauth.py and mcp_oauth_manager.py.

4. Test covers the key invariant: test_concurrent_providers_use_different_ports verifies back-to-back providers get different ports AND that closures capture their own port (not the final global). The _spy_redirect approach is clean.

LGTM — eliminates a subtle concurrency bug in multi-provider OAuth flows.

[HrushiYadav]

Friendly ping -- this fixes the global _oauth_port race condition with per-provider closures. Any feedback?

[HrushiYadav]

Rebased on latest main. Thanks @liuhao1024 for the review earlier. Ready whenever a maintainer can take a look.

[teknium1]

Thanks for isolating the per-provider callback state.

Problems

• The regression added in bd88c162351c67362c07bb846d7d5dfdf29d4b8a verifies only build_oauth_auth() redirect closures. It does not invoke the changed callback closures, and it does not cover the manager construction path at tools/mcp_oauth_manager.py:544-555, which is the production path used by tools/mcp_tool.py:2362-2367.

Suggested changes

• Add deterministic two-provider coverage for MCPOAuthManager and both handler types. Spy on _redirect_handler and _wait_for_callback, use explicit distinct redirect_port values, and assert each provider closure forwards its own port.

Current main still assigns the global at tools/mcp_oauth.py:828-833 and consumes it in the handlers at tools/mcp_oauth.py:575 and 655; the closure approach addresses that verified race.

Automated hermes-sweeper review.

[alt-glitch]

This was generated by AI during triage.

Duplicate of #5345 (earliest open PR taking the same per-provider-closure approach to the _oauth_port global race). Sibling PRs #44607 and #44872 take the identical closure-scoping mechanism and are also tracked against #5345. Marking duplicate for cluster/DB consistency — the maintainer can pick the canonical implementation. (Note: the distinct bind-first rewrite in #54325 is related, not a duplicate.)

[teknium1]

Resolved on main via PR #65622, which merged the same closure-factory approach (salvaged from #44872; #5345 by @caseyg was the earliest submission — all credited in the PR body). Thanks for the contribution @HrushiYadav!