Skip to content

Emit event: message on streamable-http SSE frames - #5954

Merged
ChrisJBurns merged 4 commits into
stacklok:mainfrom
stantheman0128:fix/5655-sse-event-name
Jul 28, 2026
Merged

Emit event: message on streamable-http SSE frames#5954
ChrisJBurns merged 4 commits into
stacklok:mainfrom
stantheman0128:fix/5655-sse-event-name

Conversation

@stantheman0128

Copy link
Copy Markdown
Contributor

Summary

The streamable-http proxy wrote JSON-RPC SSE frames as data:-only (no event: line). That is legal per WHATWG SSE (unnamed frames dispatch as message), but it diverges from MCP reference transports and ToolHive's own ssecommon serializer, which always emit event: message. Spec-lenient clients such as @ai-sdk/mcp only dispatch frames where event === "message" and hang on initialize against stdio servers fronted by this proxy. The same omission also breaks ToolHive vMCP backend init when responses arrive on the GET/async SSE stream (see issue comments).

What changed

  • Update the shared writeSSEData helper to emit event: message\ndata: ...\n\n
  • That covers every JSON-RPC SSE writer in the package: POST success, POST error (writeSSEErrorEvent), GET standalone notifications, and progress frames
  • Add unit + POST SSE + error-path regression tests that assert the raw frame starts with event: message

Fixes #5655

Type of change

  • Bug fix

Test plan

  • Unit / package tests (go test ./pkg/transport/proxy/streamable/ ...)
  • E2E tests (task test-e2e)
  • Linting (task lint-fix)
  • Manual testing (describe below)

Evidence

HEAD before fix: writeSSEData used fmt.Fprintf(w, "data: %s\n\n", data) only.

After fix, sample SSE frame bytes (hex):

6576656e743a206d6573736167650a646174613a207b226a736f6e727063223a22322e30222c226964223a312c22726573756c74223a7b226f6b223a747275657d7d0a0a

Decoded:

event: message
data: {"jsonrpc":"2.0","id":1,"result":{"ok":true}}

Targeted tests (Windows):

go test ./pkg/transport/proxy/streamable/ -count=1 -run "TestWriteSSEDataIncludesEventMessage|TestSSEResponseIncludesEventMessage|TestSSEErrorEventIncludesEventMessage" -v
--- PASS: TestWriteSSEDataIncludesEventMessage (0.00s)
--- PASS: TestSSEErrorEventIncludesEventMessage (0.00s)
--- PASS: TestSSEResponseIncludesEventMessage (0.06s)
PASS

Full package (skipping pre-existing unrelated failure on main from #5944, where filtered tools/call now returns HTTP 200 JSON-RPC instead of 400):

go test ./pkg/transport/proxy/streamable/ -count=1 -skip TestStandaloneSSE_ListChangedRefiltersThroughExistingMiddleware
ok  	github.com/stacklok/toolhive/pkg/transport/proxy/streamable	11.581s

What was not tested

  • Live @ai-sdk/mcp or vMCP operator click-through on this box (no studio / cluster harness here). Framing is asserted via raw SSE bytes on the proxy's own HTTP path.
  • Full task lint-fix / e2e suite (targeted package tests only).

Does this introduce a user-facing change?

Yes. Streamable-http proxy SSE responses now include an explicit event: message line, matching MCP reference transports. Clients that already accept data-only frames remain compatible.

Special notes for reviewers

AI disclosure

This change was developed with AI assistance (Cursor/Grok). The author reviewed the diff, ran the Evidence commands above, and takes responsibility for the patch.

Signed-off-by: Stan Shih <stantheman0128@users.noreply.github.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@stantheman0128
stantheman0128 force-pushed the fix/5655-sse-event-name branch from aca4e76 to d1cd71f Compare July 28, 2026 02:28
@stantheman0128

Copy link
Copy Markdown
Contributor Author

Rebased onto current main and force-pushed.

  • Resolved merge conflict in streamable_proxy.go (kept upstream writeSSEErrorEvent id-key comment + our event: message framing via writeSSEData).
  • Updated TestWriteSSEErrorEvent to expect event: message + data: frames (matches TestWriteSSEDataIncludesEventMessage).
  • Local: go test ./pkg/transport/proxy/streamable/... passes.

If Go Vulnerability Check still fails after this push, it looks upstream-related (same check was red before the rebase); happy to dig in if a maintainer points at a specific advisory.

@github-actions github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Jul 28, 2026
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 72.43%. Comparing base (ac16484) to head (84bb28f).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
pkg/transport/proxy/streamable/utils.go 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5954      +/-   ##
==========================================
+ Coverage   72.40%   72.43%   +0.03%     
==========================================
  Files         733      733              
  Lines       75845    75845              
==========================================
+ Hits        54918    54942      +24     
+ Misses      17034    17013      -21     
+ Partials     3893     3890       -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ChrisJBurns

Copy link
Copy Markdown
Collaborator

Thanks for the PR @stantheman0128 ! 🚀

@github-actions github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Jul 28, 2026
@ChrisJBurns

Copy link
Copy Markdown
Collaborator

/retest

@github-actions github-actions Bot added size/S Small PR: 100-299 lines changed and removed size/S Small PR: 100-299 lines changed labels Jul 28, 2026
@ChrisJBurns
ChrisJBurns merged commit 525a308 into stacklok:main Jul 28, 2026
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Small PR: 100-299 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

streamable-http proxy omits the SSE event name, breaking spec-lenient MCP clients

2 participants