Skip to content

fix(mcp): negotiate MCP 2025-11-25 so clients see invalid-argument errors - #18490

Open
mfazekas wants to merge 1 commit into
pingdotgg:mainfrom
mfazekas:fix/mcp-protocol-2025-11-25
Open

mfazekas wants to merge 1 commit into
pingdotgg:mainfrom
mfazekas:fix/mcp-protocol-2025-11-25

Conversation

@mfazekas

@mfazekas mfazekas commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Problem

T3's MCP server offers only protocol 2025-06-18, so every client is held to it, even when it asks for 2025-11-25. The Claude app (Anthropic/ClaudeAI) and Claude Code both ask for 2025-11-25. Under 2025-06-18, a tool call with invalid arguments fails as a JSON-RPC InvalidParams error. The Claude app shows that only as a generic "MCP tool call failed", so neither the user nor the model sees what was wrong. With strict params on t3_thread_read (#17823), any unknown param now ends that way.

Change

Offer 2025-11-25 alongside 2025-06-18 in layerMcpTransport. Effect's 2025-11-25 adapter already returns invalid arguments as an isError tool result carrying the validation message, as that spec revision requires. Clients that ask for 2025-06-18 still get it and keep its error behavior.

This replaces #18098, which patched Effect to return tool results under 2025-06-18 as well.

Scope and approval

There is no triaged issue for this. I hit it while using T3 from the Claude app: whenever the model passed a bad argument to a T3 tool, the app reported only "MCP tool call failed". The cause turned out to be the pinned protocol list. I think this qualifies as a small, focused fix without a prior issue because it is a one-line config change. It enables a protocol version that the Effect version T3 already ships implements and that current clients already request. Tool definitions and handlers don't change, and 2025-06-18 stays available for older clients.

Note: the single-version list looks like a leftover rather than a deliberate limit. It was added in #5331, when Effect beta.103 made protocols required and, as far as I can tell, v2025_06_18 was its only adapter. Later Effect versions added newer adapters, but the list was never revisited. I didn't find any discussion of it, so I may be missing a reason.

#16067 traces another failure to the same pinned list: clients that open with 2026-07-28 server/discover get a 400. This PR doesn't add 2026-07-28, so it doesn't fix that issue.

Verification

  • A new HTTP-level test in McpHttpServer.test.ts initializes with each version, then calls a tool with a wrong argument type. For 2025-11-25, the session negotiates 2025-11-25 and the call returns isError: true with "Invalid parameters for tool 'count': Expected number…". For 2025-06-18, the call still returns the -32602 error. Against the old protocol list, the test fails because the client is negotiated down to 2025-06-18.
  • All 289 tests under apps/server/src/mcp pass, and the server typecheck is clean.
  • I logged initialize on a running server and confirmed that the Claude app (Anthropic/ClaudeAI 1.0.0) and Claude Code (2.1.295) request 2025-11-25. With this change running, both negotiated 2025-11-25 (the server spans were @effect/mcp/2025-11-25/tools/call). A t3_thread_read call with limit: 500 showed both clients "Invalid parameters for tool 't3_thread_read': Expected a value less than or equal to 100 at ["limit"]". Before the change, the Claude app showed only "MCP tool call failed" for such calls.

Done with Claude Opus 5.5 (claude-opus-5-5) in Claude Code, run through T3 Code.

…rors

T3 offered only MCP 2025-06-18, so every client was held to it even when
it asked for 2025-11-25, as the Claude app and Claude Code both do. Under
2025-06-18 a tool call with invalid arguments fails as a JSON-RPC
InvalidParams error, which the Claude app shows only as a generic "tool
call failed", hiding the validation message from the user and the model.
Effect already reports these as isError tool results for 2025-11-25, so
offer that version and keep 2025-06-18 for older clients.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 12, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The PR changes production MCP negotiation so clients requesting 2025-11-25 receive different invalid-argument semantics, while preserving the legacy 2025-06-18 path. The behavior is narrowly tested, but this product-facing default negotiation change warrants human review.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 12, 2026 — with ChatGPT Codex Connector
@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 70d79cb0-ccda-4001-8379-98b726f87909




📥 Commits

Reviewing files that changed from the base of the PR and between e688034 and 723d587.





📒 Files selected for processing (2)
  • apps/server/src/mcp/McpHttpServer.test.ts
  • apps/server/src/mcp/McpHttpServer.ts




Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.






📝 Walkthrough
📝 Walkthrough
📝 Walkthrough
📝 Walkthrough

Walkthrough

The HTTP MCP transport now advertises protocol versions 2025-11-25 and 2025-06-18. A test checks the invalid-argument response for each version.

Changes

MCP protocol compatibility

Layer / File(s) Summary
Protocol support and invalid-argument responses
apps/server/src/mcp/McpHttpServer.ts, apps/server/src/mcp/McpHttpServer.test.ts
The transport advertises both protocol versions. The test checks for an error-marked tool result with an invalid-parameters message for 2025-11-25, and JSON-RPC error code -32602 for 2025-06-18.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge








Merge Risk: ⚪ Minimal · up to 723d5

The transport retains the older protocol and supports the newer version’s validation-message response. No actionable merge-blocking risk is established; normal checks remain appropriate.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 723d5

The newer protocol preserves legacy support and uses the existing authentication and tool-access controls. No introduced security weakness was identified. Compatibility for clients requesting other protocol versions remains unverified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed behavior is reachable by callers of the existing authenticated MCP endpoint. The examined request path retains credential-derived environment, caller and capability scope; no new unauthenticated entrypoint or broader handler authority was identified.

Trust Boundaries and Controls

  • observed — The HTTP middleware resolves the bearer credential through the session registry or client authenticator and rejects unresolved credentials before running the transport. It supplies invocation scope per request. Toolkit registration continues to use declared tool-access layers without capturing a registration-time caller scope.
  • observed — The newer failure conversion uses the same message fields that the legacy JSON-RPC mapper already returned. Toolkit registration also retains its existing handling that logs and scrubs unexpected internal failures. This is a client-visible representation change, not evidence that new internal error detail is exposed.

Resilience and Maintainability Implications

  • inferred — The lifecycle inspection found shared session binding, request tracking, cancellation and termination machinery for both adapters. Selecting the newer adapter does not introduce a separate ownership or cleanup transition; session termination continues to interrupt requests bound to that session. This conclusion is source-based, not runtime concurrency verification.





Pre-merge checks | Passed 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly and concisely describes the main change: negotiating MCP protocol version 2025-11-25 so clients receive invalid-argument errors.
Description check Passed The description covers the problem, change, scope and approval rationale, verification steps, observed results, limitations, and agent attribution. It matches the required template and explains the be…

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR







  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants