fix(streamable-http): expose session_idle_timeout and pause idle reaping during active requests#2457
Open
shaun0927 wants to merge 4 commits intomodelcontextprotocol:mainfrom
Open
Conversation
The StreamableHTTP idle-timeout feature was implemented on the session manager but not exposed through the canonical streamable_http_app() entrypoints, pushing users toward lower-level wiring. More importantly, the idle deadline could fire while a request was still in flight, terminating the session mid-handler even though the feature is documented as reaping sessions that receive no HTTP requests. This threads session_idle_timeout through the public app builders, pauses idle reaping while requests are active, restores the deadline after the last request completes, and adds regression coverage for both passthrough and long-running request behavior. Constraint: Must preserve existing idle-session cleanup semantics once no requests remain in flight Rejected: Expose session_idle_timeout without changing runtime semantics | leaves the documented feature behavior observably incorrect Confidence: medium Scope-risk: moderate Reversibility: clean Directive: If idle reaping changes again, verify both public API reachability and active-request semantics together Tested: uv run --frozen pytest tests/server/test_streamable_http_manager.py; uv run --frozen ruff check src/mcp/server/lowlevel/server.py src/mcp/server/mcpserver/server.py src/mcp/server/streamable_http.py src/mcp/server/streamable_http_manager.py tests/server/test_streamable_http_manager.py; uv run --frozen pyright src/mcp/server/lowlevel/server.py src/mcp/server/mcpserver/server.py src/mcp/server/streamable_http.py src/mcp/server/streamable_http_manager.py Not-tested: Full integration matrix outside the focused StreamableHTTP manager suite
The new session_idle_timeout tests started exercising two paths the existing source annotations and timing assumptions no longer matched. CI's strict-no-cover gate now observes the session_manager return path, and the idle-reap assertion can race under slower coverage-instrumented runs. This follow-up removes the stale pragma and makes the idle-reap assertion wait for actual session removal before checking the 404 behavior. Constraint: Must preserve the PR's runtime semantics and only address CI-validity drift Rejected: Increase fixed sleeps again | keeps the test timing-sensitive and flaky under slower environments Confidence: high Scope-risk: narrow Reversibility: clean Directive: Prefer state-based waits over fixed sleeps in StreamableHTTP timeout tests Tested: uv run --frozen pytest tests/server/test_streamable_http_manager.py; coverage run of the same suite; strict-no-cover-sensitive paths exercised locally Not-tested: Full upstream matrix rerun after push
The StreamableHTTP follow-up test was logically correct but not yet aligned with the repository's stricter pyright settings in CI. This updates the test to use the typed Tool field name, annotate the ASGI handler parameters explicitly, and narrow the tool result content before reading .text. The runtime behavior under test is unchanged. Constraint: Must keep the fix scoped to type-check compliance for the new regression test Rejected: Silence pyright with ignores | conflicts with repo guidance and hides actionable typing issues Confidence: high Scope-risk: narrow Reversibility: clean Directive: New regression tests in this repo should satisfy the same pyright strictness as production code Tested: uv run --frozen pytest tests/server/test_streamable_http_manager.py; uv run --frozen ruff check tests/server/test_streamable_http_manager.py; uv run --frozen pyright tests/server/test_streamable_http_manager.py Not-tested: Full upstream CI rerun after push
Author
|
for additional signal: |
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.
Fixes #2455
Summary
This PR fixes two linked
session_idle_timeoutproblems in the StreamableHTTP stack:session_idle_timeoutthrough the publicstreamable_http_app()entrypointsMotivation and Context
PR #1994 / #2022 describe
session_idle_timeoutas a way to reap sessions that receive no HTTP requests for the configured duration.Current
mainhas two mismatches with that contract:StreamableHTTPSessionManager(...)acceptssession_idle_timeout, butServer.streamable_http_app(...)andMCPServer.streamable_http_app(...)do not, so the canonical high-level API cannot configure the featureChanges
session_idle_timeoutpassthrough to:mcp.server.lowlevel.server.Server.streamable_http_app()mcp.server.mcpserver.server.MCPServer.streamable_http_app()StreamableHTTPServerTransportHow Has This Been Tested?
Added focused regression coverage in
tests/server/test_streamable_http_manager.py:test_streamable_http_app_exposes_session_idle_timeouttest_session_idle_timeout_does_not_cancel_in_flight_requestThe second test uses a handler that runs longer than the configured idle timeout and verifies that the request still completes successfully.
Existing idle reaping coverage (
test_idle_session_is_reaped) continues to pass, so finished sessions are still cleaned up normally.Local verification:
uv run --frozen pytest tests/server/test_streamable_http_manager.pyuv run --frozen ruff check src/mcp/server/lowlevel/server.py src/mcp/server/mcpserver/server.py src/mcp/server/streamable_http.py src/mcp/server/streamable_http_manager.py tests/server/test_streamable_http_manager.pyuv run --frozen pyright src/mcp/server/lowlevel/server.py src/mcp/server/mcpserver/server.py src/mcp/server/streamable_http.py src/mcp/server/streamable_http_manager.pyBreaking Changes
None.
Types of changes
Checklist