fix(oauth): narrow context.lock scope in async_auth_flow (rebase of #2660 by @peisuke, closes #2847)

[Bartok9]

Summary

Conflict-free salvage of #2660 by @peisuke onto current main, opened as a fresh, mergeable PR so the fix can land without waiting on the original draft to be un-conflicted. All credit for the fix goes to @peisuke — the commit is authored by them; I only resolved the rebase against main and squashed to a single commit.

Closes #2847. Addresses the same RuntimeError: The current task is not holding this lock reported in #2644 and #2847.

Why a new PR

#2660 has been a CONFLICTING draft since May 22 with no maintainer review. Issue #2847 (June 12) is a second, independent production repro of the exact bug, and the reporter explicitly asks for #2660 to be reviewed/merged. Rather than let the fix keep sitting, this PR provides a ready-to-merge version. If maintainers prefer to merge #2660 directly, or @peisuke wants to push the rebase themselves, this PR can be closed — the goal is just to unblock the fix.

The bug

OAuthContext.lock is an anyio.Lock, which records task identity at acquire() and enforces same-task release(). async_auth_flow holds this lock across yield points. When httpx drives the generator from a different task during concurrent OAuth connections (e.g. Notion's frequent token refresh behind a gateway), release() raises RuntimeError: The current task is not holding this lock.

The fix (@peisuke's approach)

Narrow the lock scope so no HTTP yield (including the long-poll GET SSE and the token-refresh round trips) runs while holding context.lock, plus a single-flight refresh_lock with a re-check under the lock. Keeps trio portability (unlike swapping to asyncio.Lock).

Verification

Rebased onto current main. Single commit (authored by @peisuke), touches only src/mcp/client/auth/oauth2.py and tests/client/test_auth.py (+348/-18).

$ uv run pytest tests/client/test_auth.py -q
131 passed, 1 xfailed

$ uv run ruff check src/mcp/client/auth/oauth2.py tests/client/test_auth.py
All checks passed!

[ctaylor86]

I reproduced this downstream with MCP SDK 1.28.1 against a production Streamable HTTP OAuth server: OAuth and initialize succeeded, but a session-long GET held OAuthContext.lock, so the subsequent tools/list POST never reached the network and timed out. Supplying the same stored bearer token directly listed 171 tools immediately.

I opened the v1.x backport in #3243 and validated it against that server plus two existing MCP connections. While testing, I found two details that may also be relevant to this main PR:

1. The final resource retries after the 401/403 branches must also yield outside the provider-state critical section; they can themselves be session-long GET requests.
2. anyio.Lock is task-owned. HTTPX can resume or close an auth async generator from a different task, reproducing RuntimeError: The current task is not holding this lock even for bounded OAuth requests. fix(oauth): avoid lock contention for long-running requests #3243 uses task-agnostic bounded semaphores for critical sections and includes a cross-task generator-close regression test.

The backport also serializes refresh and authorization transitions under one semaphore, re-checks tokens after acquiring it, and restores each request's MCP protocol version before RFC 8707 request construction.

[Bartok9]

Thanks @ctaylor86 — really helpful production repro and the v1.x work in #3243.

Agree on both points for main as well:

1. Final resource retries after 401/403 must yield outside the provider-state critical section. Session-long GETs on those paths would reintroduce the same lock-hold / starvation you hit (initialize OK, then tools/list blocked behind the long GET).
2. Task-owned anyio.Lock is still a footgun even with narrowed scope if HTTPX resumes or closes the auth generator from another task. Task-agnostic bounded semaphores + a cross-task generator-close regression (as in fix(oauth): avoid lock contention for long-running requests #3243) look like the right hardening beyond the original fix(oauth): narrow context.lock scope in async_auth_flow #2660 narrow-scope fix.

This PR is a conflict-free salvage of @peisuke's #2660 onto main (credit stays with them). Your backport goes further on semaphore choice, single-flight refresh/auth transitions, protocol-version restore before RFC 8707 construction, and live validation — maintainers may want to land the fuller design on main via #3243's approach (or cherry-picks) rather than only the narrower lock-scope change here.

Happy to fold (1)/(2) into this branch if that's the preferred vehicle for main; otherwise I'll defer to whatever landing path maintainers pick between #2858 / #3243 / #2660. Appreciate the downstream proof (171 tools once the lock wasn't held across the session GET).