fix: accept text/event-stream and unwrap SSE on /mcp (GV-401)

[diogocabral]

Problem (found in live testing)

After gv auth login succeeded, every MCP call failed:

transport: 406: {"jsonrpc":"2.0","error":{"code":-32000,"message":"Not Acceptable: Client must accept both application/json and text/event-stream"},"id":null}}

GitVelocity's MCP server uses the Streamable HTTP transport, which (per the MCP spec) rejects any request whose Accept header doesn't include both application/json and text/event-stream. Our client (like searchlight-cli's) sent only application/json → HTTP 406 on the very first initialize, so tools/list, whoami, and the post-login cache all failed. The httptest-based tests never caught it because the fake handler didn't enforce the header — exactly the kind of gap only a live run surfaces.

Fix

• Send Accept: application/json, text/event-stream.
• Unwrap SSE responses: the transport replies with Content-Type: text/event-stream, framing the JSON-RPC message as data: {...}. The response path now extracts the data: payload before decoding; a plain application/json response still passes through untouched.
• Propagate Mcp-Session-Id: capture it from any response and echo it on subsequent requests, so the stateful transport mode works (no-op when the server is stateless).

Tests

• TestExtractRPCPayload — table-driven SSE unwrapping (json passthrough, single event, CRLF, multi-line data:, charset suffix, no-data fallback).
• TestStreamableHTTP_AcceptAndSSE — regression: an httptest handler that 406s without the dual Accept, replies via SSE, and hands out a session id; asserts initialize+tools/list succeed, every request advertises both media types, and the session id rides along on the second call.

Verify

go test -race ./... ✅ · coverage 81.1% · golangci-lint ✅ · gosec ✅

Note

Once merged, this rides release-please → v0.1.1 → GoReleaser → updated Homebrew formula. The v0.1.0 already on the tap has the broken transport, so a patch release is needed for brew-installed users to get a working CLI.

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• Bug Fixes & Improvements

  • Enhanced MCP client HTTP handling to support streamable protocols with improved session state management.
  • Better support for streaming server responses with automatic session tracking across requests.

• Tests

  • Added regression tests for streamable HTTP behavior, including payload extraction and session ID propagation validation.

[linear]

GV-401

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The PR adds streamable HTTP session semantics to the MCP client by introducing mutex-protected session ID tracking, SSE payload extraction logic, updated request headers that advertise both JSON and event-stream support, refactored retry handling that persists session IDs, and comprehensive tests validating both low-level SSE unwrapping and end-to-end client behavior with a mock server.

Changes

MCP Streamable HTTP Session Support

Layer / File(s)	Summary
Session State Management internal/mcp/client.go	sync package imported; Client struct gains mutex-protected sessionID field with getSession/setSession helper methods for thread-safe access.
SSE Payload Extraction Logic and Unit Tests internal/mcp/client.go, internal/mcp/streamhttp_test.go	extractRPCPayload detects text/event-stream Content-Type and extracts the first SSE event's data: payload (handling multi-line data, CRLF, and charset variants); TestExtractRPCPayload validates extraction across plain JSON, SSE, and edge-case body variants.
HTTP Request/Response Handling with Sessions internal/mcp/client.go	Accept header set to "application/json, text/event-stream" and conditionally includes stored Mcp-Session-Id; new attempt helper captures response status/Content-Type, drains body fully, stores returned Mcp-Session-Id, and retries once on 401 after token refresh; SSE unwrapping applied before JSON unmarshalling.
End-to-End Integration Tests internal/mcp/streamhttp_test.go	streamHTTPHandler mocks an SSE-capable server that validates dual-Accept requirement, emits Mcp-Session-Id on initialize, and returns SSE-framed JSON-RPC; TestStreamableHTTP_AcceptAndSSE runs the client against the server and asserts correct header negotiation, SSE unwrapping, and session ID propagation across multiple requests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Sessions flow through SSE streams so bright,
Data wrapped in frames, now parse just right!
Mutex guards the session state secure,
Headers dance in sync, the protocol's pure. 🔄✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately captures the main change: accepting text/event-stream and unwrapping SSE responses in the MCP client, matching the core fixes implemented across the two modified files.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/mcp-streamable-http-accept

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gitvelocity-reviewer]

📊 Code Quality Score: 28/100

46 × 0.6 = 27.6 → 28

Category	Score	Factors
🔭 Scope	8/20	Two files, single MCP subsystem, touches core transport path on every call, no new public APIs
🏗️ Architecture	7/20	Adds mutex-protected session state to Client struct, introduces pure extractRPCPayload abstraction, no new external dependencies
⚙️ Implementation	10/20	SSE parsing with multi-line data support per spec, mutex-protected session state, content-type negotiation, retry flow extension with contentType threading
⚠️ Risk	8/20	Touches hot path of every MCP call, Accept header change is a behavioral fix, session state adds statefulness, no mechanism to clear stale sessions on 400
✅ Quality	11/15	7 unit test cases for extractRPCPayload, integration test with realistic mock server validating Accept header and session propagation; missing concurrent access and 401+session retry tests
🔒 Perf / Security	2/5	Mutex appropriately scoped to session field only, no security concerns introduced, no benchmarks

Was this score accurate? 👍 Yes · 👎 No

_{Scored by GitVelocity · How are scores calculated?}

[gitvelocity-reviewer]

📊 Code Quality Score: 26/100

43 × 0.6 = 25.8, rounded to 26

Category	Score	Factors
🔭 Scope	7/20	2 files, single subsystem (MCP transport), targeted bug fix, no new public APIs or external integrations
🏗️ Architecture	6/20	New session state with mutex in Client struct, new extractRPCPayload pure function, attempt() signature extended; no new dependencies or service boundaries
⚙️ Implementation	10/20	SSE spec parsing (multi-line data, CRLF, blank-line dispatch), thread-safe session management, clean closure refactor; moderate complexity well-executed
⚠️ Risk	6/20	Transport layer change affects all MCP calls; Accept header fix is protocol-correct; session propagation is additive/harmless in stateless mode; mutex protects concurrent access; easily reversible
✅ Quality	12/15	7 unit test cases for extractRPCPayload covering edge cases; httptest integration test pins Accept header and session propagation; clear inline protocol comments; ~85%+ coverage of new code
🔒 Perf / Security	2/5	Mutex not held during I/O; O(n) SSE parsing appropriate; no security concerns; no benchmarks

Was this score accurate? 👍 Yes · 👎 No

_{Scored by GitVelocity · How are scores calculated?}