Skip to content

Add SSE polling Phase 2 and Phase 3 tests (SEP-1699)#73

Open
pcarleton wants to merge 1 commit intomainfrom
pcarleton/more-sse-polling
Open

Add SSE polling Phase 2 and Phase 3 tests (SEP-1699)#73
pcarleton wants to merge 1 commit intomainfrom
pcarleton/more-sse-polling

Conversation

@pcarleton
Copy link
Member

@pcarleton pcarleton commented Dec 2, 2025

Summary

Adds two new test phases to the SSE polling scenario for SEP-1699:

Phase 2: Event Replay Test

  • Uses test_event_replay tool
  • Tests that server properly stores and replays events sent during disconnect
  • Server sends notification1, closes stream, sends notification2 and notification3
  • Client should receive all notifications via event replay

Phase 3: Multiple Reconnections Test

  • Uses test_multiple_reconnections tool
  • Tests that server can close stream multiple times during single tool call
  • Validates proper event store management across multiple reconnections

Changes

  1. src/scenarios/server/sse-polling.ts: Add Phase 2 and Phase 3 test methods
  2. examples/servers/typescript/everything-server.ts: Add corresponding tools

Test plan

  • All existing tests pass
  • Phase 2 and Phase 3 tests work against everything-server

@pkg-pr-new
Copy link

pkg-pr-new bot commented Dec 2, 2025

Open in StackBlitz

npx https://pkg.pr.new/modelcontextprotocol/conformance/@modelcontextprotocol/conformance@73

commit: e5786e1

@pcarleton pcarleton force-pushed the pcarleton/more-sse-polling branch 2 times, most recently from 5d68a29 to 43ff945 Compare January 5, 2026 14:49
@pcarleton pcarleton changed the title wip... Add SSE polling Phase 2 and Phase 3 tests (SEP-1699) Jan 5, 2026
Add two new test phases to the SSE polling scenario:

Phase 2: Event Replay Test (test_event_replay)
- Tests that server properly stores and replays events sent during disconnect
- Server sends notification1, closes stream, sends notification2 and notification3
- Client should receive all notifications via event replay

Phase 3: Multiple Reconnections Test (test_multiple_reconnections)
- Tests that server can close stream multiple times during single tool call
- Validates proper event store management across multiple reconnections

Also adds corresponding tools to everything-server.ts.
@pcarleton pcarleton force-pushed the pcarleton/more-sse-polling branch from 43ff945 to e5786e1 Compare January 5, 2026 14:52
@pcarleton pcarleton marked this pull request as ready for review January 5, 2026 14:55
@pcarleton
Copy link
Member Author

@claude review

Comment on lines +706 to +709
}

const contentType = postResponse.headers.get('content-type');
if (!contentType?.includes('text/event-stream') || !postResponse.body) {
Copy link

Choose a reason for hiding this comment

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

🟡 Phase 2 (testEventReplay) and Phase 3 (testMultipleReconnections) silently return without pushing any check when the server responds with a non-SSE content type or a non-400/404 HTTP error. Phase 1 (line 196) properly pushes an INFO check explaining why SSE polling tests are not applicable. Adding similar INFO checks in Phase 2 and Phase 3 would maintain consistency and improve diagnostic output.

Extended reasoning...

What the bug is

In testEventReplay (lines 704-710) and testMultipleReconnections (equivalent lines), there are two silent-return paths:

  1. When the HTTP response is an error but not 400/404 (line 705: bare return;)
  2. When the response is 200 OK but the content type is not text/event-stream or the body is null (lines 709-710: bare return;)

In both cases, no conformance check is pushed to the checks array, so the test phase produces no output.

How Phase 1 handles this differently

Phase 1 (lines 196-214) properly handles the non-SSE content type case by pushing an INFO check with details explaining "Server returned JSON instead of SSE stream - priming event tests not applicable". Phase 1 also handles non-400/404 errors by pushing a FAILURE check (lines 183-195). The new phases lack both of these diagnostic messages.

Step-by-step proof

  1. Phase 2 header is pushed: === Phase 2: Event Replay Test (test_event_replay) ===
  2. testEventReplay is called
  3. The POST request returns 200 OK with Content-Type: application/json
  4. Code reaches line 709: the content-type check evaluates to true
  5. The method returns without pushing any check
  6. Phase 3 header is pushed, and the same silent return happens in testMultipleReconnections
  7. The user sees Phase 2 and Phase 3 headers in the output but zero checks underneath them, with no explanation

Impact

This is a minor diagnostic issue. In practice, Phase 2 and Phase 3 only run if Phase 1 succeeded with an SSE response, so hitting these silent-return paths is unlikely during normal operation. However, if a server returns SSE for test_reconnection but JSON for test_event_replay (e.g., because it does not recognize the tool and returns a JSON-RPC error with 200 status), the empty phases with no explanation would be confusing for users trying to understand the test output.

Fix

Add INFO checks in both testEventReplay and testMultipleReconnections for these two cases, matching the pattern used in Phase 1. For the non-SSE content type case, push an INFO check explaining the response type and that the test is not applicable. For the non-400/404 error case, push a WARNING or FAILURE check with the HTTP status code.

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.

2 participants