feat(coderd/x/chatd/mcpclient): migrate external MCP client to official Go SDK - #28058
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6255ea7720
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
When an authenticated MCP endpoint returns a redirect to another origin, net/http removes sensitive headers, but the redirected request passes through this wrapper again and these lines restore every configured header. This can disclose OAuth bearer tokens, API keys, custom secrets, and forwarded Coder identity headers to the redirect target; notably, RevokeOAuth2Token already installs an explicit redirect guard to prevent the same class of leak. Restrict header injection to the configured MCP origin or reject cross-origin redirects.
Useful? React with 👍 / 👎.
6255ea7 to
77e3912
Compare
…al Go SDK Replace the mark3labs client with the official SDK client for chatd's external MCP server connections. All four auth modes (oauth2, api_key, custom_headers, user_oidc) now inject headers through an http.RoundTripper on the transport's HTTPClient instead of per-header transport options; header keys still pass through http.Header.Set so case-insensitive collisions stay deterministic. The SDK negotiates the protocol version internally (2026-07-28 down to 2024-11-05), so older external servers keep working. Tool name prefixing, allow/deny filtering, model-intent wrapping, and content conversion are behavior-identical; the SDK decodes base64 image, audio, and blob payloads during unmarshal, so the manual decode paths are gone.
## Stack Context PR 2 of 6 in a stack that migrates every Coder MCP surface from the archived `github.com/mark3labs/mcp-go` library to the official `github.com/modelcontextprotocol/go-sdk` v1.7.0. Stack: #28056 -> #28057 -> #28058 -> #28059 -> #28060 -> #28061 ## Why `coder exp mcp server` (stdio) now uses the official SDK server with `mcp.IOTransport` over the invocation's stdin/stdout, and reuses the shared `coderd/mcp.RegisterSDKTool` helper from PR #28056 so both servers register tools identically. - A `nopWriteCloser` prevents the SDK from closing the invocation's stdout. - Tests send spec-compliant initialize params and `notifications/initialized` before `tools/list` because the official SDK enforces the protocol lifecycle. > Mux created this PR on Mike's behalf.
77e3912 to
ad75b38
Compare
2 participants
Stack Context
PR 3 of 6 in a stack that migrates every Coder MCP surface from the archived
github.com/mark3labs/mcp-golibrary to the officialgithub.com/modelcontextprotocol/go-sdkv1.7.0.Stack: #28056 -> #28057 -> #28058 -> #28059 -> #28060 -> #28061
Why
The chatd external MCP client (admin-configured MCP servers used by Agent chat) now holds
*mcp.ClientSessionconnections created viamcp.NewClientandClient.Connect, withStreamableClientTransportorSSEClientTransportper server config.http.RoundTripperbecause the official SDK has no per-header transport options.map[string]anydecoding.