Skip to content

fix(mcp): Use SDK web-standard transport for copilot mcp - #4320

Merged
TheodoreSpeaks merged 2 commits into
stagingfrom
fix/recategorize-copilot-mcp-fault
Apr 28, 2026
Merged

fix(mcp): Use SDK web-standard transport for copilot mcp #4320
TheodoreSpeaks merged 2 commits into
stagingfrom
fix/recategorize-copilot-mcp-fault

Conversation

@TheodoreSpeaks

@TheodoreSpeaksTheodoreSpeaks commented Apr 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Our mcp copilot was broken, consistently returning 5xx. This was due to upgrading mcp sdk versions. Since now the sdk supports WebStandardStreamableHTTPServerTransport, removed the shims and directly used that class instead.

When it broke

Commit 45bf39696"fix(deps): bump drizzle-orm 0.45.2 + adopt MCP SDK 1.25.3 native types (#4252)" — merged 2026-04-21.

That PR touched only apps/sim/package.json (one-line bump @modelcontextprotocol/sdk 1.20.2 → 1.25.3) and apps/sim/lib/copilot/tools/mcp/definitions.ts (cosmetic type cleanup). The route file itself was not modified. The shim there had been working untouched since 2026-02-09.

Why the PR review didn't catch it

The SDK's StreamableHTTPServerTransport was silently re-architected upstream:

  • Same export name, same handleRequest(req, res, parsedBody) signature → TypeScript saw nothing.
  • 1.20.2: self-contained Node implementation using raw-body + node:crypto. Only ever read req.headers.*. Our minimal shim { method, headers } was sufficient.
  • 1.25.3: "a thin wrapper around WebStandardStreamableHTTPServerTransport" using @hono/node-server's getRequestListener. For any non-GET/HEAD request, Hono calls incoming.on("end", …) (listener.js:598).
  • Our shim has no .onTypeError → caught by Hono → routed to handleFetchErrorResponse(null, { status: 500 }) → written to our NextResponseCapture via writeHead(500, …) → returned by our route as if it were a normal response.
  • Our route's catch block never saw the error (which is why the "recategorize" framing of this branch was misleading — there was no fault to recategorize, the route handler never ran).
  • No route.test.ts under apps/sim/app/api/mcp/copilot/ — nothing in CI round-tripped a POST through the transport, so the regression slipped to prod.

The fix

Swap the broken Node-style transport for the SDK's web-standard transport, which Next.js can drive directly (NextRequest already extends Web Request)

Also drops the now-unused NextResponseCapture shim (~190 lines) and normalizeRequestHeaders helper. The catch block additionally maps client-aborted requests to 499 (ConnectionClosed) since aborts do legitimately reach it now (unlike everything else, which the SDK handles internally).

Verification (local)

BeforeAfter
initialize POST500, empty body, text/plain200, full JSON-RPC envelope, application/json
tools/list POSTunreachable200, all 23 tools (list_workspaces, create_workflow, sim_workflow, …)
Catch block firesnever (synthetic 500 from Hono)only on real exceptions

Confirmed end-to-end with claude mcp add --transport http sim-copilot http://localhost:3000/api/mcp/copilot --header "x-api-key: …"Connected.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation
  • Other: ___________

Testing

  • Tested locally. Initialize handshake returns 200 with proper MCP server info; tools/list returns all 23 tools.
  • Validated claude mcp add --transport http sim-copilot http://localhost:3000/api/mcp/copilot succeeds with Connected.

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercelBot commented Apr 28, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
docsSkippedSkippedApr 28, 2026 7:15pm

Request Review

@cursor

cursorBot commented Apr 28, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes request/response handling for the MCP copilot endpoint by swapping transport implementations and altering how responses are produced, which could affect streaming/timeout behavior. Also reclassifies aborted requests to 499, reducing noisy 5xxs but touching error-path behavior.

Overview
Reduces spurious 5xxs from /api/mcp/copilot by detecting client-aborted requests (AbortError/aborted signal) and returning a JSON-RPC ConnectionClosed error with HTTP 499 instead of 500.

Simplifies MCP request handling by switching to WebStandardStreamableHTTPServerTransport and removing the custom NextResponseCapture/header normalization adapter, returning the transport’s Response directly.

Reviewed by Cursor Bugbot for commit 71c906a. Bugbot is set up for automated code reviews on this repo. Configure here.

@greptile-apps

greptile-appsBot commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR reverts the MCP copilot route to use WebStandardStreamableHTTPServerTransport from the SDK, replacing the custom NextResponseCapture adapter class and normalizeRequestHeaders helper that were introduced in commit 45bf396 and caused 5xx failures. The simplification removes ~185 lines of adapter boilerplate and adds a 499 response for client-cancelled requests in the outer catch.

Confidence Score: 5/5

Safe to merge — straightforward transport swap that removes a broken custom adapter; only finding is a P2 abort edge case.

All findings are P2 (minor inconsistency in abort handling between inner/outer catch). No P0 or P1 issues found. The core change is a clean simplification that delegates to well-tested SDK internals.

No files require special attention.

Important Files Changed

FilenameOverview
apps/sim/app/api/mcp/copilot/route.tsReplaces the custom StreamableHTTPServerTransport + NextResponseCapture adapter with WebStandardStreamableHTTPServerTransport, greatly simplifying the handler. Adds a 499 abort response in the outer catch. One minor issue: inner try/catch around request.json() still swallows AbortError before the new 499 path.

Sequence Diagram

sequenceDiagram
participant Client
participant POST Handler
participant handleMcpRequestWithSdk
participant WebStandardTransport
participant MCP Server
Client->>POST Handler: POST /api/mcp/copilot
POST Handler->>POST Handler: Auth check
POST Handler->>POST Handler: request.json()
POST Handler->>handleMcpRequestWithSdk: (request, parsedBody)
handleMcpRequestWithSdk->>MCP Server: buildMcpServer(request.signal)
handleMcpRequestWithSdk->>WebStandardTransport: new WebStandardStreamableHTTPServerTransport
handleMcpRequestWithSdk->>MCP Server: server.connect(transport)
handleMcpRequestWithSdk->>WebStandardTransport: transport.handleRequest(request, {parsedBody})
WebStandardTransport->>MCP Server: dispatch tool call
MCP Server-->>WebStandardTransport: tool result
WebStandardTransport-->>handleMcpRequestWithSdk: Response
handleMcpRequestWithSdk-->>POST Handler: Response
POST Handler-->>Client: JSON response
note over POST Handler: On AbortError → 499
note over POST Handler: On other error → 500
Loading

Reviews (2): Last reviewed commit: "fix(mcp): fix copilot mcp response" | Re-trigger Greptile

@TheodoreSpeaksTheodoreSpeaks changed the title fix(api): return 499 on copilot mcp user abortsfix(mcp): Use SDK web-standard transport for copilot mcp Apr 28, 2026
@TheodoreSpeaks

Copy link
Copy Markdown
CollaboratorAuthor

@greptile review

@TheodoreSpeaks
TheodoreSpeaks merged commit 69dc2f0 into stagingApr 28, 2026
14 checks passed
@TheodoreSpeaks
TheodoreSpeaks deleted the fix/recategorize-copilot-mcp-fault branch April 28, 2026 20:05
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@TheodoreSpeaks