fix(mcp): Fix legacy MCP SSE connections

[zhuangbiaowei]

Summary

• treat URLs ending in /sse as legacy MCP SSE endpoints instead of first POSTing to the SSE URL as Streamable HTTP
• send JSON-RPC request ids as strings while still accepting numeric response echoes
• add regression coverage for SSE endpoint detection and mixed string/numeric response ids

Bugfix

This fixes legacy SSE MCP servers that expose an initial GET /sse stream with an endpoint event, but reject direct Streamable HTTP POSTs to /sse. One observed failure was a 403 api-key not found response from the wrong POST path even though the correct SSE flow does not require an API key.

Verification

• cargo test -p codewhale-tui mcp
• cargo run -p codewhale-tui -- mcp validate

Greptile Summary

This PR fixes legacy MCP SSE server compatibility by replacing a URL-path heuristic (/sse suffix detection) with an explicit transport: "sse" config field, propagating custom headers to both the initial SSE GET and the subsequent POST, and switching JSON-RPC request IDs from u64 to String while accepting numeric echoes via the new response_id_matches helper.

• Transport routing: SseTransport::connect now receives headers directly and applies them via the refactored apply_safe_custom_headers helper; the connection path branches on is_legacy_sse_transport instead of guessing from the URL path.
• ID compatibility: next_id() returns a String; response_id_matches accepts either a string or unsigned-integer echo from the server, preserving interoperability with older MCP servers.
• CLI/TUI wiring: A new --transport flag is added to mcp add, and /mcp add sse <name> <url> is split into its own match arm in the TUI command parser, with corresponding tests and template updates.

Confidence Score: 5/5

Safe to merge — transport routing, header propagation, and ID-compatibility changes are all well-scoped and backed by new regression tests.

All changed code paths are exercised by targeted new tests. The only findings are stale help-text strings that do not affect runtime behavior.

crates/tui/src/commands/mcp.rs — three user-facing error/usage strings were not updated to mention the new sse subcommand.

Important Files Changed

Filename	Overview
crates/tui/src/commands/mcp.rs	Splits the combined `"http"
crates/tui/src/mcp.rs	Core MCP transport changes: adds transport field to McpServerConfig, refactors header application into apply_safe_custom_headers, adds headers propagation to SseTransport, changes JSON-RPC IDs to strings, and adds response_id_matches for backward-compatible numeric echoes. Logic looks correct and well-tested.
crates/tui/src/main.rs	Adds --transport CLI flag with inline validation; propagates the field through template generation and config insertion. Transport validation is duplicated inline rather than reusing validate_mcp_transport, but both paths produce identical errors.
crates/tui/src/tui/app.rs	Adds transport: Option<String> to McpUiAction::AddHttp struct field — a straightforward, mechanical change.
crates/tui/src/tui/ui.rs	Updates two add_server_config call sites to pass the new transport parameter; no logic changes beyond wiring the field through.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[McpConnection::connect] --> B{is_legacy_sse_transport?}
    B -- yes --> C[SseTransport::connect\nGET /sse + custom headers]
    B -- no --> D[HttpTransport::new\nStreamable HTTP]
    C --> E[run_sse_loop\napply_safe_custom_headers on GET]
    E --> F[Receive endpoint event]
    F --> G[SseTransport::send\napply_safe_custom_headers on POST]
    D --> H[try_establish_session GET]
    H --> I[StreamableHttpTransport::send\nPOST with headers + session-id]
    J[next_id to String] --> K[JSON-RPC request with string id]
    K --> L[response_id_matches\naccept string OR u64 echo]

Loading

_{Reviews (4): Last reviewed commit: "Improve MCP SSE error diagnostics" | Re-trigger Greptile}

[gemini-code-assist]

Code Review

This pull request transitions MCP JSON-RPC request IDs from numeric values to strings to improve compatibility with various gateways, while maintaining support for numeric echoes. It also routes legacy SSE endpoint URLs directly to SseTransport instead of HttpTransport. The review feedback highlights a potential issue where custom headers (such as authentication) are ignored when bypassing HttpTransport for legacy SSE connections. Additionally, several performance optimizations are suggested to pass references instead of cloning string IDs when constructing JSON payloads.

[gemini-code-assist]

By bypassing HttpTransport and directly connecting via SseTransport::connect for legacy SSE URLs, any custom headers configured in config.headers (such as Authorization or API keys) will be completely ignored. SseTransport currently does not accept or apply custom headers in its connection loop or message sending. If a legacy SSE server is behind an API gateway or requires authentication headers, the connection will fail. Consider updating SseTransport to accept and apply these custom headers.

[gemini-code-assist]

You can avoid cloning init_id here by passing a reference &init_id to the serde_json::json! macro. Since serde_json::json! only needs to serialize the value, a reference is sufficient and avoids an unnecessary heap allocation. This also leaves init_id owned and available to be passed to self.recv(init_id) later without cloning.

Suggested change

	"id": init_id.clone(),
	"id": &init_id,

[gemini-code-assist]

Avoid cloning list_id by passing a reference &list_id to the serde_json::json! macro.

Suggested change

	"id": list_id.clone(),
	"id": &list_id,

[gemini-code-assist]

Avoid cloning list_id by passing a reference &list_id to the serde_json::json! macro.

Suggested change

	"id": list_id.clone(),
	"id": &list_id,

[gemini-code-assist]

Avoid cloning list_id by passing a reference &list_id to the serde_json::json! macro.

Suggested change

	"id": list_id.clone(),
	"id": &list_id,

[gemini-code-assist]

Avoid cloning list_id by passing a reference &list_id to the serde_json::json! macro.

Suggested change

	"id": list_id.clone(),
	"id": &list_id,

[gemini-code-assist]

Avoid cloning call_id by passing a reference &call_id to the serde_json::json! macro.

Suggested change

	"id": call_id.clone(),
	"id": &call_id,

[zhuangbiaowei]

Addressed the review feedback in dcbb3d72:

• legacy SseTransport now carries configured custom headers and applies them to both the initial SSE GET and message POST
• shared the safe custom-header filtering path across Streamable HTTP and legacy SSE paths
• replaced JSON-RPC id clones in json! payloads with references
• added regression coverage for custom headers on legacy SSE GET/POST

Verification:

• cargo test -p codewhale-tui mcp
• cargo run -p codewhale-tui -- mcp validate