Delay SSE GET connection until after session ID is established

[mattt]

Follow-up to #96

This addresses the initialization timing issue where the SSE GET could be attempted before the session was established, as discussed in MCP PR #91.

• Add a signaling mechanism to ensure the SSE streaming task waits for the initial session ID to be set before attempting the GET connection.
• Use a TaskGroup to race between session ID signal and a configurable timeout (sseInitializationTimeout, 10 seconds by default), logging the outcome.
• Trigger the signal when a session ID is received for the first time from any response.
• Add detailed logging for all code paths to aid debugging and clarify connection timing.
• Clean up signal resources on disconnect to prevent leaks.

This PR also adds documentation comments for HTTPClientTransport.

[mattt]

@stallent How does this line up with your understanding of the problem that you identified in #91 regarding SSE timing?

[stallent]

@mattt it looks like it should. Using the flow sequence here https://modelcontextprotocol.io/specification/2025-03-26/basic/transports#sequence-diagram its not completely clear if all we need is to wait for an Mcp-Session-Id to exist (which is how you have it) or if it should wait until after the client has sent the InitializedNotification. The latter would obviously require the transport to have a bit more knowledge of the state of the client, which is not cool, so yeah i think we roll with what you have until we learn otherwise.

The one thing i would like to mention that is a tad wonk is the Transport requires the app to call client.initialize() within timeout seconds after calling client.connect(). That sorta imposes an implicit but unclear timing dependency. I am not exactly sure the best solution. Could be argued that there shouldn't be two explicit steps for the client, but not sure i love calling initialize within connect. hmmmm

[mattt]

@stallent Thanks for weighing in!

The one thing i would like to mention that is a tad wonk is the Transport requires the app to call client.initialize() within timeout seconds after calling client.connect(). That sorta imposes an implicit but unclear timing dependency. I am not exactly sure the best solution. Could be argued that there shouldn't be two explicit steps for the client, but not sure i love calling initialize within connect. hmmmm

I'm with you. Snooping on the data flowing through the transport feels wrong, and seems to go against the notion of an agnostic transport layer (although ironically, that creates the conditions by which we must snoop to know when an initialized notification comes through).

For now, I think a configurable timeout (spelled sseInitializationTimeout) set to 10 seconds by default should (🤞) satisfy most use cases. API consumers can bump that out to be larger if needed.

In the future, I think we can solve the coordination problem in a less janky way with a higher-level abstraction, like we're discussing in #27.

[stallent]

Oh i DEF like renaming it to sseInitializationTimeout for clarity. good call.

[stallent]

@mattt one last thing, just as a point of reference... I looked at the official TS client and it DOES call initialize from within the connect() method, so there is at least precedent for it.

override async connect(transport: Transport, options?: RequestOptions): Promise<void> {
    await super.connect(transport);
    // When transport sessionId is already set this means we are trying to reconnect.
    // In this case we don't need to initialize again.
    if (transport.sessionId !== undefined) {
      return;
    }
    try {
      const result = await this.request(
        {
          method: "initialize",
          params: {
            protocolVersion: LATEST_PROTOCOL_VERSION,
            capabilities: this._capabilities,
            clientInfo: this._clientInfo,
          },
        },
        InitializeResultSchema,
        options
      );

[mattt]

@stallent That's a good callout. I think it's totally reasonable to call initialize automatically with connect. I just opened #100 to make this change.