Skip to content

[Fix] SSE event ordering: emit response.created before MCP discovery events#22286

Open
shivamrawat1 wants to merge 2 commits intomainfrom
litellm_sse_event_ordering
Open

[Fix] SSE event ordering: emit response.created before MCP discovery events#22286
shivamrawat1 wants to merge 2 commits intomainfrom
litellm_sse_event_ordering

Conversation

@shivamrawat1
Copy link
Collaborator

Relevant issues

The /v1/responses streaming endpoint with MCP tools sent response.mcp_list_tools.in_progress, response.mcp_list_tools.completed, and response.output_item.done before response.created. The OpenAI Node SDK (openai@^6.7.0) expects response.created as the first SSE event and can fail when other event types appear first.

MCPEnhancedStreamingIterator started in the mcp_discovery phase and emitted MCP discovery events before creating the base response iterator and yielding its first chunk (response.created).

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

  • I have Added testing in the tests/litellm/ directory, Adding at least 1 test is a hard requirement - see details
  • My PR passes all unit tests on make test-unit
  • My PR's scope is as isolated as possible, it only solves 1 specific problem
  • I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

CI (LiteLLM team)

CI status guideline:

  • 50-55 passing tests: main is stable with minor issues.
  • 45-49 passing tests: acceptable but needs attention
  • <= 40 passing tests: unstable; be careful with your merges and assess the risk.
  • Branch creation CI run
    Link:

  • CI run for the last commit
    Link:

  • Merge / cherry-pick CI run
    Links:

Type

🐛 Bug Fix
🧹 Refactoring
✅ Test

Changes

Introduce a get_response_created phase that runs first: create the base iterator, yield its first chunk (response.created), then move to mcp_discovery and emit MCP events. Refactor anext into phase-specific helpers to keep the logic clear and satisfy the PLR0915 lint rule. Add test_sse_event_ordering_response_created_first to assert the correct event order.

@vercel
Copy link

vercel bot commented Feb 27, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
litellm Ready Ready Preview, Comment Feb 27, 2026 1:06pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 27, 2026

Greptile Summary

Fixes SSE event ordering in the MCP streaming iterator so that response.created is always emitted before MCP discovery events (response.mcp_list_tools.in_progress, etc.). This aligns with what the OpenAI Node SDK (v6.7.0+) expects.

  • Introduces a new get_response_created phase that runs first: creates the base iterator, yields its first chunk (response.created), then transitions to mcp_discovery
  • Refactors the monolithic __anext__ into phase-specific helper methods (_handle_get_response_created_phase, _handle_mcp_discovery_phase, etc.) for clarity and to satisfy lint rules
  • Applies the same ordering fix to the sync __next__ path with a _sync_response_created_emitted flag and _ensure_sync_base_iterator helper
  • Adds two tests (test_sse_event_ordering_response_created_first for async, test_sse_event_ordering_sync_response_created_first for sync) that assert response.created is the first event emitted

Confidence Score: 4/5

  • This PR is safe to merge — it fixes a real SDK compatibility issue with a clean, well-tested refactor.
  • The fix correctly reorders SSE event emission to satisfy OpenAI SDK expectations. The refactor from a monolithic anext to phase-specific helpers is clean and preserves existing behavior. Both async and sync paths are addressed, and two tests directly validate the ordering invariant. The only minor concern is the implicit assumption that the first chunk from the base iterator is always response.created (no validation).
  • No files require special attention. The changes in litellm/responses/mcp/mcp_streaming_iterator.py are well-structured.

Important Files Changed

Filename Overview
litellm/responses/mcp/mcp_streaming_iterator.py Core fix: introduces get_response_created phase to emit response.created before MCP discovery events. Refactors __anext__ into phase-specific helpers and fixes sync __next__ path. Logic is correct and clean.
tests/mcp_tests/test_aresponses_api_with_mcp.py Adds two new tests for SSE event ordering (async and sync paths) and updates docstring for existing test. Tests directly verify the fix.

Sequence Diagram

sequenceDiagram
    participant Client as OpenAI SDK Client
    participant MCP as MCPEnhancedStreamingIterator
    participant LLM as aresponses (LLM call)

    Note over MCP: Phase: get_response_created
    MCP->>LLM: _create_initial_response_iterator()
    LLM-->>MCP: base_iterator (streaming)
    MCP->>MCP: await base_iterator.__anext__()
    MCP-->>Client: response.created (first chunk)

    Note over MCP: Phase: mcp_discovery
    MCP-->>Client: response.mcp_list_tools.in_progress
    MCP-->>Client: response.mcp_list_tools.completed
    MCP-->>Client: response.output_item.done

    Note over MCP: Phase: initial_response
    loop Stream remaining chunks
        MCP->>MCP: await base_iterator.__anext__()
        MCP-->>Client: streaming chunk (text delta, etc.)
    end

    Note over MCP: Phase: tool_execution (if auto-execute)
    MCP-->>Client: mcp_call events

    Note over MCP: Phase: follow_up_response (if tools executed)
    MCP-->>Client: follow-up streaming chunks
Loading

Last reviewed commit: c9ed2eb

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 27, 2026

Additional Comments (1)

litellm/responses/mcp/mcp_streaming_iterator.py
Sync __next__ still has the original ordering bug

The async __anext__ was correctly refactored to emit response.created before MCP discovery events, but the sync __next__ path (lines 727-743) still emits self.mcp_events before delegating to the base iterator. If this sync path is ever used with a base_iterator that yields response.created first, MCP events will still be emitted out of order — the same bug this PR aims to fix.

Consider applying the same phase-based approach here, or at a minimum documenting that __next__ is not used in production MCP flows and is safe to leave as-is.

@shivamrawat1
Copy link
Collaborator Author

@greptile re-review with the new commit now that we implemented the fix for sync path. Also note thatthe contribution guideline is wrong about having the test in tests/litellm/. It is in the right place.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant