fix(mcp): add SSE keepalive ping task#21343
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an SSE-specific keepalive coroutine to MCPServerTask that periodically issues lightweight MCP send_ping() calls while an SSE session is open, triggering a reconnect signal on ping failure without changing teardown semantics for other transports.
Changes:
- Add
_sse_keepalive()coroutine to ping SSE sessions and request reconnect on failure. - Start/cancel the SSE keepalive task within the SSE branch of
_run_http(). - Add tests ensuring the keepalive task is started for SSE and that it uses
send_ping()until failure.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tools/mcp_tool.py | Adds SSE keepalive coroutine and wires it into the SSE session lifecycle in _run_http(). |
| tests/tools/test_mcp_sse_transport.py | Adds unit tests for SSE keepalive task startup and ping loop behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| await keepalive_task | ||
| except (asyncio.CancelledError, Exception): | ||
| pass |
|
Small clarification on scope: this intentionally does not replace the existing lifecycle keepalive in This PR only adds an SSE-scoped |
a909863 to
7fc9e90
Compare
|
Adjusted the keepalive cleanup per Copilot's cancellation note. The SSE branch now cancels the keepalive task and awaits it with: await asyncio.gather(keepalive_task, return_exceptions=True)That still drains exceptions from the child keepalive task, but avoids explicitly catching |
Summary
send_ping()calls while an SSE transport session is open_wait_for_lifecycle_event()/ SSE session lifecycle pathContext
Follow-up to #5981 after #21323 landed the timeout and OAuth-forwarding pieces. This keeps the remaining keepalive work as a small single-commit PR on current
mainand avoids resurrecting the stale branch.Related: #3976 proposed SSE/HTTP keepalive work more broadly; this PR intentionally limits scope to the unmerged SSE keepalive coroutine.
Test plan
python -m pytest tests/tools/test_mcp_sse_transport.py::TestSSEKeepalive::test_sse_keepalive_sends_ping_until_ping_fails -q -o 'addopts='failed on current main because_sse_keepalivedid not existpython -m pytest tests/tools/test_mcp_sse_transport.py::TestSSEKeepalive -q -o 'addopts='python -m pytest tests/tools/test_mcp_tool.py tests/tools/test_mcp_sse_transport.py -q -o 'addopts='