Skip to content

Bound client-suite concurrency for timing-sensitive SSE checks #500

Description

@CharlesMcMillan

The client suite starts every selected scenario at once with Promise.all. With @modelcontextprotocol/conformance@0.2.0-alpha.11, --suite all starts 44 client processes together.

That makes the sse-retry check sensitive to load from the suite itself. The server sends retry: 500, and the client schedules a 500 ms timer. The check measures from the server closing the stream until the server handles the reconnect, so the result also includes event-loop delay and local HTTP dispatch.

The current thresholds pass at 450 to 700 ms and report a warning from 700 to 1000 ms. The warning says the delay is acceptable, but the suite runner treats any warning as an unexpected failure.

This happened in modelcontextprotocol/typescript-sdk#2757: sse-retry reported 2 passes, 0 failures, and 1 warning, which failed the client-conformance job. The OAuth code in that PR does not change SSE reconnect behavior.

I reproduced the same result without changing the client's requested delay. I kept the 500 ms timer and added controlled event-loop work near its deadline. The timer fired at 753 to 754 ms, reconnect and Last-Event-ID checks passed, and the runner produced the same warning and nonzero exit. The isolated scenario and three fresh full-suite runs passed locally at 502 to 505 ms. This does not prove the failed CI run hit the same warning branch because that run did not retain checks.json, but it shows that a correct retry timer can fail under scheduling pressure.

Could the harness support:

  • a bounded --concurrency <n> option for client suites;
  • a repeatable way to isolate wall-clock-sensitive scenarios such as sse-retry from the concurrent pool;
  • the same authoritative suite selection, result order, expected-failure reconciliation, and aggregate exit status as today;
  • detailed result artifacts even when a suite fails?

I do not think sse-retry should be added to the expected-failures baseline or given looser timing assertions. Those choices would hide real reconnect regressions. The goal is to keep the check while removing load created by its own test runner.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions