rpc/mcp, cmd/mcp: add streamable HTTP transport - #22624
Conversation
Serve streamable HTTP (MCP spec 2025-03-26, which deprecated SSE) at /mcp on the same listener as the existing SSE endpoints, for both the embedded server (--mcp.addr/--mcp.port) and the standalone mcp binary (--transport http; sse kept as a deprecated alias). SSE clients keep working unchanged at /sse + /message. MCPTransport.ServeSSE becomes ListenAndServe: the mux-owned http.Server replaces the WithHTTPServer pre-wiring hack, and SSE sessions are closed explicitly on shutdown so Shutdown does not hang on live event streams. The NonBlockingAcquire request-context wrap now covers both transports.
There was a problem hiding this comment.
Pull request overview
Adds support for MCP “streamable HTTP” transport served at /mcp while keeping legacy SSE compatibility (/sse + /message) on the same listener, for both embedded MCP (Erigon node) and the standalone mcp binary.
Changes:
- Replace the previous SSE-only serving entrypoint with a single HTTP server/mux that hosts both streamable HTTP (
/mcp) and SSE (/sse+/message). - Update embedded node startup + standalone CLI (
--transport http, withsseas a deprecated alias) to use the new unified HTTP transport. - Update docs and add a transport-level test validating both endpoints are served from one port.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| rpc/mcp/transport_test.go | New test verifying /mcp (streamable HTTP) and /sse are both served from ListenAndServe. |
| rpc/mcp/standalone.go | Renames/rewires standalone HTTP serving entrypoint to ListenAndServe. |
| rpc/mcp/mcp.go | Updates transport interface; implements unified serveHTTP mux that serves both transports and applies request-context wrapping. |
| rpc/mcp/mcp_test.go | Removes the old SSE-handler wiring test superseded by the new combined-transport test. |
| node/eth/backend.go | Embedded MCP now starts via ListenAndServe and logs both endpoints. |
| docs/site/docs/fundamentals/mcp.mdx | Docs updated to recommend streamable HTTP at /mcp, with SSE kept for older clients. |
| cmd/mcp/main.go | Adds --transport http (and keeps sse as deprecated alias), updates help text and serving path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… alias Tie request contexts to the server context via BaseContext so open GET event streams end on cancellation instead of stalling http.Server.Shutdown until its 5s deadline (which surfaced as a spurious 'context deadline exceeded' on every clean shutdown with a connected client); force-close as a fallback if Shutdown still times out. Also route the trailing-slash /mcp/ form to the streamable handler, and update cmd/mcp/README.md for the transport change.
…rface; bound header wait in stream test
eff0ca3 to
d1c0225
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
rpc/mcp/mcp.go:1015
- In serveHTTP's ctx-cancel shutdown path, an httpServer.Shutdown error is silently ignored (the function may return nil even if shutdown timed out/failed). That makes shutdown failures invisible to callers and can mask hung connections.
sse.CloseSessions()
if err := httpServer.Shutdown(shutdownCtx); err != nil {
_ = httpServer.Close()
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
rpc/mcp/mcp.go:1021
- In the ctx-cancel shutdown path, any error returned by httpServer.Shutdown is currently dropped (it only triggers a Close). This can mask real shutdown failures (e.g. context deadline exceeded) and make diagnosing stuck connections harder. Return the shutdown error after joining the serve goroutine (unless it’s ErrServerClosed).
shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
sse.CloseSessions()
if err := httpServer.Shutdown(shutdownCtx); err != nil {
_ = httpServer.Close()
}
// Join the serve goroutine; this also surfaces a bind/serve error
// that raced with the cancellation instead of dropping it.
if err := <-errCh; err != nil && !errors.Is(err, http.ErrServerClosed) {
return err
}
return nil
…on /message in endpoint logs
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
rpc/mcp/mcp.go:1020
- In the ctx-cancellation shutdown path, the error from httpServer.Shutdown is ignored. If Shutdown times out or otherwise fails, serveHTTP can still return nil (e.g., if the serve goroutine returns http.ErrServerClosed), which masks an unsuccessful/partial shutdown and makes callers think the server stopped cleanly.
if err := httpServer.Shutdown(shutdownCtx); err != nil {
_ = httpServer.Close()
}
…ementation (#22625) Stacked on #22624 (retargets to `main` when that merges). The MCP package carried three parallel implementations of the same ~60 tools: Go-API handlers for the embedded server (`mcp.go`), raw JSON-RPC handlers for the standalone proxy (`standalone.go`), and a separate tool-spec list (`tools.go`) — plus two copies of the resource handlers. This PR replaces all of it with a single declarative call table (`calls.go`) that derives both the tool schema and the handler for every tool, dispatched through a minimal `rpcCaller` interface. **Net −1,700 lines.** How each mode gets its `rpcCaller`: - **Proxy mode** (`mcp --rpc.url/--port`): the remote `rpc.Client`, as before. - **Embedded** (`--mcp.addr/--mcp.port`) and **datadir** (`mcp --datadir`): build the same `jsonrpc.APIList` as rpcdaemon, register it into an `rpc.Server`, connect with the new `rpc.DialInProcWithContext` — the connection context carries `kv.WithNonBlockingAcquire`, replacing the per-transport context hooks (stdio `SetContextFunc` + SSE/streamable request wrappers). Why this shape (beyond the line count): - Adding an API namespace to MCP becomes a cfg-list entry plus table rows — this is the base for the upcoming txpool / debug+trace / net+admin tool PRs. - Erigon's debug/trace APIs stream their responses and are impractical to call through Go interfaces; through the rpc layer they just work. - Embedded MCP now uses a fixed namespace list (`eth`, `erigon`, `ots`) independent of the user's `--http.api`, built in `Init` next to the rpcdaemon apiList (its former home in `New` predated some of its dependencies, e.g. `heimdallService`). Behavior deltas (intentional): - `eth_getStorageValues` is available in all modes (was embedded-only). - Metrics tools are gated by a runtime flag; `mcp --datadir` now reports metrics unavailable instead of returning the mcp process's own registry. - `eth_getBlockReceipts` on an empty block renders `[]` instead of "not found" (null still says not found). - `eth_getStorageValues` renders the raw JSON result map instead of the old padded per-slot text listing. - Follow-up review round: input-schema validation (mistyped args are rejected instead of silently becoming defaults), plain decimal block numbers accepted everywhere, defaulted params no longer marked required, missing-block count tools say "Block not found", HTTP-RPC and MCP share one BaseApi cache set, and the MCP goroutine joins the shutdown accounting. Verified end-to-end against a live chiado node in proxy mode: initialize → `eth_blockNumber`, `eth_chainId` (network-name resolution), `erigon_blockNumber` (omitted optional arg), `eth_getBalance`, `eth_getLogs` (filter-object builder) over `/mcp`.
Conflicts resolved: - node/eth/backend.go: combine the eager witness-cache setup (this branch) with the MCP streamable-HTTP refactor from main (#22624). Keep the witness-cache block and main's shared-APIList/apisForNamespaces + MCP server block; the single APIList call now passes both trailing params (testingEntry from main, witnessCache from this branch). - cmd/mcp/main.go: main rewrote the MCP path to serve via an in-process rpc.Server + DialInProc (#22624), superseding the ethAPI/erigonAPI/otsAPI extraction on this branch. Kept main's architecture and bumped the APIList call to the merged 14-arg signature (nil, nil). - rpc/jsonrpc/witness_cache_builder.go (non-conflict, merged clean but stale against main): db/services -> db/dbservices package rename, and NewBaseApi's positional args consolidated into NewBaseApiConfig(cfg); engine param widened to rules.Engine to match.
Serve streamable HTTP (the current MCP transport, spec 2025-03-26 — which deprecated SSE) at
/mcpon the same listener as the existing SSE endpoints. Applies to both the embedded server (--mcp.addr/--mcp.port, default127.0.0.1:8553) and the standalonemcpbinary (--transport http;ssekept as a deprecated alias). Existing SSE clients keep working unchanged at/sse+/message.Details:
MCPTransport.ServeSSEbecomesListenAndServe: one mux-ownedhttp.Serverhosts both transports, replacing theWithHTTPServerpre-wiring hack.http.Server.Shutdowndoes not hang on live event streams.kv.WithNonBlockingAcquirerequest-context wrap (fail-fastBeginRo) now covers both transports in embedded mode.claude mcp add --transport http erigon http://127.0.0.1:8553/mcp.New tools-level clients can connect with streamable HTTP while nothing changes for current setups.
Verified end-to-end against a live node:
initialize→ session →tools/call eth_blockNumberover/mcp, and/ssestill serving event streams.