From aa5e6e90923a19fbebb041462e13ef3123da646c Mon Sep 17 00:00:00 2001 From: a-akimov Date: Sun, 1 Nov 2026 22:52:29 +0100 Subject: [PATCH 1/2] Fix what the clients put in a tool_result Three defects decided what the model received after a tool call: 1. A tools/call the server rejected with a JSON-RPC error ended the query in the Python, TypeScript, Go and Ruby clients, so the model never learned its call failed. It now goes back as a tool_result with is_error set, as the Rust client already did. 2. A result with structuredContent and no text block reached the model as an empty tool_result in the same four clients. Its JSON is now sent. 3. Non-text blocks were dropped in all five clients. Images in a type the Messages API accepts become base64 image blocks, text resources become their text, and anything else becomes a placeholder such as "[unsupported result type: audio]". All five clients use the same mapping. Python, TypeScript and Ruby catch only the SDK's server-error class (MCPError, ProtocolError, MCP::Client::ServerError); Go and Rust catch any error from the call. The tool-loop test gains a fourth query, "tool result contents", backed by three new mock-server tools (rejected_call, structured_only, non_text). The unfixed Python client fails it; all five fixed clients pass. Closes #273 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01NkL34oWhufSSqj9FbPx5dr --- mcp-client-go/README.md | 2 + mcp-client-go/main.go | 103 ++++++++++++++++++++++++++----- mcp-client-python/README.md | 2 + mcp-client-python/client.py | 60 ++++++++++++++++-- mcp-client-ruby/README.md | 2 + mcp-client-ruby/client.rb | 61 +++++++++++++++--- mcp-client-rust/README.md | 2 + mcp-client-rust/src/main.rs | 77 +++++++++++++++++------ mcp-client-typescript/README.md | 2 + mcp-client-typescript/index.ts | 87 ++++++++++++++++++++++---- tests/README.md | 9 ++- tests/helpers/mock-mcp-server.ts | 52 ++++++++++++++++ tests/helpers/mock-values.ts | 11 ++++ tests/helpers/tool-loop-test.ts | 87 ++++++++++++++++++++++++-- tests/smoke-test.sh | 8 ++- 15 files changed, 493 insertions(+), 72 deletions(-) create mode 100644 tests/helpers/mock-values.ts diff --git a/mcp-client-go/README.md b/mcp-client-go/README.md index 5cf29715..81b7bf4c 100644 --- a/mcp-client-go/README.md +++ b/mcp-client-go/README.md @@ -41,3 +41,5 @@ Without an `ANTHROPIC_API_KEY`, the client still connects, prints the server's t The SDK does not validate tool output, so this client compiles each declared `outputSchema` at connect time and checks results against it — the spec's client-side SHOULD. It uses [`jsonschema-go`](https://pkg.go.dev/github.com/google/jsonschema-go/jsonschema), already an SDK dependency. The two channels go to different readers: `Content` is forwarded to the model, while `StructuredContent` is used as data — the client counts the items it returns. See [Structured Content](https://modelcontextprotocol.io/specification/2026-07-28/server/tools#structured-content). + +Before a tool result goes to the model, the client converts it to the Messages API's block types. Text stays text, a PNG, JPEG, GIF or WebP image becomes an image block, and a text resource becomes its text. Anything else, such as audio, becomes a placeholder like `[unsupported result type: audio]`, so the model knows something was there. A result with structured content and no text block is sent as its JSON. A call the server rejects goes back to the model as a failed tool result, so it can correct itself, rather than ending the query. diff --git a/mcp-client-go/main.go b/mcp-client-go/main.go index efe6f54b..50b8f75a 100644 --- a/mcp-client-go/main.go +++ b/mcp-client-go/main.go @@ -3,6 +3,7 @@ package main import ( "bufio" "context" + "encoding/base64" "encoding/json" "fmt" "log" @@ -24,6 +25,77 @@ var model = anthropic.ModelClaudeSonnet5 const maxTokens = 10000 const maxToolTurns = 10 +// Image types the Messages API accepts. +var supportedImageTypes = map[string]bool{ + "image/jpeg": true, + "image/png": true, + "image/gif": true, + "image/webp": true, +} + +// toolResultContent translates an MCP tool result into content for an +// Anthropic tool_result. +// +// MCP and Anthropic content blocks differ, so each kind is converted. A kind +// the model cannot take becomes a placeholder rather than vanishing, so the +// model knows something was there. +func toolResultContent(result *mcp.CallToolResult) []anthropic.ToolResultBlockParamContentUnion { + var content []anthropic.ToolResultBlockParamContentUnion + text := func(s string) { + content = append(content, anthropic.ToolResultBlockParamContentUnion{OfText: &anthropic.TextBlockParam{Text: s}}) + } + + hasText := false + for _, block := range result.Content { + switch b := block.(type) { + case *mcp.TextContent: + hasText = true + text(b.Text) + case *mcp.ImageContent: + if !supportedImageTypes[b.MIMEType] { + text("[unsupported result type: image]") + continue + } + content = append(content, anthropic.ToolResultBlockParamContentUnion{OfImage: &anthropic.ImageBlockParam{ + Source: anthropic.ImageBlockParamSourceUnion{OfBase64: &anthropic.Base64ImageSourceParam{ + // The SDK holds the decoded bytes; the API wants base64. + Data: base64.StdEncoding.EncodeToString(b.Data), + MediaType: anthropic.Base64ImageSourceMediaType(b.MIMEType), + }}, + }}) + case *mcp.EmbeddedResource: + if b.Resource == nil || b.Resource.Blob != nil { + text("[unsupported result type: resource]") + continue + } + text(b.Resource.Text) + default: + text(fmt.Sprintf("[unsupported result type: %s]", contentType(block))) + } + } + + // A structured result SHOULD also carry serialized JSON in a text block, but + // only SHOULD, so serialize it when no text block came with it. + if !hasText && result.StructuredContent != nil { + if data, err := json.Marshal(result.StructuredContent); err == nil { + text(string(data)) + } + } + + return content +} + +// contentType returns an MCP content block's wire type, such as "audio". +func contentType(block mcp.Content) string { + var wire struct { + Type string `json:"type"` + } + if data, err := block.MarshalJSON(); err == nil && json.Unmarshal(data, &wire) == nil && wire.Type != "" { + return wire.Type + } + return "unknown" +} + type MCPClient struct { anthropic *anthropic.Client session *mcp.ClientSession @@ -205,7 +277,16 @@ func (c *MCPClient) ProcessQuery(ctx context.Context, query string) (string, err Arguments: toolUseBlock.Input, }) if err != nil { - return "", fmt.Errorf("tool call %s failed: %w", toolUseBlock.Name, err) + // A rejected call is the model's mistake to correct, so it goes + // back as a failed tool_result rather than ending the query. + // Every tool_use still needs a matching result. + finalText = append(finalText, fmt.Sprintf("[%s failed: %v]", toolUseBlock.Name, err)) + anthropicToolResults = append(anthropicToolResults, anthropic.NewToolResultBlock( + toolUseBlock.ID, + fmt.Sprintf("Tool call %s failed: %v", toolUseBlock.Name, err), + true, + )) + continue } if err := c.validateToolOutput(toolUseBlock.Name, mcpToolResult); err != nil { @@ -217,19 +298,13 @@ func (c *MCPClient) ProcessQuery(ctx context.Context, query string) (string, err finalText = append(finalText, fmt.Sprintf("[%s returned %d items]", toolUseBlock.Name, len(items))) } - // Content is a list of block types; forward only the text ones. - var texts []string - for _, block := range mcpToolResult.Content { - if text, ok := block.(*mcp.TextContent); ok { - texts = append(texts, text.Text) - } - } - - anthropicToolResults = append(anthropicToolResults, anthropic.NewToolResultBlock( - toolUseBlock.ID, - strings.Join(texts, "\n"), - mcpToolResult.IsError, - )) + anthropicToolResults = append(anthropicToolResults, anthropic.ContentBlockParamUnion{ + OfToolResult: &anthropic.ToolResultBlockParam{ + ToolUseID: toolUseBlock.ID, + Content: toolResultContent(mcpToolResult), + IsError: anthropic.Bool(mcpToolResult.IsError), + }, + }) } // Append the assistant's response and all tool results, in one user diff --git a/mcp-client-python/README.md b/mcp-client-python/README.md index fff5cbe0..6e970afb 100644 --- a/mcp-client-python/README.md +++ b/mcp-client-python/README.md @@ -43,4 +43,6 @@ Without an `ANTHROPIC_API_KEY`, the client still connects, prints the server's t The two channels go to different readers: `content` is forwarded to the model, while `structured_content` is used as data — when a tool returns an array, the client counts its items rather than re-reading the prose. See [Structured Content](https://modelcontextprotocol.io/specification/2026-07-28/server/tools#structured-content). +Before a tool result goes to the model, the client converts it to the Messages API's block types. Text stays text, a PNG, JPEG, GIF or WebP image becomes an image block, and a text resource becomes its text. Anything else, such as audio, becomes a placeholder like `[unsupported result type: audio]`, so the model knows something was there. A result with structured content and no text block is sent as its JSON. A call the server rejects goes back to the model as a failed tool result, so it can correct itself, rather than ending the query. + `Client(server_params, mode="auto")` launches the server from its `StdioServerParameters`, then probes `server/discover` and falls back to the `2025-11-25` handshake; `client.protocol_version` reports which era you got. See [Protocol versions](https://py.sdk.modelcontextprotocol.io/v2/protocol-versions/). diff --git a/mcp-client-python/client.py b/mcp-client-python/client.py index 80435c5c..4816f4b6 100644 --- a/mcp-client-python/client.py +++ b/mcp-client-python/client.py @@ -1,12 +1,13 @@ import asyncio +import json import os from contextlib import AsyncExitStack from pathlib import Path from anthropic import Anthropic from dotenv import load_dotenv -from mcp import Client, StdioServerParameters -from mcp_types import TextContent +from mcp import Client, MCPError, StdioServerParameters +from mcp_types import CallToolResult, EmbeddedResource, ImageContent, TextContent, TextResourceContents load_dotenv() # load environment variables from .env @@ -15,6 +16,39 @@ # plus the reply, so leave room for both. MAX_TOKENS = 10000 MAX_TOOL_TURNS = 10 +# Image types the Messages API accepts. +SUPPORTED_IMAGE_TYPES = {"image/jpeg", "image/png", "image/gif", "image/webp"} + + +def tool_result_content(result: CallToolResult) -> list[dict]: + """Translate an MCP tool result into content for an Anthropic tool_result. + + MCP and Anthropic content blocks differ, so each kind is converted. A kind + the model cannot take becomes a placeholder rather than vanishing, so the + model knows something was there. + """ + content: list[dict] = [] + for block in result.content: + if isinstance(block, TextContent): + content.append({"type": "text", "text": block.text}) + elif isinstance(block, ImageContent) and block.mime_type in SUPPORTED_IMAGE_TYPES: + content.append( + { + "type": "image", + "source": {"type": "base64", "media_type": block.mime_type, "data": block.data}, + } + ) + elif isinstance(block, EmbeddedResource) and isinstance(block.resource, TextResourceContents): + content.append({"type": "text", "text": block.resource.text}) + else: + content.append({"type": "text", "text": f"[unsupported result type: {block.type}]"}) + + # A structured result SHOULD also carry serialized JSON in a text block, but + # only SHOULD, so serialize it when no text block came with it. + if result.structured_content is not None and not any(isinstance(block, TextContent) for block in result.content): + content.append({"type": "text", "text": json.dumps(result.structured_content)}) + + return content class MCPClient: @@ -90,20 +124,34 @@ async def process_query(self, query: str) -> str: tool_results = [] for tool_use in tool_uses: - # call_tool validates the result against the declared schema. - result = await self.client.call_tool(tool_use.name, tool_use.input) final_text.append(f"[Calling tool {tool_use.name} with args {tool_use.input}]") + try: + # call_tool validates the result against the declared schema. + result = await self.client.call_tool(tool_use.name, tool_use.input) + except MCPError as error: + # A rejected call is the model's mistake to correct, so it goes + # back as a failed tool_result rather than ending the query. + # Every tool_use still needs a matching result. + final_text.append(f"[{tool_use.name} failed: {error}]") + tool_results.append( + { + "type": "tool_result", + "tool_use_id": tool_use.id, + "content": f"Tool call {tool_use.name} failed: {error}", + "is_error": True, + } + ) + continue # structured_content is data the application can use directly. if isinstance(result.structured_content, list): final_text.append(f"[{tool_use.name} returned {len(result.structured_content)} items]") - # content is a list of block types; forward only the text ones. tool_results.append( { "type": "tool_result", "tool_use_id": tool_use.id, - "content": "\n".join(block.text for block in result.content if isinstance(block, TextContent)), + "content": tool_result_content(result), "is_error": bool(result.is_error), } ) diff --git a/mcp-client-ruby/README.md b/mcp-client-ruby/README.md index e61119ae..ac05606b 100644 --- a/mcp-client-ruby/README.md +++ b/mcp-client-ruby/README.md @@ -38,4 +38,6 @@ The spec says clients SHOULD validate structured results against the schema the The two channels go to different readers: `content` is forwarded to the model, while `structured_content` is used as data — when a tool returns an array, the client counts its items rather than re-reading the prose. See [Structured Content](https://modelcontextprotocol.io/specification/2026-07-28/server/tools#structured-content). +Before a tool result goes to the model, the client converts it to the Messages API's block types. Text stays text, a PNG, JPEG, GIF or WebP image becomes an image block, and a text resource becomes its text. Anything else, such as audio, becomes a placeholder like `[unsupported result type: audio]`, so the model knows something was there. A result with structured content and no text block is sent as its JSON. A call the server rejects goes back to the model as a failed tool result, so it can correct itself, rather than ending the query. + `MCP::Client::Stdio` offers `2026-07-28` by default, so no negotiation setup is needed here. diff --git a/mcp-client-ruby/client.rb b/mcp-client-ruby/client.rb index f14d81be..4390cf72 100644 --- a/mcp-client-ruby/client.rb +++ b/mcp-client-ruby/client.rb @@ -11,6 +11,8 @@ class MCPClient # plus the reply, so leave room for both. MAX_TOKENS = 10000 MAX_TOOL_TURNS = 10 + # Image types the Messages API accepts. + SUPPORTED_IMAGE_TYPES = ["image/jpeg", "image/png", "image/gif", "image/webp"].freeze def initialize @mcp_client = nil @@ -76,6 +78,35 @@ def cleanup private + # Translate an MCP tool result into content for an Anthropic tool_result. + # + # MCP and Anthropic content blocks differ, so each kind is converted. A kind + # the model cannot take becomes a placeholder rather than vanishing, so the + # model knows something was there. + def tool_result_content(result) + blocks = Array(result["content"]) + content = blocks.map do |block| + if block["type"] == "text" + { type: "text", text: block["text"] } + elsif block["type"] == "image" && SUPPORTED_IMAGE_TYPES.include?(block["mimeType"]) + { type: "image", source: { type: "base64", media_type: block["mimeType"], data: block["data"] } } + elsif block["type"] == "resource" && block.dig("resource", "text") + { type: "text", text: block.dig("resource", "text") } + else + { type: "text", text: "[unsupported result type: #{block["type"]}]" } + end + end + + # A structured result SHOULD also carry serialized JSON in a text block, but + # only SHOULD, so serialize it when no text block came with it. + structured = result["structuredContent"] + if !structured.nil? && blocks.none? { |block| block["type"] == "text" } + content << { type: "text", text: structured.to_json } + end + + content + end + def process_query(query) messages = [{ role: "user", content: query }] @@ -104,8 +135,21 @@ def process_query(query) # Execute every tool call in this response and collect the results. tool_results = tool_uses.map do |tool_use| - result = @mcp_client.call_tool(name: tool_use.name, arguments: tool_use.input) final_text << "[Calling tool #{tool_use.name} with args #{tool_use.input.to_json}]" + begin + result = @mcp_client.call_tool(name: tool_use.name, arguments: tool_use.input) + rescue MCP::Client::ServerError => e + # A rejected call is the model's mistake to correct, so it goes back as + # a failed tool_result rather than ending the query. Every tool_use + # still needs a matching result. + final_text << "[#{tool_use.name} failed: #{e.message}]" + next { + type: "tool_result", + tool_use_id: tool_use.id, + content: "Tool call #{tool_use.name} failed: #{e.message}", + is_error: true + } + end # structured_content is data the application can use directly; when a # tool returns an array, count its items rather than re-reading prose. @@ -116,15 +160,12 @@ def process_query(query) final_text << "[#{tool_use.name} returned #{structured.length} items]" if structured.is_a?(Array) end - # content is a list of block types; forward only the text ones. - tool_result_content = result.dig("result", "content") - result_text = if tool_result_content.is_a?(Array) - tool_result_content.filter_map { |content_item| content_item["text"] }.join("\n") - else - tool_result_content.to_s - end - - { type: "tool_result", tool_use_id: tool_use.id, content: result_text, is_error: is_error } + { + type: "tool_result", + tool_use_id: tool_use.id, + content: tool_result_content(result["result"]), + is_error: is_error + } end # Append the assistant's response and all tool results, in one user diff --git a/mcp-client-rust/README.md b/mcp-client-rust/README.md index d0430342..77a1bc28 100644 --- a/mcp-client-rust/README.md +++ b/mcp-client-rust/README.md @@ -46,3 +46,5 @@ rmcp's `serve` only performs the legacy `2025-11-25` `initialize` handshake, so rmcp does not validate tool output, so this client compiles each declared `outputSchema` at connect time and checks results against it — the spec's client-side SHOULD. It uses the [`jsonschema`](https://docs.rs/jsonschema) crate, which rmcp's own documentation recommends. The two channels go to different readers: `content` is forwarded to the model, while `structured_content` is used as data — the client counts the items it returns. See [Structured Content](https://modelcontextprotocol.io/specification/2026-07-28/server/tools#structured-content). + +Before a tool result goes to the model, the client converts it to the Messages API's block types. Text stays text, a PNG, JPEG, GIF or WebP image becomes an image block, and a text resource becomes its text. Anything else, such as audio, becomes a placeholder like `[unsupported result type: audio]`, so the model knows something was there. A result with structured content and no text block is sent as its JSON. A call the server rejects goes back to the model as a failed tool result, so it can correct itself, rather than ending the query. diff --git a/mcp-client-rust/src/main.rs b/mcp-client-rust/src/main.rs index b9599636..99cb78b2 100644 --- a/mcp-client-rust/src/main.rs +++ b/mcp-client-rust/src/main.rs @@ -1,8 +1,8 @@ use anyhow::{Context, Result, bail}; use jsonschema::Validator; use rmcp::model::{ - CallToolRequestParams, CallToolResult, ClientCapabilities, ClientConfig, Implementation, - ProtocolVersion, Tool as McpTool, + CallToolRequestParams, CallToolResult, ClientCapabilities, ClientConfig, ContentBlock, + Implementation, ProtocolVersion, ResourceContents, Tool as McpTool, }; use rmcp::service::{ClientLifecycleMode, ClientServiceExt, RoleClient, RunningService}; use rmcp::transport::TokioChildProcess; @@ -22,6 +22,60 @@ const MAX_TOOL_TURNS: usize = 10; const ANTHROPIC_API_BASE: &str = "https://api.anthropic.com"; const ANTHROPIC_VERSION: &str = "2023-06-01"; const REQUEST_TIMEOUT: Duration = Duration::from_secs(120); +/// Image types the Messages API accepts. +const SUPPORTED_IMAGE_TYPES: [&str; 4] = ["image/jpeg", "image/png", "image/gif", "image/webp"]; + +/// Translate an MCP tool result into content for an Anthropic tool_result. +/// +/// MCP and Anthropic content blocks differ, so each kind is converted. A kind +/// the model cannot take becomes a placeholder rather than vanishing, so the +/// model knows something was there. +fn tool_result_content(result: &CallToolResult) -> Vec { + let mut content: Vec = result + .content + .iter() + .map(|block| match block { + ContentBlock::Text(text) => json!({"type": "text", "text": text.text}), + ContentBlock::Image(image) + if SUPPORTED_IMAGE_TYPES.contains(&image.mime_type.as_str()) => + { + json!({ + "type": "image", + "source": {"type": "base64", "media_type": image.mime_type, "data": image.data}, + }) + } + ContentBlock::Resource(resource) => match &resource.resource { + ResourceContents::TextResourceContents { text, .. } => { + json!({"type": "text", "text": text}) + } + _ => unsupported("resource"), + }, + other => { + let kind = serde_json::to_value(other) + .ok() + .and_then(|value| value.get("type")?.as_str().map(str::to_string)) + .unwrap_or_else(|| "unknown".to_string()); + unsupported(&kind) + } + }) + .collect(); + + // A structured result SHOULD also carry serialized JSON in a text block, + // but only SHOULD, so serialize it when no text block came with it. + let has_text = result + .content + .iter() + .any(|block| matches!(block, ContentBlock::Text(_))); + if !has_text && let Some(structured) = &result.structured_content { + content.push(json!({"type": "text", "text": structured.to_string()})); + } + + content +} + +fn unsupported(kind: &str) -> Value { + json!({"type": "text", "text": format!("[unsupported result type: {kind}]")}) +} /// One turn of the conversation, as the Messages API takes it. /// @@ -261,27 +315,10 @@ impl MCPClient { final_text.push(format!("[{name} returned {} items]", items.len())); } - // content is a list of block types; forward only the text ones. - let mut payload = tool_result - .content - .iter() - .filter_map(|block| block.as_text().map(|text| text.text.as_str())) - .collect::>() - .join("\n"); - - // A structured result SHOULD also carry serialized JSON in a text - // block, but only SHOULD, so fall back to serializing it rather - // than sending the model an empty result. - if payload.is_empty() { - if let Some(structured) = &tool_result.structured_content { - payload = structured.to_string(); - } - } - tool_results.push(json!({ "type": "tool_result", "tool_use_id": id, - "content": payload, + "content": tool_result_content(&tool_result), "is_error": tool_result.is_error.unwrap_or(false), })); } diff --git a/mcp-client-typescript/README.md b/mcp-client-typescript/README.md index 6a3e6f96..4071097d 100644 --- a/mcp-client-typescript/README.md +++ b/mcp-client-typescript/README.md @@ -41,4 +41,6 @@ The SDK validates every result against the tool's declared `outputSchema`, so th The two channels go to different readers: `content` is forwarded to the model, while `structuredContent` is used as data — when a tool returns an array, the client counts its items rather than re-reading the prose. See [Structured Content](https://modelcontextprotocol.io/specification/draft/server/tools#structured-content). +Before a tool result goes to the model, the client converts it to the Messages API's block types. Text stays text, a PNG, JPEG, GIF or WebP image becomes an image block, and a text resource becomes its text. Anything else, such as audio, becomes a placeholder like `[unsupported result type: audio]`, so the model knows something was there. A result with structured content and no text block is sent as its JSON. A call the server rejects goes back to the model as a failed tool result, so it can correct itself, rather than ending the query. + `versionNegotiation: { mode: 'auto' }` probes `server/discover` and falls back to the `2025-11-25` handshake; the SDK's default is `'legacy'`. See the [SDK documentation](https://ts.sdk.modelcontextprotocol.io/v2/). diff --git a/mcp-client-typescript/index.ts b/mcp-client-typescript/index.ts index 935274df..9a04136a 100644 --- a/mcp-client-typescript/index.ts +++ b/mcp-client-typescript/index.ts @@ -1,6 +1,7 @@ import Anthropic from "@anthropic-ai/sdk"; -import { Client } from "@modelcontextprotocol/client"; +import { Client, ProtocolError } from "@modelcontextprotocol/client"; +import type { CallToolResult } from "@modelcontextprotocol/client"; import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; import readline from "readline/promises"; @@ -14,6 +15,57 @@ const ANTHROPIC_MODEL = "claude-sonnet-5"; const MAX_TOKENS = 10000; const MAX_TOOL_TURNS = 10; +type ImageMediaType = Anthropic.Base64ImageSource["media_type"]; +// Image types the Messages API accepts. +const SUPPORTED_IMAGE_TYPES = new Set([ + "image/jpeg", + "image/png", + "image/gif", + "image/webp", +] satisfies ImageMediaType[]); + +/** + * Translate an MCP tool result into content for an Anthropic tool_result. + * + * MCP and Anthropic content blocks differ, so each kind is converted. A kind + * the model cannot take becomes a placeholder rather than vanishing, so the + * model knows something was there. + */ +function toolResultContent( + result: CallToolResult, +): Array { + const content: Array = []; + for (const block of result.content) { + if (block.type === "text") { + content.push({ type: "text", text: block.text }); + } else if (block.type === "image" && SUPPORTED_IMAGE_TYPES.has(block.mimeType)) { + content.push({ + type: "image", + source: { + type: "base64", + media_type: block.mimeType as ImageMediaType, + data: block.data, + }, + }); + } else if (block.type === "resource" && "text" in block.resource) { + content.push({ type: "text", text: block.resource.text }); + } else { + content.push({ type: "text", text: `[unsupported result type: ${block.type}]` }); + } + } + + // A structured result SHOULD also carry serialized JSON in a text block, but + // only SHOULD, so serialize it when no text block came with it. + if ( + result.structuredContent !== undefined && + !result.content.some((block) => block.type === "text") + ) { + content.push({ type: "text", text: JSON.stringify(result.structuredContent) }); + } + + return content; +} + class MCPClient { private mcp: Client; private _anthropic: Anthropic | null = null; @@ -123,11 +175,27 @@ class MCPClient { finalText.push( `[Calling tool ${toolUse.name} with args ${JSON.stringify(toolArgs)}]`, ); - // callTool validates the result against the declared schema. - const result = await this.mcp.callTool({ - name: toolUse.name, - arguments: toolArgs, - }); + let result: CallToolResult; + try { + // callTool validates the result against the declared schema. + result = await this.mcp.callTool({ + name: toolUse.name, + arguments: toolArgs, + }); + } catch (error) { + if (!(error instanceof ProtocolError)) throw error; + // A rejected call is the model's mistake to correct, so it goes back + // as a failed tool_result rather than ending the query. Every + // tool_use still needs a matching result. + finalText.push(`[${toolUse.name} failed: ${error.message}]`); + toolResults.push({ + type: "tool_result", + tool_use_id: toolUse.id, + content: `Tool call ${toolUse.name} failed: ${error.message}`, + is_error: true, + }); + continue; + } // structuredContent is data the application can use directly. if (Array.isArray(result.structuredContent)) { @@ -136,15 +204,10 @@ class MCPClient { ); } - // content is a list of block types; forward only the text ones. MCP - // and Anthropic block shapes differ, so other kinds need converting. toolResults.push({ type: "tool_result", tool_use_id: toolUse.id, - content: result.content - .filter((block) => block.type === "text") - .map((block) => block.text) - .join("\n"), + content: toolResultContent(result), is_error: result.isError === true, }); } diff --git a/tests/README.md b/tests/README.md index 5e796308..6f151fb8 100644 --- a/tests/README.md +++ b/tests/README.md @@ -23,15 +23,16 @@ Tool calls reach the live NWS API. When it is unreachable the tools return an er ## Tool loop -Connecting and listing tools does not exercise the chat loop, so each client is also driven through three scripted queries with `tool-loop-test.ts`. No real API is involved: the helper starts a fake Anthropic Messages API on a loopback port and hands it to the client through `ANTHROPIC_BASE_URL`, which every quickstart SDK reads. The client talks to the mock MCP server as in the no-key test. The helper types each query only after the client shows its `Query:` prompt, as a person would. +Connecting and listing tools does not exercise the chat loop, so each client is also driven through four scripted queries with `tool-loop-test.ts`. No real API is involved: the helper starts a fake Anthropic Messages API on a loopback port and hands it to the client through `ANTHROPIC_BASE_URL`, which every quickstart SDK reads. The client talks to the mock MCP server as in the no-key test. The helper types each query only after the client shows its `Query:` prompt, as a person would. The fake API picks a script from the query text and checks every request the client sends: - **parallel tools** — a response with two `tool_use` blocks, then one for a tool the mock server lacks, then an answer. `tools` must be passed on every call, `max_tokens` must be 10000 (room for the model's adaptive thinking), every `tool_use` must get a `tool_result` in a single following user message with matching `tool_use_id`s, and the MCP `isError` result must be forwarded as `is_error`. - **ten tool turns** — exactly `MAX_TOOL_TURNS` tool calls, then an answer. The client must print the answer and no stop notice: nothing was cut short. - **endless tool turns** — a tool call on every response. The client must stop after `MAX_TOOL_TURNS` rounds, print `[Stopped after 10 tool-use turns]` once, and make no further call. +- **tool result contents** — one call to each of three tools whose results need translating before they reach the model. A call the server rejects with a JSON-RPC error must come back as a `tool_result` with `is_error` set, and the query must go on. A result with `structuredContent` and no text block must arrive as its JSON. A result with an image, a text resource and an audio block must arrive as an Anthropic base64 image block, the resource's text, and the placeholder `[unsupported result type: audio]`. -Finally the client must exit 0 on `quit`. A client that does one tool round and stops, drops `tools` on the follow-up call, sends one `tool_result` per message, or gets the turn cap wrong passes the no-key test and fails this one. +Finally the client must exit 0 on `quit`. A client that does one tool round and stops, drops `tools` on the follow-up call, sends one `tool_result` per message, gets the turn cap wrong, or drops what a tool returned passes the no-key test and fails this one. All five clients are covered, including Rust: it calls the Messages API directly, so it honours `ANTHROPIC_BASE_URL` like the official SDKs do. @@ -103,6 +104,8 @@ A minimal MCP server that verifies clients call the `tools/list` method. Used to It advertises two tools whose output schemas cover both shapes a structured result can take: an object root, and an array root. The array-rooted one is deliberate — a client that compiles every declared `outputSchema` up front, as the Go and Rust quickstart clients do, will fail here if it assumes an output schema is always `{"type": "object"}`. +Three more tools serve the tool-loop test's **tool result contents** query: `rejected_call` fails with a JSON-RPC error, `structured_only` returns `structuredContent` and no text block, and `non_text` returns an image, a text resource and an audio block. The values the test looks for are in `mock-values.ts`. + **Usage**: ```bash @@ -111,7 +114,7 @@ node tests/helpers/build/mock-mcp-server.js ### tool-loop-test.ts -Runs a client through the scripted tool loop described above. It starts the fake Anthropic API, spawns the client command with `ANTHROPIC_BASE_URL` and a dummy `ANTHROPIC_API_KEY` set, types the three queries and then `quit` at the client's prompts, and reports which checks failed along with the client's output. +Runs a client through the scripted tool loop described above. It starts the fake Anthropic API, spawns the client command with `ANTHROPIC_BASE_URL` and a dummy `ANTHROPIC_API_KEY` set, types the four queries and then `quit` at the client's prompts, and reports which checks failed along with the client's output. **Usage**: diff --git a/tests/helpers/mock-mcp-server.ts b/tests/helpers/mock-mcp-server.ts index 0ab6cd73..c241b575 100644 --- a/tests/helpers/mock-mcp-server.ts +++ b/tests/helpers/mock-mcp-server.ts @@ -6,6 +6,14 @@ * schemas cover both shapes a structured result can take: an object root, and * an array root (allowed as of protocol revision 2026-07-28). * + * Three more tools return results a client must translate with care before + * passing them to the model (see tool-loop-test.ts): + * + * - `rejected_call` fails at the protocol level (a JSON-RPC error, not an + * `isError` result); + * - `structured_only` returns `structuredContent` and no text block; + * - `non_text` returns an image, a text resource and an audio block. + * * The array-rooted tool is deliberate. Clients that compile every declared * `outputSchema` up front — as the Go and Rust quickstart clients do — will * fail here if they assume an output schema is always `{"type": "object"}`. @@ -18,6 +26,7 @@ import type { Tool } from "@modelcontextprotocol/server"; import { Server } from "@modelcontextprotocol/server"; import { serveStdio } from "@modelcontextprotocol/server/stdio"; +import { IMAGE_DATA, RESOURCE_TEXT, STRUCTURED_ONLY_VALUE } from "./mock-values.js"; // Track whether tools/list was called let toolsListCalled = false; @@ -62,6 +71,26 @@ const TOOLS: Tool[] = [ required: ["latitude", "longitude", "summary"], }, }, + { + name: "rejected_call", + description: "Mock tool the server always rejects with a JSON-RPC error", + inputSchema: { type: "object", properties: {} }, + }, + { + name: "structured_only", + description: "Mock tool returning structuredContent and no text block", + inputSchema: { type: "object", properties: {} }, + outputSchema: { + type: "object", + properties: { value: { type: "string" } }, + required: ["value"], + }, + }, + { + name: "non_text", + description: "Mock tool returning image, resource and audio blocks", + inputSchema: { type: "object", properties: {} }, + }, ]; function buildServer(): Server { @@ -99,6 +128,29 @@ function buildServer(): Server { }; } + if (name === "rejected_call") { + // Thrown from a low-level handler, this reaches the client as a JSON-RPC + // error response rather than a result. + throw new Error("rejected_call is always rejected"); + } + + if (name === "structured_only") { + return { content: [], structuredContent: { value: STRUCTURED_ONLY_VALUE } }; + } + + if (name === "non_text") { + return { + content: [ + { type: "image", data: IMAGE_DATA, mimeType: "image/png" }, + { + type: "resource", + resource: { uri: "mock://notes", mimeType: "text/plain", text: RESOURCE_TEXT }, + }, + { type: "audio", data: "AAAA", mimeType: "audio/wav" }, + ], + }; + } + return { content: [{ type: "text", text: `Unknown tool: ${name}` }], isError: true, diff --git a/tests/helpers/mock-values.ts b/tests/helpers/mock-values.ts new file mode 100644 index 00000000..e3092ae1 --- /dev/null +++ b/tests/helpers/mock-values.ts @@ -0,0 +1,11 @@ +/** + * Values the mock MCP server returns and the tool-loop test looks for in what + * the client forwards to the model. They live here rather than in + * mock-mcp-server.ts, because importing that file would start the server. + */ + +export const STRUCTURED_ONLY_VALUE = "structured-only-value"; +export const RESOURCE_TEXT = "resource-text-value"; +/** A 1x1 transparent PNG. */ +export const IMAGE_DATA = + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII="; diff --git a/tests/helpers/tool-loop-test.ts b/tests/helpers/tool-loop-test.ts index 6cdd761a..96601fa0 100644 --- a/tests/helpers/tool-loop-test.ts +++ b/tests/helpers/tool-loop-test.ts @@ -10,7 +10,7 @@ * - a fake Anthropic Messages API, started here on a loopback port and handed * to the client through ANTHROPIC_BASE_URL, which every quickstart SDK reads. * - * Three queries are piped into one client process, and the fake API picks the + * Four queries are piped into one client process, and the fake API picks the * script from the query text: * * - "parallel tools": text plus two tool_use blocks in one response, then one @@ -22,6 +22,12 @@ * client must print the answer without a premature stop notice. * - "endless tool turns": a tool call on every response. The client must stop * after MAX_TOOL_TURNS rounds, say so, and make no further call. + * - "tool result contents": three tool calls whose results need translating + * before they reach the model. One the server rejects with a JSON-RPC error + * must come back as an is_error tool_result, not end the query. One with + * only structuredContent must arrive as JSON. One with image, resource and + * audio blocks must arrive as an image block, the resource's text, and a + * placeholder for the audio. * * A client that does one tool round and stops, drops `tools` on the second * call, sends one tool_result per message, or gets the turn cap wrong fails @@ -32,9 +38,12 @@ import { spawn } from "node:child_process"; import http from "node:http"; +import { IMAGE_DATA, RESOURCE_TEXT, STRUCTURED_ONLY_VALUE } from "./mock-values.js"; /** Sonnet 5 thinks adaptively; the clients reserve this much for it. */ const EXPECTED_MAX_TOKENS = 10000; +/** How many tools the mock server lists. */ +const MOCK_TOOL_COUNT = 5; /** MAX_TOOL_TURNS in every client. */ const MAX_TOOL_TURNS = 10; const STOP_NOTICE = `[Stopped after ${MAX_TOOL_TURNS} tool-use turns]`; @@ -46,6 +55,7 @@ const QUERIES = { parallel: "parallel tools", atLimit: "ten tool turns", overrun: "endless tool turns", + contents: "tool result contents", } as const; type Scenario = keyof typeof QUERIES; @@ -53,6 +63,7 @@ const FINAL_ANSWER: Record = { parallel: "All done.", atLimit: "Answered at the limit.", overrun: "never sent", + contents: "Contents handled.", }; type Block = Record & { type: string }; @@ -66,7 +77,7 @@ type Request = { const failures: string[] = []; let passed = 0; -const calls: Record = { parallel: 0, atLimit: 0, overrun: 0 }; +const calls: Record = { parallel: 0, atLimit: 0, overrun: 0, contents: 0 }; function check(condition: boolean, what: string) { if (condition) { @@ -185,6 +196,69 @@ function respondParallel(n: number, request: Request): Block[] { return [text("extra")]; } +/** + * The scripted turn for the tool-result-contents query: one call to each of the + * mock server's three awkward tools, then an answer. + */ +function respondContents(n: number, request: Request): Block[] { + const label = `contents call ${n}`; + const last = request.messages[request.messages.length - 1]; + + if (n === 1) { + return [ + toolUse("tc_1", "rejected_call", {}), + toolUse("tc_2", "structured_only", {}), + toolUse("tc_3", "non_text", {}), + ]; + } + + if (n === 2) { + const results = toolResultsOf(last); + const byId = (id: string) => results.find((block) => block.tool_use_id === id); + check( + results.length === 3, + `${label}: all three tool_results arrive, the rejected call's too (got ${results.length})`, + ); + + const rejected = byId("tc_1"); + check(rejected?.is_error === true, `${label}: rejected call is forwarded as is_error`); + check( + textOf(rejected?.content).length > 0, + `${label}: rejected call's tool_result says what went wrong`, + ); + + const structured = byId("tc_2"); + check( + textOf(structured?.content).includes(STRUCTURED_ONLY_VALUE), + `${label}: structuredContent without a text block is forwarded as JSON`, + ); + + const nonText = byId("tc_3"); + const blocks = Array.isArray(nonText?.content) ? (nonText.content as Block[]) : []; + const image = blocks.find((block) => block.type === "image") as + | { source?: { type?: string; media_type?: string; data?: string } } + | undefined; + check( + image?.source?.type === "base64" && + image.source.media_type === "image/png" && + image.source.data === IMAGE_DATA, + `${label}: image block is converted to an Anthropic base64 image`, + ); + check( + textOf(nonText?.content).includes(RESOURCE_TEXT), + `${label}: text resource is forwarded as its text`, + ); + check( + textOf(nonText?.content).includes("[unsupported result type: audio]"), + `${label}: audio block is replaced by a placeholder`, + ); + return [text(FINAL_ANSWER.contents)]; + } + + check(false, `${label}: unexpected extra call`); + return [text("extra")]; +} + /** * The scripted turn for the two turn-limit queries. Each response asks for one * more tool call; the previous call's result must be present. At the limit the @@ -227,7 +301,7 @@ function respond(request: Request): Block[] { const n = ++calls[scenario]; check( - Array.isArray(request.tools) && request.tools.length === 2, + Array.isArray(request.tools) && request.tools.length === MOCK_TOOL_COUNT, `${scenario} call ${n}: tools are passed (got ${request.tools?.length ?? "none"})`, ); check( @@ -235,7 +309,9 @@ function respond(request: Request): Block[] { `${scenario} call ${n}: max_tokens is ${EXPECTED_MAX_TOKENS} (got ${request.max_tokens})`, ); - return scenario === "parallel" ? respondParallel(n, request) : respondTurnLimit(scenario, n, request); + if (scenario === "parallel") return respondParallel(n, request); + if (scenario === "contents") return respondContents(n, request); + return respondTurnLimit(scenario, n, request); } function startFakeApi(): Promise { @@ -340,6 +416,9 @@ function checkOutput(code: number | null, stdout: string) { const notices = stdout.slice(overrunAt).split(STOP_NOTICE).length - 1; check(notices === 1, `overrun: stop notice printed once (got ${notices})`); } + + check(calls.contents === 2, `contents: API was called 2 times (got ${calls.contents})`); + check(stdout.includes(FINAL_ANSWER.contents), "contents: client printed the final answer"); } async function main() { diff --git a/tests/smoke-test.sh b/tests/smoke-test.sh index b4bacaf3..cfa890c7 100755 --- a/tests/smoke-test.sh +++ b/tests/smoke-test.sh @@ -156,10 +156,12 @@ test_mcp_client_rust() { # The tool-loop tests drive the chat loop itself, which the no-key tests never # reach. The helper starts a fake Anthropic API on a loopback port, points the -# client at it through ANTHROPIC_BASE_URL, and types three queries at the +# client at it through ANTHROPIC_BASE_URL, and types four queries at the # client's prompt: two parallel tool calls then a failing one, exactly ten -# tool turns then an answer, and tool calls past the turn limit. It checks -# every request the client sends; see tests/README.md for what is checked. +# tool turns then an answer, tool calls past the turn limit, and three tools +# whose results need translating (a rejected call, structured content with no +# text, and non-text blocks). It checks every request the client sends; see +# tests/README.md for what is checked. # Test: Python MCP client tool loop test_tool_loop_python() { From 2cf62b377258e6fd648d8455b56e2f7d9d1cef4c Mon Sep 17 00:00:00 2001 From: a-akimov Date: Mon, 2 Nov 2026 22:16:07 +0100 Subject: [PATCH 2/2] Address comments --- mcp-client-go/README.md | 2 +- mcp-client-go/main.go | 7 +++++-- mcp-client-python/README.md | 2 +- mcp-client-python/client.py | 6 +++--- mcp-client-ruby/README.md | 2 +- mcp-client-ruby/client.rb | 6 +++--- mcp-client-rust/README.md | 2 +- mcp-client-rust/src/main.rs | 5 ++--- mcp-client-typescript/README.md | 2 +- mcp-client-typescript/index.ts | 7 +++---- tests/README.md | 4 ++-- tests/helpers/mock-mcp-server.ts | 16 +++++++++++----- tests/helpers/tool-loop-test.ts | 22 ++++++++++++++++++---- 13 files changed, 52 insertions(+), 31 deletions(-) diff --git a/mcp-client-go/README.md b/mcp-client-go/README.md index 81b7bf4c..8c9f2360 100644 --- a/mcp-client-go/README.md +++ b/mcp-client-go/README.md @@ -42,4 +42,4 @@ The SDK does not validate tool output, so this client compiles each declared `ou The two channels go to different readers: `Content` is forwarded to the model, while `StructuredContent` is used as data — the client counts the items it returns. See [Structured Content](https://modelcontextprotocol.io/specification/2026-07-28/server/tools#structured-content). -Before a tool result goes to the model, the client converts it to the Messages API's block types. Text stays text, a PNG, JPEG, GIF or WebP image becomes an image block, and a text resource becomes its text. Anything else, such as audio, becomes a placeholder like `[unsupported result type: audio]`, so the model knows something was there. A result with structured content and no text block is sent as its JSON. A call the server rejects goes back to the model as a failed tool result, so it can correct itself, rather than ending the query. +Before a tool result goes to the model, the client converts it to the Messages API's block types. Text stays text, a PNG, JPEG, GIF or WebP image becomes an image block, and a text resource becomes its text. Anything else, such as audio, becomes a placeholder like `[unsupported result type: audio]`, so the model knows something was there. Empty text is dropped, because the Messages API rejects empty text blocks. A result with structured content and no text is sent as its JSON. A call the server rejects goes back to the model as a failed tool result, so it can correct itself, rather than ending the query. diff --git a/mcp-client-go/main.go b/mcp-client-go/main.go index 50b8f75a..6482c3fe 100644 --- a/mcp-client-go/main.go +++ b/mcp-client-go/main.go @@ -42,6 +42,9 @@ var supportedImageTypes = map[string]bool{ func toolResultContent(result *mcp.CallToolResult) []anthropic.ToolResultBlockParamContentUnion { var content []anthropic.ToolResultBlockParamContentUnion text := func(s string) { + if s == "" { + return + } content = append(content, anthropic.ToolResultBlockParamContentUnion{OfText: &anthropic.TextBlockParam{Text: s}}) } @@ -49,7 +52,7 @@ func toolResultContent(result *mcp.CallToolResult) []anthropic.ToolResultBlockPa for _, block := range result.Content { switch b := block.(type) { case *mcp.TextContent: - hasText = true + hasText = hasText || b.Text != "" text(b.Text) case *mcp.ImageContent: if !supportedImageTypes[b.MIMEType] { @@ -75,7 +78,7 @@ func toolResultContent(result *mcp.CallToolResult) []anthropic.ToolResultBlockPa } // A structured result SHOULD also carry serialized JSON in a text block, but - // only SHOULD, so serialize it when no text block came with it. + // only SHOULD, so serialize it when no text came with it. if !hasText && result.StructuredContent != nil { if data, err := json.Marshal(result.StructuredContent); err == nil { text(string(data)) diff --git a/mcp-client-python/README.md b/mcp-client-python/README.md index 6e970afb..dd5a0719 100644 --- a/mcp-client-python/README.md +++ b/mcp-client-python/README.md @@ -43,6 +43,6 @@ Without an `ANTHROPIC_API_KEY`, the client still connects, prints the server's t The two channels go to different readers: `content` is forwarded to the model, while `structured_content` is used as data — when a tool returns an array, the client counts its items rather than re-reading the prose. See [Structured Content](https://modelcontextprotocol.io/specification/2026-07-28/server/tools#structured-content). -Before a tool result goes to the model, the client converts it to the Messages API's block types. Text stays text, a PNG, JPEG, GIF or WebP image becomes an image block, and a text resource becomes its text. Anything else, such as audio, becomes a placeholder like `[unsupported result type: audio]`, so the model knows something was there. A result with structured content and no text block is sent as its JSON. A call the server rejects goes back to the model as a failed tool result, so it can correct itself, rather than ending the query. +Before a tool result goes to the model, the client converts it to the Messages API's block types. Text stays text, a PNG, JPEG, GIF or WebP image becomes an image block, and a text resource becomes its text. Anything else, such as audio, becomes a placeholder like `[unsupported result type: audio]`, so the model knows something was there. Empty text is dropped, because the Messages API rejects empty text blocks. A result with structured content and no text is sent as its JSON. A call the server rejects goes back to the model as a failed tool result, so it can correct itself, rather than ending the query. `Client(server_params, mode="auto")` launches the server from its `StdioServerParameters`, then probes `server/discover` and falls back to the `2025-11-25` handshake; `client.protocol_version` reports which era you got. See [Protocol versions](https://py.sdk.modelcontextprotocol.io/v2/protocol-versions/). diff --git a/mcp-client-python/client.py b/mcp-client-python/client.py index 4816f4b6..f4fa476d 100644 --- a/mcp-client-python/client.py +++ b/mcp-client-python/client.py @@ -43,9 +43,9 @@ def tool_result_content(result: CallToolResult) -> list[dict]: else: content.append({"type": "text", "text": f"[unsupported result type: {block.type}]"}) - # A structured result SHOULD also carry serialized JSON in a text block, but - # only SHOULD, so serialize it when no text block came with it. - if result.structured_content is not None and not any(isinstance(block, TextContent) for block in result.content): + content = [block for block in content if block.get("text") != ""] + has_text = any(isinstance(block, TextContent) and block.text for block in result.content) + if result.structured_content is not None and not has_text: content.append({"type": "text", "text": json.dumps(result.structured_content)}) return content diff --git a/mcp-client-ruby/README.md b/mcp-client-ruby/README.md index ac05606b..cb74633f 100644 --- a/mcp-client-ruby/README.md +++ b/mcp-client-ruby/README.md @@ -38,6 +38,6 @@ The spec says clients SHOULD validate structured results against the schema the The two channels go to different readers: `content` is forwarded to the model, while `structured_content` is used as data — when a tool returns an array, the client counts its items rather than re-reading the prose. See [Structured Content](https://modelcontextprotocol.io/specification/2026-07-28/server/tools#structured-content). -Before a tool result goes to the model, the client converts it to the Messages API's block types. Text stays text, a PNG, JPEG, GIF or WebP image becomes an image block, and a text resource becomes its text. Anything else, such as audio, becomes a placeholder like `[unsupported result type: audio]`, so the model knows something was there. A result with structured content and no text block is sent as its JSON. A call the server rejects goes back to the model as a failed tool result, so it can correct itself, rather than ending the query. +Before a tool result goes to the model, the client converts it to the Messages API's block types. Text stays text, a PNG, JPEG, GIF or WebP image becomes an image block, and a text resource becomes its text. Anything else, such as audio, becomes a placeholder like `[unsupported result type: audio]`, so the model knows something was there. Empty text is dropped, because the Messages API rejects empty text blocks. A result with structured content and no text is sent as its JSON. A call the server rejects goes back to the model as a failed tool result, so it can correct itself, rather than ending the query. `MCP::Client::Stdio` offers `2026-07-28` by default, so no negotiation setup is needed here. diff --git a/mcp-client-ruby/client.rb b/mcp-client-ruby/client.rb index 4390cf72..32696c63 100644 --- a/mcp-client-ruby/client.rb +++ b/mcp-client-ruby/client.rb @@ -97,10 +97,10 @@ def tool_result_content(result) end end - # A structured result SHOULD also carry serialized JSON in a text block, but - # only SHOULD, so serialize it when no text block came with it. + content.reject! { |block| block[:text] == "" } structured = result["structuredContent"] - if !structured.nil? && blocks.none? { |block| block["type"] == "text" } + has_text = blocks.any? { |block| block["type"] == "text" && !block["text"].to_s.empty? } + if !structured.nil? && !has_text content << { type: "text", text: structured.to_json } end diff --git a/mcp-client-rust/README.md b/mcp-client-rust/README.md index 77a1bc28..a835f416 100644 --- a/mcp-client-rust/README.md +++ b/mcp-client-rust/README.md @@ -47,4 +47,4 @@ rmcp does not validate tool output, so this client compiles each declared `outpu The two channels go to different readers: `content` is forwarded to the model, while `structured_content` is used as data — the client counts the items it returns. See [Structured Content](https://modelcontextprotocol.io/specification/2026-07-28/server/tools#structured-content). -Before a tool result goes to the model, the client converts it to the Messages API's block types. Text stays text, a PNG, JPEG, GIF or WebP image becomes an image block, and a text resource becomes its text. Anything else, such as audio, becomes a placeholder like `[unsupported result type: audio]`, so the model knows something was there. A result with structured content and no text block is sent as its JSON. A call the server rejects goes back to the model as a failed tool result, so it can correct itself, rather than ending the query. +Before a tool result goes to the model, the client converts it to the Messages API's block types. Text stays text, a PNG, JPEG, GIF or WebP image becomes an image block, and a text resource becomes its text. Anything else, such as audio, becomes a placeholder like `[unsupported result type: audio]`, so the model knows something was there. Empty text is dropped, because the Messages API rejects empty text blocks. A result with structured content and no text is sent as its JSON. A call the server rejects goes back to the model as a failed tool result, so it can correct itself, rather than ending the query. diff --git a/mcp-client-rust/src/main.rs b/mcp-client-rust/src/main.rs index 99cb78b2..661919c2 100644 --- a/mcp-client-rust/src/main.rs +++ b/mcp-client-rust/src/main.rs @@ -60,12 +60,11 @@ fn tool_result_content(result: &CallToolResult) -> Vec { }) .collect(); - // A structured result SHOULD also carry serialized JSON in a text block, - // but only SHOULD, so serialize it when no text block came with it. + content.retain(|block| block["text"] != ""); let has_text = result .content .iter() - .any(|block| matches!(block, ContentBlock::Text(_))); + .any(|block| matches!(block, ContentBlock::Text(text) if !text.text.is_empty())); if !has_text && let Some(structured) = &result.structured_content { content.push(json!({"type": "text", "text": structured.to_string()})); } diff --git a/mcp-client-typescript/README.md b/mcp-client-typescript/README.md index 4071097d..ea0f0435 100644 --- a/mcp-client-typescript/README.md +++ b/mcp-client-typescript/README.md @@ -41,6 +41,6 @@ The SDK validates every result against the tool's declared `outputSchema`, so th The two channels go to different readers: `content` is forwarded to the model, while `structuredContent` is used as data — when a tool returns an array, the client counts its items rather than re-reading the prose. See [Structured Content](https://modelcontextprotocol.io/specification/draft/server/tools#structured-content). -Before a tool result goes to the model, the client converts it to the Messages API's block types. Text stays text, a PNG, JPEG, GIF or WebP image becomes an image block, and a text resource becomes its text. Anything else, such as audio, becomes a placeholder like `[unsupported result type: audio]`, so the model knows something was there. A result with structured content and no text block is sent as its JSON. A call the server rejects goes back to the model as a failed tool result, so it can correct itself, rather than ending the query. +Before a tool result goes to the model, the client converts it to the Messages API's block types. Text stays text, a PNG, JPEG, GIF or WebP image becomes an image block, and a text resource becomes its text. Anything else, such as audio, becomes a placeholder like `[unsupported result type: audio]`, so the model knows something was there. Empty text is dropped, because the Messages API rejects empty text blocks. A result with structured content and no text is sent as its JSON. A call the server rejects goes back to the model as a failed tool result, so it can correct itself, rather than ending the query. `versionNegotiation: { mode: 'auto' }` probes `server/discover` and falls back to the `2025-11-25` handshake; the SDK's default is `'legacy'`. See the [SDK documentation](https://ts.sdk.modelcontextprotocol.io/v2/). diff --git a/mcp-client-typescript/index.ts b/mcp-client-typescript/index.ts index 9a04136a..0292e182 100644 --- a/mcp-client-typescript/index.ts +++ b/mcp-client-typescript/index.ts @@ -34,7 +34,7 @@ const SUPPORTED_IMAGE_TYPES = new Set([ function toolResultContent( result: CallToolResult, ): Array { - const content: Array = []; + let content: Array = []; for (const block of result.content) { if (block.type === "text") { content.push({ type: "text", text: block.text }); @@ -54,11 +54,10 @@ function toolResultContent( } } - // A structured result SHOULD also carry serialized JSON in a text block, but - // only SHOULD, so serialize it when no text block came with it. + content = content.filter((block) => block.type !== "text" || block.text !== ""); if ( result.structuredContent !== undefined && - !result.content.some((block) => block.type === "text") + !result.content.some((block) => block.type === "text" && block.text !== "") ) { content.push({ type: "text", text: JSON.stringify(result.structuredContent) }); } diff --git a/tests/README.md b/tests/README.md index 6f151fb8..55f7e4a0 100644 --- a/tests/README.md +++ b/tests/README.md @@ -30,7 +30,7 @@ The fake API picks a script from the query text and checks every request the cli - **parallel tools** — a response with two `tool_use` blocks, then one for a tool the mock server lacks, then an answer. `tools` must be passed on every call, `max_tokens` must be 10000 (room for the model's adaptive thinking), every `tool_use` must get a `tool_result` in a single following user message with matching `tool_use_id`s, and the MCP `isError` result must be forwarded as `is_error`. - **ten tool turns** — exactly `MAX_TOOL_TURNS` tool calls, then an answer. The client must print the answer and no stop notice: nothing was cut short. - **endless tool turns** — a tool call on every response. The client must stop after `MAX_TOOL_TURNS` rounds, print `[Stopped after 10 tool-use turns]` once, and make no further call. -- **tool result contents** — one call to each of three tools whose results need translating before they reach the model. A call the server rejects with a JSON-RPC error must come back as a `tool_result` with `is_error` set, and the query must go on. A result with `structuredContent` and no text block must arrive as its JSON. A result with an image, a text resource and an audio block must arrive as an Anthropic base64 image block, the resource's text, and the placeholder `[unsupported result type: audio]`. +- **tool result contents** — one call to each of three tools whose results need translating before they reach the model. A call the server rejects with a JSON-RPC error must come back as a `tool_result` with `is_error` set, and the query must go on. A result with `structuredContent` and only an empty text block must arrive as its JSON. A result with an image, a text resource and an audio block must arrive as an Anthropic base64 image block, the resource's text, and the placeholder `[unsupported result type: audio]`. Empty text from a text block or a resource must be dropped: like the real API, the fake one fails any request that carries an empty text block. Finally the client must exit 0 on `quit`. A client that does one tool round and stops, drops `tools` on the follow-up call, sends one `tool_result` per message, gets the turn cap wrong, or drops what a tool returned passes the no-key test and fails this one. @@ -104,7 +104,7 @@ A minimal MCP server that verifies clients call the `tools/list` method. Used to It advertises two tools whose output schemas cover both shapes a structured result can take: an object root, and an array root. The array-rooted one is deliberate — a client that compiles every declared `outputSchema` up front, as the Go and Rust quickstart clients do, will fail here if it assumes an output schema is always `{"type": "object"}`. -Three more tools serve the tool-loop test's **tool result contents** query: `rejected_call` fails with a JSON-RPC error, `structured_only` returns `structuredContent` and no text block, and `non_text` returns an image, a text resource and an audio block. The values the test looks for are in `mock-values.ts`. +Three more tools serve the tool-loop test's **tool result contents** query: `rejected_call` fails with a JSON-RPC error, `structured_only` returns `structuredContent` and an empty text block, and `non_text` returns an image, a text resource, an audio block, and an empty text block and resource. The values the test looks for are in `mock-values.ts`. **Usage**: diff --git a/tests/helpers/mock-mcp-server.ts b/tests/helpers/mock-mcp-server.ts index c241b575..1ffb57a5 100644 --- a/tests/helpers/mock-mcp-server.ts +++ b/tests/helpers/mock-mcp-server.ts @@ -11,8 +11,9 @@ * * - `rejected_call` fails at the protocol level (a JSON-RPC error, not an * `isError` result); - * - `structured_only` returns `structuredContent` and no text block; - * - `non_text` returns an image, a text resource and an audio block. + * - `structured_only` returns `structuredContent` and an empty text block; + * - `non_text` returns an image, a text resource, an audio block, and an + * empty text block and resource (the Messages API rejects empty text). * * The array-rooted tool is deliberate. Clients that compile every declared * `outputSchema` up front — as the Go and Rust quickstart clients do — will @@ -78,7 +79,7 @@ const TOOLS: Tool[] = [ }, { name: "structured_only", - description: "Mock tool returning structuredContent and no text block", + description: "Mock tool returning structuredContent and only empty text", inputSchema: { type: "object", properties: {} }, outputSchema: { type: "object", @@ -88,7 +89,7 @@ const TOOLS: Tool[] = [ }, { name: "non_text", - description: "Mock tool returning image, resource and audio blocks", + description: "Mock tool returning image, resource, audio and empty text blocks", inputSchema: { type: "object", properties: {} }, }, ]; @@ -135,7 +136,10 @@ function buildServer(): Server { } if (name === "structured_only") { - return { content: [], structuredContent: { value: STRUCTURED_ONLY_VALUE } }; + return { + content: [{ type: "text", text: "" }], + structuredContent: { value: STRUCTURED_ONLY_VALUE }, + }; } if (name === "non_text") { @@ -147,6 +151,8 @@ function buildServer(): Server { resource: { uri: "mock://notes", mimeType: "text/plain", text: RESOURCE_TEXT }, }, { type: "audio", data: "AAAA", mimeType: "audio/wav" }, + { type: "text", text: "" }, + { type: "resource", resource: { uri: "mock://empty", mimeType: "text/plain", text: "" } }, ], }; } diff --git a/tests/helpers/tool-loop-test.ts b/tests/helpers/tool-loop-test.ts index 96601fa0..68dfe121 100644 --- a/tests/helpers/tool-loop-test.ts +++ b/tests/helpers/tool-loop-test.ts @@ -25,9 +25,10 @@ * - "tool result contents": three tool calls whose results need translating * before they reach the model. One the server rejects with a JSON-RPC error * must come back as an is_error tool_result, not end the query. One with - * only structuredContent must arrive as JSON. One with image, resource and - * audio blocks must arrive as an image block, the resource's text, and a - * placeholder for the audio. + * structuredContent and only empty text must arrive as JSON. One with image, + * resource and audio blocks must arrive as an image block, the resource's + * text, and a placeholder for the audio. Empty text must be dropped: like + * the real API, this one rejects an empty text block in any request. * * A client that does one tool round and stops, drops `tools` on the second * call, sends one tool_result per message, or gets the turn cap wrong fails @@ -104,6 +105,15 @@ function textOf(content: unknown): string { return ""; } +/** Whether any text block in the request, at any depth, is empty. */ +function hasEmptyText(value: unknown): boolean { + if (Array.isArray(value)) return value.some(hasEmptyText); + if (value === null || typeof value !== "object") return false; + const block = value as Record; + if (block.type === "text" && block.text === "") return true; + return Object.values(block).some(hasEmptyText); +} + /** The scenario a request belongs to: named by its most recent user query. */ function scenarioOf(messages: Message[]): Scenario | undefined { for (let i = messages.length - 1; i >= 0; i--) { @@ -230,7 +240,7 @@ function respondContents(n: number, request: Request): Block[] { const structured = byId("tc_2"); check( textOf(structured?.content).includes(STRUCTURED_ONLY_VALUE), - `${label}: structuredContent without a text block is forwarded as JSON`, + `${label}: structuredContent with only empty text is forwarded as JSON`, ); const nonText = byId("tc_3"); @@ -304,6 +314,10 @@ function respond(request: Request): Block[] { Array.isArray(request.tools) && request.tools.length === MOCK_TOOL_COUNT, `${scenario} call ${n}: tools are passed (got ${request.tools?.length ?? "none"})`, ); + check( + !hasEmptyText(request.messages), + `${scenario} call ${n}: no text block is empty (the API rejects them)`, + ); check( request.max_tokens === EXPECTED_MAX_TOKENS, `${scenario} call ${n}: max_tokens is ${EXPECTED_MAX_TOKENS} (got ${request.max_tokens})`,