feat: add reset_timeout_on_progress and max_total_timeout to send_request#2589
Open
stefandevo wants to merge 5 commits into
Open
feat: add reset_timeout_on_progress and max_total_timeout to send_request#2589stefandevo wants to merge 5 commits into
stefandevo wants to merge 5 commits into
Conversation
…uest When an MCP client calls a long-running tool that sends periodic progress notifications, the request currently times out on a fixed wall-clock budget even though the server is actively sending progress updates. This breaks blocking UI-card tools in agent frameworks (e.g., DevFlow's devflow_user_choice, devflow_suggest_create_pr) that intentionally wait for human interaction and rely on progress notifications as heartbeats. Add two new opt-in parameters to BaseSession.send_request(): - reset_timeout_on_progress (bool, default False): when enabled, each matching notifications/progress resets the timeout window to current_time + timeout. - max_total_timeout (float | None, default None): optional absolute ceiling measured from the original request start time. If exceeded, the request fails even if progress keeps arriving. Both parameters are threaded through ClientSession.call_tool() and are backward-compatible — existing code is unaffected. Implementation uses anyio.move_on_after() in a loop, checking a per-request _ProgressTimeoutInfo.was_reset flag set by the receive loop when a matching progress notification arrives. This avoids cross-task CancelScope manipulation and works with both asyncio and trio backends. Semantics mirror the TypeScript SDK's resetTimeoutOnProgress / maxTotalTimeout in RequestOptions. Tests cover: no progress → timeout, progress resets timeout, max total timeout ceiling, progress stops → timeout fires, multiple progress notifications, default-off behavior, and call_tool passthrough. DEV-3332
Remove unused `field` import and break long line (E501).
Add isinstance(msg, SessionMessage) guard before accessing .message to satisfy pyright's type narrowing. Mark unused server_write with underscore prefix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds two new opt-in parameters to
BaseSession.send_request()andClientSession.call_tool():reset_timeout_on_progress(bool, defaultFalse) — when enabled, each matchingnotifications/progressresets the timeout window tocurrent_time + timeout, keeping the request alive as long as the server keeps sending progress.max_total_timeout(float | None, defaultNone) — optional absolute ceiling (seconds) measured from the original request start time. If exceeded, the request fails even if progress keeps arriving.These mirror the TypeScript SDK's
resetTimeoutOnProgress/maxTotalTimeoutsemantics inRequestOptions.Motivation
Long-running MCP tool calls that send periodic progress updates currently time out on a fixed wall-clock budget. This breaks tools that intentionally wait for external events (human clicks, approval workflows) and use progress notifications as heartbeats. The TypeScript SDK already solved this with
resetTimeoutOnProgress; this PR brings parity to the Python SDK.Implementation
anyio.move_on_after()in a loop with a per-request_ProgressTimeoutInfodataclass. When the receive loop processes a matching progress notification, it setswas_reset = True, causing the send loop to restart the timeout window.CancelScopemanipulation and works with bothasyncioandtriobackends.reset_timeout_on_progress=Truebut noprogress_callbackis provided, a no-op callback is registered so the progress token is included in the request and the receive loop processes notifications.Tests
7 new tests covering all TypeScript SDK scenarios:
test_no_progress_no_reset_timeout_firestest_progress_resets_timeouttest_max_total_timeout_exceededtest_progress_stops_timeout_firestest_multiple_progress_notificationstest_reset_timeout_false_by_defaulttest_call_tool_threads_reset_timeoutClientSession.call_toolFull suite: 1179 passed, 98 skipped, 1 xfailed (zero regressions from 1172 baseline).
Related
packages/core/src/shared/protocol.ts(_setupTimeout,_resetTimeout,_cleanupTimeout)