Repository navigation
Fix protocol version negotiation and threading for SSE streams - #19
Merged
Merged
Conversation
…igravity) The Streamable-HTTP standalone GET stream (HandleStreamableHttpGet) set its response headers but wrote no bytes until the first keep-alive ping 15s later, so the HTTP status line + headers were never committed to the client. The official MCP Go SDK (used by Antigravity IDE and Codex) opens this GET stream *synchronously during connect* and blocks on it until the response headers arrive, before it sends notifications/initialized. With the headers withheld, that GET hung, the client's connect deadline expired, and the follow-up notifications/initialized POST failed instantly with "context deadline exceeded" — exactly the symptom in issue #18. Claude Code and mark3labs-style clients were unaffected because they don't gate the handshake on this GET. Root cause was reproduced empirically against github.com/modelcontextprotocol/ go-sdk: flushing the GET headers immediately was the single load-bearing fix (connect dropped from a 15s timeout to ~3ms). Changes: - HandleStreamableHttpGet: write an initial ": ok" SSE comment to flush the response headers before entering the ping loop (mirrors the legacy GET /sse handler, which always wrote its endpoint event immediately and so never hit this). - Run the accept loop and every request on dedicated background threads instead of Task.Run on the shared ThreadPool. The SSE/GET handlers block for the whole stream lifetime; on the pool they tie up workers and, on low-core machines or across handshake retries, starve short requests like notifications/initialized (queued but never scheduled, never even logged) — a second, independent path to the same timeout. - Keep-alive loops now wait on cts.Token.WaitHandle.WaitOne(...) instead of Thread.Sleep, so server shutdown tears streams down promptly. - initialize now negotiates protocolVersion (echo the client's requested version when supported, else newest) instead of hardcoding "2024-11-05", which violated the MCP lifecycle spec. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LK8KEexyCd3UHEve2zspNq
The SSE / Streamable-HTTP keep-alive loops waited on cts.Token.WaitHandle.WaitOne(...) with a null-forgiving cts!. A concurrent Stop() (EnableServer toggled off, or AppExit) disposes and nulls cts, so between the loop's cancellation check and the WaitHandle access the token source can be disposed -> ObjectDisposedException (or NRE) thrown out of the stream thread on shutdown. Route both loops through WaitForKeepAliveOrStop(), which snapshots cts, returns "stop" when it's null, and catches ObjectDisposedException from a mid-wait dispose -> treats it as cancellation. Shutdown now tears streams down cleanly without a stray exception. Verified: dnSpy launches, Antigravity (Go SDK) connects, initialize echoes the negotiated protocolVersion; E2E suite 154 pass / 0 fail. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #18
Summary
This PR fixes two critical issues in the MCP server: (1) protocol version negotiation now respects the client's requested version per the MCP lifecycle spec instead of hardcoding an old version, and (2) long-lived SSE/Streamable-HTTP streams are now dispatched to dedicated background threads instead of the ThreadPool, preventing worker thread starvation that caused client-side timeouts.
Key Changes
Protocol version negotiation: Added
supportedProtocolVersionsarray (2025-06-18,2025-03-26,2024-11-05) andNegotiateProtocolVersion()method. Theinitializehandler now echoes the client's requested version when supported, falling back to the newest supported version otherwise. Previously hardcoded2024-11-05, ignoring client requests.Threading model for HTTP request handling: Replaced
Task.Run()with dedicated background threads:Thread(not ThreadPool) sinceGetContext()blocks indefinitelynotifications/initialized) that need ThreadPool workersCancellation-aware keep-alive waits: Replaced
Thread.Sleep(15000)withcts.Token.WaitHandle.WaitOne(sseKeepAliveMs)in both legacy SSE and Streamable-HTTP GET handlers. Server shutdown now tears down streams promptly instead of blocking up to 15 seconds.Streamable-HTTP GET header flushing: Added immediate SSE comment write (
session.WriteComment("ok")) to flush response headers before the first keep-alive ping. The official MCP client (Go SDK) opens this stream synchronously during connect and blocks until headers arrive; without this, the client's connect deadline expires beforenotifications/initializedcan be sent.Code cleanup: Removed unused
using System.Threading.Tasks;directive.Implementation Details
The threading changes address a real-world issue: on low-core machines or during client handshake retries, long-lived streams (which block for their entire lifetime) were consuming all available ThreadPool workers, causing short requests to time out with "context deadline exceeded" client-side. Dedicated threads per request decouple long-lived streams from the pool, ensuring quick calls always get a thread.
The Streamable-HTTP GET header flush is necessary because the official MCP client blocks synchronously on the response headers before sending follow-up requests; delaying header commit until the first ping interval (15s later) caused the client's connect deadline to expire.
https://claude.ai/code/session_01LK8KEexyCd3UHEve2zspNq