Skip to content

Commit ed9b2d6

Browse files
authored
Stop counting an interactive OAuth login against request timeouts (#3635)
1 parent d5cebd1 commit ed9b2d6

7 files changed

Lines changed: 538 additions & 13 deletions

File tree

‎src/mcp/client/auth/oauth2.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
validate_metadata_issuer,
4444
)
4545
from mcp.shared._httpx_utils import RedirectAwareAuth, redirect_note
46+
from mcp.shared._request_clock import waiting_on_a_person
4647
from mcp.shared.auth import (
4748
AuthorizationCodeResult,
4849
OAuthClientInformationFull,
@@ -425,10 +426,9 @@ async def _perform_authorization_code_grant(self) -> tuple[str, str]:
425426
auth_params["prompt"] = "consent"
426427

427428
authorization_url = f"{auth_endpoint}?{urlencode(auth_params)}"
428-
await self.context.redirect_handler(authorization_url)
429-
430-
# Wait for callback
431-
result = await self.context.callback_handler()
429+
with waiting_on_a_person():
430+
await self.context.redirect_handler(authorization_url)
431+
result = await self.context.callback_handler()
432432

433433
if result.state is None or not secrets.compare_digest(result.state, state):
434434
raise OAuthFlowError(f"State parameter mismatch: {result.state} != {state}")

‎src/mcp/shared/_request_clock.py‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
"""A request's timeout measures the peer, so its clock stops while the request waits on a person.
2+
3+
The clock travels in a `ContextVar`: the HTTP transports run each outgoing message in its sender's
4+
context, which is how code far below `send_raw_request` finds the clock of the request it serves.
5+
6+
Nothing in this module is public API: it may change or be removed without notice. It is likely to
7+
change with the client dispatcher work in https://github.com/modelcontextprotocol/python-sdk/pull/3517.
8+
"""
9+
10+
import math
11+
from collections.abc import Iterator
12+
from contextlib import contextmanager
13+
from contextvars import ContextVar
14+
15+
import anyio
16+
17+
18+
class RequestClock:
19+
"""One request's timeout, as a budget of seconds that is spent only while the clock runs.
20+
21+
Not public API: may change or be removed without notice.
22+
"""
23+
24+
def __init__(self, timeout: float | None, scope: anyio.CancelScope) -> None:
25+
self._budget = math.inf if timeout is None else timeout
26+
self._scope = scope
27+
self._pauses = 1 # the write, which is off the clock too; `start()` ends it
28+
29+
def start(self) -> None:
30+
self.resume()
31+
32+
def pause(self) -> None:
33+
if not self._pauses:
34+
self._budget = self._scope.deadline - anyio.current_time()
35+
self._scope.deadline = math.inf
36+
self._pauses += 1
37+
38+
def resume(self) -> None:
39+
self._pauses -= 1
40+
if not self._pauses:
41+
self._scope.deadline = anyio.current_time() + self._budget
42+
43+
44+
_clock: ContextVar[RequestClock | None] = ContextVar("request_clock", default=None)
45+
46+
47+
@contextmanager
48+
def request_clock(timeout: float | None) -> Iterator[RequestClock]:
49+
"""Put a clock, not yet started, in the context of the request sent in this block.
50+
51+
Raises `TimeoutError` once the clock has run for `timeout` seconds.
52+
53+
Not public API: may change or be removed without notice.
54+
"""
55+
with anyio.CancelScope() as scope:
56+
clock = RequestClock(timeout, scope)
57+
token = _clock.set(clock)
58+
try:
59+
yield clock
60+
finally:
61+
_clock.reset(token)
62+
# Not `fail_after`: it re-reads the deadline here, which a pause may have moved since it expired.
63+
if scope.cancelled_caught:
64+
raise TimeoutError
65+
66+
67+
@contextmanager
68+
def waiting_on_a_person() -> Iterator[None]:
69+
"""Stop the clock of the request this code is serving while the block is open; a no-op if there is none.
70+
71+
Not public API: may change or be removed without notice.
72+
"""
73+
clock = _clock.get()
74+
if clock is None:
75+
yield
76+
return
77+
clock.pause()
78+
try:
79+
yield
80+
finally:
81+
clock.resume()

‎src/mcp/shared/jsonrpc_dispatcher.py‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838

3939
from mcp.shared._compat import resync_tracer
4040
from mcp.shared._otel import inject_trace_context, otel_span
41+
from mcp.shared._request_clock import request_clock
4142
from mcp.shared._stream_protocols import ReadStream, WriteStream
4243
from mcp.shared.dispatcher import (
4344
CallOptions,
@@ -404,12 +405,13 @@ async def send_raw_request(
404405
# never started; past this point a cancelled write counts as issued.
405406
await anyio.lowlevel.checkpoint_if_cancelled()
406407
request_write_started = True
407-
try:
408-
await self._write(msg, plan.metadata)
409-
except (anyio.BrokenResourceError, anyio.ClosedResourceError):
410-
# Transport tore down before run() noticed EOF; surface the documented contract.
411-
raise MCPError(code=CONNECTION_CLOSED, message="Connection closed") from None
412-
with anyio.fail_after(opts.get("timeout")):
408+
with request_clock(opts.get("timeout")) as clock:
409+
try:
410+
await self._write(msg, plan.metadata)
411+
except (anyio.BrokenResourceError, anyio.ClosedResourceError):
412+
# Transport tore down before run() noticed EOF; surface the documented contract.
413+
raise MCPError(code=CONNECTION_CLOSED, message="Connection closed") from None
414+
clock.start()
413415
timeout_armed = True
414416
outcome = await receive.receive()
415417
if isinstance(outcome, ErrorData) and outcome is not _CLOSED_OUTCOME:
@@ -418,7 +420,7 @@ async def send_raw_request(
418420
span.set_status(StatusCode.ERROR, outcome.message)
419421
except TimeoutError:
420422
if not timeout_armed:
421-
# `fail_after` arms only after the write, so this TimeoutError is the
423+
# The clock starts only after the write, so this TimeoutError is the
422424
# transport's own bounded send() failing - a transport error, not
423425
# `opts["timeout"]` elapsing. Propagate it raw (v1 kept the write
424426
# outside the timeout-catching try and did the same).

‎tests/interaction/_requirements.py‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,15 @@ def __post_init__(self) -> None:
430430
),
431431
added_in="2026-07-28",
432432
),
433+
"lifecycle:discover:fallback-silence": Requirement(
434+
source=f"{SPEC_2026_BASE_URL}/basic/transports/stdio#backward-compatibility",
435+
behavior=(
436+
"When server/discover goes unanswered for the probe deadline, an auto-negotiating client falls "
437+
"back to the legacy initialize handshake."
438+
),
439+
added_in="2026-07-28",
440+
note="The spec states the timeout rule for stdio only; the SDK applies it on every transport.",
441+
),
433442
"lifecycle:discover:network-error-raises": Requirement(
434443
source="sdk",
435444
behavior=(
@@ -3826,6 +3835,25 @@ def __post_init__(self) -> None:
38263835
transports=("streamable-http",),
38273836
note="OAuth is HTTP-only.",
38283837
),
3838+
"client-auth:login-time:not-counted": Requirement(
3839+
source="issue:#3601",
3840+
behavior=(
3841+
"The time OAuthClientProvider spends awaiting redirect_handler and callback_handler does not count "
3842+
"against the timeout of the request that was challenged; afterwards the timeout resumes with the "
3843+
"budget that was left. The provider's own HTTP calls do count."
3844+
),
3845+
transports=("streamable-http",),
3846+
note="OAuth is HTTP-only.",
3847+
),
3848+
"client-auth:login-time:other-requests-keep-counting": Requirement(
3849+
source="sdk",
3850+
behavior=(
3851+
"A login suspends the timeout of the challenged request only: a request queued behind it in the "
3852+
"provider still times out on its own clock."
3853+
),
3854+
transports=("streamable-http",),
3855+
note="OAuth is HTTP-only.",
3856+
),
38293857
"client-auth:pkce:refuse-if-unsupported": Requirement(
38303858
source=f"{SPEC_BASE_URL}/basic/authorization#authorization-code-protection",
38313859
behavior=(

‎tests/interaction/auth/_harness.py‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,9 @@ async def connect_with_oauth(
418418
verify_tokens: bool = True,
419419
app_shim: Callable[[ASGIApp], ASGIApp] | None = None,
420420
on_request: Callable[[httpx2.Request], None] | None = None,
421+
mode: str = "legacy",
422+
read_timeout_seconds: float | None = None,
423+
json_response: bool = False,
421424
) -> AsyncIterator[tuple[Client, HeadlessOAuth]]:
422425
"""Connect a `Client` to a server's bearer-gated streamable-HTTP app, completing OAuth in process.
423426
@@ -455,6 +458,7 @@ async def connect_with_oauth(
455458
)
456459

457460
app: ASGIApp = server.streamable_http_app(
461+
json_response=json_response,
458462
auth=settings,
459463
token_verifier=ProviderTokenVerifier(provider) if verify_tokens else None,
460464
auth_server_provider=provider,
@@ -481,7 +485,11 @@ async def hook(request: httpx2.Request) -> None:
481485
)
482486
headless.bind(http_client)
483487
client = await stack.enter_async_context(
484-
# The auth flow tests snapshot the legacy initialize-handshake HTTP shape.
485-
Client(streamable_http_client(f"{BASE_URL}/mcp", http_client=http_client), mode="legacy")
488+
# The auth flow tests snapshot the legacy initialize-handshake HTTP shape, hence the default mode.
489+
Client(
490+
streamable_http_client(f"{BASE_URL}/mcp", http_client=http_client),
491+
mode=mode,
492+
read_timeout_seconds=read_timeout_seconds,
493+
)
486494
)
487495
yield client, headless

0 commit comments

Comments
 (0)