feat: migrate aibridge injected-MCP proxy to official MCP Go SDK - #28060
Merged
Conversation
This was referenced Aug 12, 2026
ibetitsmike
changed the title
ibetitsmike
marked this pull request as ready for review
August 13, 2026 09:42
ThomasK33
approved these changes
Aug 13, 2026
![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: 587cce04ae
ℹ️ 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".
Comment on lines
+35
to
+36
| for k, v := range h.headers { | ||
| clone.Header.Set(k, v) |
There was a problem hiding this comment.
When an MCP endpoint redirects to another origin, http.Client normally strips sensitive headers such as Authorization, but the redirected request passes through this transport again and the loop restores every configured header. A redirect to an authentication service, CDN, or other host therefore discloses the user's MCP OAuth token; restrict injection to the configured MCP origin or enforce equivalent redirect filtering.
Useful? React with 👍 / 👎.
Comment on lines
+71
to
+75
| if p.session != nil { | ||
| if err := p.session.Close(); err != nil { | ||
| p.logger.Debug(ctx, "failed to close previous MCP session", slog.Error(err)) | ||
| } | ||
| p.session = nil |
There was a problem hiding this comment.
When Init is called after a previous successful initialization and the new connection or ListTools call fails, this closes the old session but leaves p.tools populated with Tool values whose clients point to that closed session. ListTools then continues advertising those stale tools, and ServerProxyManager.Init aggregates them even though initialization returned an error, so every subsequent invocation fails; clear the old catalog when retiring the session or keep the old session alive until its replacement is fully initialized.
Useful? React with 👍 / 👎.
ibetitsmike
force-pushed
the
mike/mcp-go-sdk/aibridge-proxy
branch
from
August 13, 2026 09:50
587cce0 to
e39c84e
Compare
ibetitsmike
added a commit
that referenced
this pull request
Aug 13, 2026
## 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.
ibetitsmike
force-pushed
the
mike/mcp-go-sdk/aibridge-proxy
branch
from
August 13, 2026 10:02
e39c84e to
f48be87
Compare
ibetitsmike
added a commit
that referenced
this pull request
Aug 13, 2026
…al Go SDK (#28058) ## Stack Context PR 3 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 The chatd external MCP client (admin-configured MCP servers used by Agent chat) now holds `*mcp.ClientSession` connections created via `mcp.NewClient` and `Client.Connect`, with `StreamableClientTransport` or `SSEClientTransport` per server config. - Auth and identity headers are injected through a custom `http.RoundTripper` because the official SDK has no per-header transport options. - Tool input schemas are extracted from the SDK's `map[string]any` decoding. - Content conversion handles the official pointer content types; the SDK decodes blob resources into raw bytes, so binary content is handled without an extra base64 round trip. - Test fixtures are official stateless Streamable HTTP servers. > Mux created this PR on Mike's behalf.
ibetitsmike
force-pushed
the
mike/mcp-go-sdk/aibridge-proxy
branch
from
August 13, 2026 10:11
f48be87 to
9d21879
Compare
pull Bot
pushed a commit
to TKaxv-7S/coder
that referenced
this pull request
Aug 13, 2026
## Stack Context PR 1 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, adding MCP 2026-07-28 support while keeping compatibility with clients speaking 2024-11-05 through 2025-06-18. Stack: coder#28056 -> coder#28057 -> coder#28058 -> coder#28059 -> coder#28060 -> coder#28061 ## Why The coderd Streamable HTTP MCP server (`/api/experimental/mcp/http`) is the foundation layer: it introduces the official SDK dependency and the shared `RegisterSDKTool` helper the CLI server reuses. - The server runs the SDK handler in stateless mode with `JSONResponse: true`, preserving the previous `application/json` POST wire format. GET and DELETE return 405, and no `Mcp-Session-Id` is issued, both permitted by the Streamable HTTP spec. - `DisableLocalhostProtection` is set because coderd commonly listens on loopback behind a reverse proxy with a public Host header; the endpoint's bearer authentication is the relevant access control. - Tool registration builds raw JSON object schemas and omits empty `required`, keeping `tools/list` output byte-identical to the previous server (verified with a golden comparison). - SDK logs are adapted to `cdr.dev/slog/v3`; only warnings and errors are forwarded because the SDK logs several INFO lines per stateless request. - Tests cover the modern 2026-07-28 flow, legacy 2025-06-18 initialize, unsupported protocol version rejection (`-32022`), and non-POST method behavior. ## Known behavior deltas vs the old endpoint Both deltas come from the SDK enforcing the Streamable HTTP spec where mark3labs was lenient, on an experimental endpoint: - POST requests whose `Accept` header lists `application/json` without `text/event-stream` are now rejected with 400 (the spec requires clients to list both; a missing `Accept` header is still tolerated). mark3labs did not validate `Accept` at all. - The old server generated an unvalidated `Mcp-Session-Id` response header; the stateless SDK handler issues none. Clients that merely echo the header back are unaffected. ## Validation Beyond unit/integration tests, a remote dogfood UAT ran protocol conformance against a live dev server built from the stack tip: version negotiation matrix (2024-11-05 through bogus/omitted values), auth, session/method semantics, tool schema sanity, tools/call happy and error paths (unknown tool, schema-violating args, malformed JSON, jsonrpc "1.0"), and a concurrency smoke test. No 500s or connection drops; error shapes are clean JSON-RPC/HTTP errors. > Mux created this PR on Mike's behalf.
ibetitsmike
added a commit
that referenced
this pull request
Aug 13, 2026
…l Go SDK (#28059) ## Stack Context PR 4 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 The workspace agent MCP manager now stores `*mcp.ClientSession` per configured server. - stdio servers use `mcp.CommandTransport` with an `exec.Cmd` built from Coder's `agentexec.Execer`, preserving environment enrichment; the command uses the manager's parent context so a stdio subprocess outlives the connect handshake and stops when the session closes. - HTTP and SSE servers use header-injecting HTTP clients. - Binary tool content is re-encoded to base64 for the agent API because the official SDK decodes it into raw bytes. - The reload test now triggers config diffs via an environment variable because the official SDK drops connections on non-protocol stdout output (flags like `-test.v` made the fake server chatty). > Mux created this PR on Mike's behalf.
Replace the mark3labs client with the official SDK client for the deprecated injected-MCP proxy. The manual protocol-version handshake check and the 5s-close workaround are subsumed by SDK negotiation and session close. The proxy constructor takes an optional *http.Client instead of mark3labs transport options; auth headers ride an http.RoundTripper. Repeated Init now closes the previous session instead of leaking its transport. Test fixtures move to official SDK stateless servers, including the enterprise integration mock, which previously answered every POST with a canned initialize response that the stricter SDK client rejects. The mcpmock generate directive also pointed at the pre-vendoring aibridge module path; fix it and regenerate.
ibetitsmike
force-pushed
the
mike/mcp-go-sdk/aibridge-proxy
branch
from
August 13, 2026 10:29
9d21879 to
bda85fc
Compare
ibetitsmike
added a commit
that referenced
this pull request
Aug 13, 2026
## Stack Context PR 6 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 With every production surface migrated, this PR removes the mark3labs dependency entirely and converts the remaining test fixtures. - Migrates the remaining mark3labs test fixtures (coderd MCP e2e tests, chatd fixtures, mcpclient fixtures, and the Force On MCP policy tests) to official stateless SDK servers. - Removes `github.com/mark3labs/mcp-go` from `go.mod` and drops the corresponding dependabot ignore entry. Zero references remain repo-wide. - Updates the MCP docs for the 2026-07-28 protocol: stateless Streamable HTTP behavior, the supported 2024-11-05 through 2026-07-28 protocol range, and explicit non-features (resources, prompts, structured output, elicitation, MCP Tasks). - The e2e ping assertion is removed because MCP 2026-07-28 removed the ping method. > Mux created this PR on Mike's behalf.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack Context
PR 5 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 aibridge injected-MCP proxy now owns an official
*mcp.Client,*mcp.StreamableClientTransport, and*mcp.ClientSession.*http.Clientinstead of mark3labs options; the header-injecting wrapper shallow-copies a supplied client so its Timeout, Jar, and redirect policy survive.Connectand fails when no mutually supported version exists.Initcloses the previous session, and a failed tool fetch closes the just-created session so transports do not leak.aibridge/mcpmockis regenerated, and its stalego:generatesource path is corrected.