Skip to content

Commit db801ad

Browse files
peisukeBartok9
authored andcommitted
fix(oauth): cover Python 3.10/3.11 partial-branch coverage gap
On Python 3.10/3.11, coverage.py (sys.settrace backend) misattributes the False arm of compound boolean predicates inside an async with block in an async generator, leading to spurious partial-branch warnings on 4 lines in async_auth_flow: - line 536: `if not is_token_valid() and can_refresh_token():` (Phase 1) - line 546: same (Phase 2 double-check inside refresh_lock) - line 548: `if refresh_request is not None:` (re-check skip path) - line 553: `if not await _handle_refresh_response(...):` (refresh success path) Python 3.12+ (sys.monitoring) tracks these branches correctly. Add `# pragma: no branch` to the 4 lines as a workaround for the legacy backend only. An inline comment block above the Phase 1 if documents the rationale. Also add 2 unit tests that explicitly exercise the affected branches so the coverage drop is shown to be a tool-precision issue, not missing tests: - test_phase1_skips_refresh_when_token_valid: Phase 1 sees is_token_valid=True and skips Phase 2 entirely. - test_refresh_success_proceeds_to_phase3_without_resetting_initialized: _handle_refresh_response returns True (HTTP 200) so the _initialized reset is skipped and Phase 3 proceeds with the fresh token. Local verification (Python 3.10): pytest tests/client/test_auth.py -n auto + coverage report → 99.43% (was 98.30%, BrPart 1, Missing only line 206 which is outside the test_auth.py scope). Full suite should reach 100%.
1 parent 148ec42 commit db801ad

2 files changed

Lines changed: 76 additions & 4 deletions

File tree

src/mcp/client/auth/oauth2.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -532,7 +532,12 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
532532
# Capture protocol version from request headers
533533
self.context.protocol_version = request.headers.get(MCP_PROTOCOL_VERSION)
534534

535-
if not self.context.is_token_valid() and self.context.can_refresh_token():
535+
# pragma: no branch — coverage.py on Python 3.10/3.11 (sys.settrace
536+
# backend) cannot reliably track both arms of compound boolean
537+
# predicates inside an ``async with`` block in an async generator.
538+
# Python 3.12+ (sys.monitoring) handles this correctly; the pragmas
539+
# below are workarounds for the legacy backend only.
540+
if not self.context.is_token_valid() and self.context.can_refresh_token(): # pragma: no branch
536541
needs_refresh = True
537542

538543
# === Phase 2: single-flight token refresh (yield outside context.lock) ===
@@ -542,14 +547,14 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
542547
# refreshed while we were waiting on refresh_lock.
543548
refresh_request: httpx.Request | None = None
544549
async with self.context.lock:
545-
if not self.context.is_token_valid() and self.context.can_refresh_token():
550+
if not self.context.is_token_valid() and self.context.can_refresh_token(): # pragma: no branch
546551
refresh_request = await self._refresh_token()
547-
if refresh_request is not None:
552+
if refresh_request is not None: # pragma: no branch
548553
# yield runs outside any lock so a long network round trip
549554
# does not block unrelated concurrent requests.
550555
refresh_response = yield refresh_request
551556
async with self.context.lock:
552-
if not await self._handle_refresh_response(refresh_response):
557+
if not await self._handle_refresh_response(refresh_response): # pragma: no branch
553558
# Refresh failed; fall through to 401 handling below.
554559
self._initialized = False
555560

tests/client/test_auth.py

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2836,3 +2836,70 @@ def fake_is_token_valid(self: object) -> bool:
28362836
await flow.asend(httpx.Response(200, request=yielded))
28372837
finally:
28382838
monkeypatch.setattr(oauth_provider.context.__class__, "is_token_valid", original_is_valid)
2839+
2840+
2841+
@pytest.mark.anyio
2842+
async def test_phase1_skips_refresh_when_token_valid(oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken):
2843+
"""Phase 1 branch where ``is_token_valid()`` is True so ``needs_refresh`` stays
2844+
False and Phase 2 is skipped entirely (covers oauth2.py 536->540).
2845+
"""
2846+
oauth_provider.context.current_tokens = valid_tokens
2847+
oauth_provider.context.token_expiry_time = time.time() + 3600 # valid
2848+
oauth_provider.context.client_info = OAuthClientInformationFull(
2849+
client_id="test_client_id",
2850+
client_secret="test_client_secret",
2851+
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
2852+
)
2853+
oauth_provider._initialized = True
2854+
2855+
request = httpx.Request("POST", "https://api.example.com/v1/mcp")
2856+
flow = oauth_provider.async_auth_flow(request)
2857+
# No refresh yield: Phase 1 sees valid token, skips Phase 2 (536->540 False branch).
2858+
yielded = await flow.__anext__()
2859+
assert yielded.method == "POST"
2860+
assert yielded.headers.get("Authorization") == "Bearer test_access_token"
2861+
with contextlib.suppress(StopAsyncIteration):
2862+
await flow.asend(httpx.Response(200, request=yielded))
2863+
2864+
2865+
@pytest.mark.anyio
2866+
async def test_refresh_success_proceeds_to_phase3_without_resetting_initialized(
2867+
oauth_provider: OAuthClientProvider, valid_tokens: OAuthToken
2868+
):
2869+
"""After a successful refresh, ``_handle_refresh_response`` returns True so the
2870+
``_initialized = False`` reset is skipped and Phase 3 proceeds with the fresh
2871+
token (covers oauth2.py 553->558 branch where the False arm is taken).
2872+
"""
2873+
oauth_provider.context.current_tokens = valid_tokens
2874+
oauth_provider.context.token_expiry_time = time.time() - 100 # expired
2875+
oauth_provider.context.client_info = OAuthClientInformationFull(
2876+
client_id="test_client_id",
2877+
client_secret="test_client_secret",
2878+
redirect_uris=[AnyUrl("http://localhost:3030/callback")],
2879+
)
2880+
oauth_provider._initialized = True
2881+
2882+
request = httpx.Request("POST", "https://api.example.com/v1/mcp")
2883+
flow = oauth_provider.async_auth_flow(request)
2884+
# Phase 2 yields the refresh request.
2885+
refresh_request = await flow.__anext__()
2886+
assert "grant_type=refresh_token" in refresh_request.read().decode()
2887+
2888+
# 200 OK with a parseable token body → _handle_refresh_response returns True.
2889+
refresh_response = httpx.Response(
2890+
200,
2891+
content=(
2892+
b'{"access_token": "fresh_access_token", "token_type": "Bearer", '
2893+
b'"expires_in": 3600, "refresh_token": "fresh_refresh_token"}'
2894+
),
2895+
request=refresh_request,
2896+
)
2897+
actual_request = await flow.asend(refresh_response)
2898+
# Phase 3 yields the *original* request with the fresh Authorization header.
2899+
assert actual_request.method == "POST"
2900+
assert actual_request.headers.get("Authorization") == "Bearer fresh_access_token"
2901+
# _initialized must NOT have been reset (the True branch of _handle_refresh_response).
2902+
assert oauth_provider._initialized is True
2903+
2904+
with contextlib.suppress(StopAsyncIteration):
2905+
await flow.asend(httpx.Response(200, request=actual_request))

0 commit comments

Comments
 (0)