Skip to content

Conversation

@bjoaquinc
Copy link

@bjoaquinc bjoaquinc commented Nov 3, 2025

This implements fixes to the StreamingASGITransport to properly handle SSE streaming and disconnect signaling. The key changes were:

  • Added disconnect signaling via receive() to fix SSE hanging
  • Fixed memory stream cleanup in SseServerTransport to eliminate ResourceWarnings
  • Handle duplicate http.response.start with NoopASGI
  • Fixed global sse-starlette quirk
  • Update all tests in tests/server/test_sse_security.py and tests/shared/test_sse.py to use transports instead of real servers with uvicorn

Motivation and Context

Partially resolves #857

How Has This Been Tested?

Since I updated the testing methodology I made sure to run the test in batches of 30 (to check for race conditions). Using the command:

passed=0; failed=0; for i in {1..30}; do echo "=== Run $i/30 ===" && uv run --resolution="$RESOLUTION" pytest "$FILE_PATH" -q && passed=$((passed+1)) || failed=$((failed+1)); done; echo "==== SUMMARY: $passed passed, $failed failed out of 30 runs ===="

for the following resolutions and files:

  • tests/server/test_sse_security.py for lowest-direct and highest
  • tests/shared/test_sse.py for lowest-direct and highest

All tests passed:

Screenshot 2025-11-08 at 6 41 36 PM

Breaking Changes

None. StreamingASGITransport is only meant for testing. The only change I made to SseServerTransport was to properly clean up sse_stream_reader which is best practice and doesn't break functionality. The modified tests all function as expected.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@maxisbey maxisbey added P3 Nice to haves, rare edge cases needs more work Not ready to be merged yet, needs additional changes. labels Nov 4, 2025
@bjoaquinc
Copy link
Author

Done with all updates and ready for a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs more work Not ready to be merged yet, needs additional changes. P3 Nice to haves, rare edge cases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Drop uvicorn from test suite

2 participants