From 06c63d39ac00148d1833ad702f44cccd79fad59d Mon Sep 17 00:00:00 2001 From: a-akimov Date: Wed, 16 Sep 2026 17:37:04 +0200 Subject: [PATCH 1/3] Fix the tool-use loop in the Go, Rust, and Ruby clients and add a tool-loop smoke test ## Motivation and Context Only the Python and TypeScript clients implemented a correct tool-use loop. The other three had bugs that break ordinary multi-step requests: - **Go, Rust, Ruby** did a single tool round and then called Claude once more *without* `tools`, so any request that needs a second tool call stalled. Go silently dropped `tool_use` blocks from that final response. - **Ruby** called Claude once per `tool_use` block, each time with a single `tool_result`. Parallel tool calls produced a malformed message sequence and an API 400. - **max_tokens** was 1000 in TypeScript and Ruby, 1024 in Go, and unset in Rust. Sonnet 5 thinks adaptively, so 1000 tokens is regularly exhausted mid-thought. Python already used 10000 with a comment explaining why. - **Ruby** never forwarded MCP `isError` as `is_error`, so the model could not tell a failed tool call from a successful one. Rust cannot set the flag either (see below). - **TypeScript** forwarded raw MCP content blocks to Anthropic. The two block shapes differ, so a non-text result would have failed. The existing smoke test never noticed because it only checks that each client calls `tools/list`. None of these bugs was reachable without an API key. ### What changed All five clients now run the same loop: call tools until Claude answers without one, send every `tool_result` of a turn in one user message, pass `tools` on every call, and stop after `MAX_TOOL_TURNS = 10`. `max_tokens` is 10000 everywhere, with the same comment. Python and TypeScript also keep the last response's text when the turn cap is hit; it used to be dropped. Rust keeps the `genai` crate. Its tool-response type has no `is_error` field, so an error result is marked with an `Error:` prefix in the text and the limitation is commented. ### New smoke test `tests/helpers/tool-loop-test.ts` starts a fake Anthropic Messages API on a loopback port, points a client at it through `ANTHROPIC_BASE_URL`, pipes in one query, and checks every request the client sends. It scripts two parallel tool calls, then a call to a tool the mock server lacks (an `isError` result), then a final answer, and asserts that tools are passed each time, `max_tokens` is 10000, all `tool_result`s arrive in one message with matching ids, and the error is forwarded. `smoke-test.sh` runs it for Python, TypeScript, Go, and Ruby. The Rust client is excluded for now: `genai` reads no endpoint variable, and the client negotiates protocol `2025-11-25`, under which the mock's array-rooted tool is refused at call time. Both will be addressed in a follow-up that brings all examples to current SDK versions. ## How Has This Been Tested? - `./tests/smoke-test.sh` passes for every example. - The new tool-loop test passes all 19 checks for Python, TypeScript, Go, and Ruby. - The Rust client was driven through the same scripted conversation with a temporary endpoint resolver (not committed) and passes. - As a negative check, the Ruby client from `main` was run through the new test. It fails 10 of 19 checks, on exactly the bugs listed above. - Go and Ruby were built and run in `golang:1.25` and `ruby:3.4` containers. ## Breaking Changes None. The clients' command lines, output format, and no-key behavior are unchanged. ## Types of changes - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [x] Documentation update ## Checklist - [x] I have read the [MCP Documentation](https://modelcontextprotocol.io) - [x] My code follows the repository's style guidelines - [x] New and existing tests pass locally - [x] I have added appropriate error handling - [x] I have added or updated documentation as needed ## Additional context Two pre-existing issues surfaced while testing and are left for a follow-up: - The Rust client (rmcp 3.1.2) negotiates `2025-11-25`. The TypeScript mock server and the Python weather server both reject its calls to array-rooted tools under that revision. Even current rmcp defaults to `2025-11-25`, so the client must request `2026-07-28` explicitly. - The Go SDK's tool-result helper sends content as a list of text blocks rather than a string. The API accepts both, so it is unchanged. AI assistance: the analysis, code changes, test helper, and this description were prepared partially manually, partially with Claude Code and reviewed by me. --- mcp-client-go/main.go | 149 ++++++++++--------- mcp-client-python/client.py | 3 +- mcp-client-ruby/client.rb | 101 +++++++------ mcp-client-rust/src/main.rs | 63 +++++--- mcp-client-typescript/index.ts | 21 ++- tests/README.md | 33 +++++ tests/helpers/tool-loop-test.ts | 251 ++++++++++++++++++++++++++++++++ tests/smoke-test.sh | 56 +++++++ tests/utils.sh | 3 +- 9 files changed, 541 insertions(+), 139 deletions(-) create mode 100644 tests/helpers/tool-loop-test.ts diff --git a/mcp-client-go/main.go b/mcp-client-go/main.go index 88dd5c86..d9dd0b85 100644 --- a/mcp-client-go/main.go +++ b/mcp-client-go/main.go @@ -17,10 +17,13 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" ) -// A string literal rather than an anthropic.Model constant: the pinned SDK -// version predates this model and has no constant for it. var model anthropic.Model = "claude-sonnet-5" +// Sonnet 5 thinks adaptively unless told otherwise, and max_tokens caps thinking +// plus the reply, so leave room for both. +const maxTokens = 10000 +const maxToolTurns = 10 + type MCPClient struct { anthropic *anthropic.Client session *mcp.ClientSession @@ -166,96 +169,104 @@ func (c *MCPClient) ProcessQuery(ctx context.Context, query string) (string, err anthropic.NewUserMessage(anthropic.NewTextBlock(query)), } + var finalText []string + // Initial Claude API call with tools - response, err := c.anthropic.Messages.New(ctx, anthropic.MessageNewParams{ - Model: model, - MaxTokens: 1024, - Messages: messages, - Tools: c.tools, - }) + response, err := c.createMessage(ctx, messages) if err != nil { - return "", fmt.Errorf("anthropic API request failed: %w", err) + return "", err } - var toolUseBlocks []anthropic.ToolUseBlock - var finalText []string - for _, block := range response.Content { - switch b := block.AsAny().(type) { - case anthropic.TextBlock: - finalText = append(finalText, b.Text) - case anthropic.ToolUseBlock: - toolUseBlocks = append(toolUseBlocks, b) + // Keep calling tools until Claude answers without one, up to a cap. + for turn := 0; turn < maxToolTurns; turn++ { + var toolUseBlocks []anthropic.ToolUseBlock + for _, block := range response.Content { + switch b := block.AsAny().(type) { + case anthropic.TextBlock: + finalText = append(finalText, b.Text) + case anthropic.ToolUseBlock: + toolUseBlocks = append(toolUseBlocks, b) + } } - } - if len(toolUseBlocks) == 0 { - return strings.Join(finalText, "\n"), nil - } + if len(toolUseBlocks) == 0 { + return strings.Join(finalText, "\n"), nil + } - // Append assistant's response to message history - messages = append(messages, response.ToParam()) + // Execute every tool call in this response and collect the results + var anthropicToolResults []anthropic.ContentBlockParamUnion + for _, toolUseBlock := range toolUseBlocks { + // Add information about the tool call to final text + finalText = append(finalText, fmt.Sprintf("[Calling tool %s with args %s]", toolUseBlock.Name, string(toolUseBlock.Input))) + + // Call the MCP server tool + mcpToolResult, err := c.session.CallTool(ctx, &mcp.CallToolParams{ + Name: toolUseBlock.Name, + Arguments: toolUseBlock.Input, + }) + if err != nil { + return "", fmt.Errorf("tool call %s failed: %w", toolUseBlock.Name, err) + } - // Execute each tool call and collect responses - var anthropicToolResults []anthropic.ContentBlockParamUnion - for _, toolUseBlock := range toolUseBlocks { - // Add information about the tool call to final text - finalText = append(finalText, fmt.Sprintf("[Calling tool %s with args %s]", toolUseBlock.Name, string(toolUseBlock.Input))) + if err := c.validateToolOutput(toolUseBlock.Name, mcpToolResult); err != nil { + return "", err + } - // Call the MCP server tool - mcpToolResult, err := c.session.CallTool(ctx, &mcp.CallToolParams{ - Name: toolUseBlock.Name, - Arguments: toolUseBlock.Input, - }) - if err != nil { - return "", fmt.Errorf("tool call %s failed: %w", toolUseBlock.Name, err) - } + // StructuredContent is data the application can use directly. + if items, ok := mcpToolResult.StructuredContent.([]any); ok { + finalText = append(finalText, fmt.Sprintf("[%s returned %d items]", toolUseBlock.Name, len(items))) + } - if err := c.validateToolOutput(toolUseBlock.Name, mcpToolResult); err != nil { - return "", err - } + // 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) + } + } - // StructuredContent is data the application can use directly. - if items, ok := mcpToolResult.StructuredContent.([]any); ok { - finalText = append(finalText, fmt.Sprintf("[%s returned %d items]", toolUseBlock.Name, len(items))) + anthropicToolResults = append(anthropicToolResults, anthropic.NewToolResultBlock( + toolUseBlock.ID, + strings.Join(texts, "\n"), + mcpToolResult.IsError, + )) } - // 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) - } + // Append the assistant's response and all tool results, in one user + // message, to the history. Every tool_use needs a matching tool_result. + messages = append(messages, response.ToParam()) + messages = append(messages, anthropic.NewUserMessage(anthropicToolResults...)) + + response, err = c.createMessage(ctx, messages) + if err != nil { + return "", err } + } - anthropicToolResults = append(anthropicToolResults, anthropic.NewToolResultBlock( - toolUseBlock.ID, - strings.Join(texts, "\n"), - mcpToolResult.IsError, - )) + // The turn cap was hit. Keep the text of the last response; drop its tool calls. + for _, block := range response.Content { + if b, ok := block.AsAny().(anthropic.TextBlock); ok { + finalText = append(finalText, b.Text) + } } + finalText = append(finalText, fmt.Sprintf("[Stopped after %d tool-use turns]", maxToolTurns)) - // Append tool responses to message history - messages = append(messages, anthropic.NewUserMessage(anthropicToolResults...)) + return strings.Join(finalText, "\n"), nil +} - // Make another API call with tool results - response, err = c.anthropic.Messages.New(ctx, anthropic.MessageNewParams{ +// createMessage calls Claude with the current history. Tools are passed on +// every call so Claude can keep calling them across turns. +func (c *MCPClient) createMessage(ctx context.Context, messages []anthropic.MessageParam) (*anthropic.Message, error) { + response, err := c.anthropic.Messages.New(ctx, anthropic.MessageNewParams{ Model: model, - MaxTokens: 1024, + MaxTokens: maxTokens, Messages: messages, + Tools: c.tools, }) if err != nil { - return "", fmt.Errorf("anthropic API request with tool results failed: %w", err) - } - - // Collect text from final response - for _, block := range response.Content { - switch b := block.AsAny().(type) { - case anthropic.TextBlock: - finalText = append(finalText, b.Text) - } + return nil, fmt.Errorf("anthropic API request failed: %w", err) } - - return strings.Join(finalText, "\n"), nil + return response, nil } func (c *MCPClient) ChatLoop(ctx context.Context) error { diff --git a/mcp-client-python/client.py b/mcp-client-python/client.py index 55c986a3..86fe46f1 100644 --- a/mcp-client-python/client.py +++ b/mcp-client-python/client.py @@ -10,7 +10,6 @@ load_dotenv() # load environment variables from .env -# Claude model constant ANTHROPIC_MODEL = "claude-sonnet-5" # Sonnet 5 thinks adaptively unless told otherwise, and max_tokens caps thinking # plus the reply, so leave room for both. @@ -119,6 +118,8 @@ async def process_query(self, query: str) -> str: tools=available_tools, ) + # The turn cap was hit. Keep the text of the last response; drop its tool calls. + final_text.extend(content.text for content in response.content if content.type == "text") final_text.append(f"[Stopped after {MAX_TOOL_TURNS} tool-use turns]") return "\n".join(final_text) diff --git a/mcp-client-ruby/client.rb b/mcp-client-ruby/client.rb index 60ac772a..c72a5f47 100644 --- a/mcp-client-ruby/client.rb +++ b/mcp-client-ruby/client.rb @@ -7,6 +7,10 @@ class MCPClient ANTHROPIC_MODEL = "claude-sonnet-5" + # Sonnet 5 thinks adaptively unless told otherwise, and max_tokens caps thinking + # plus the reply, so leave room for both. + MAX_TOKENS = 10000 + MAX_TOOL_TURNS = 10 def initialize @mcp_client = nil @@ -78,40 +82,40 @@ def process_query(query) { name: tool.name, description: tool.description, input_schema: tool.input_schema } end - # Initial Claude API call. - response = chat(messages, tools: available_tools) + final_text = [] - # Process response and handle tool calls. - if response.content.any?(Anthropic::Models::ToolUseBlock) - assistant_content = response.content.filter_map do |content_block| - case content_block + # Initial Claude API call with tools. + response = chat(messages, available_tools) + + # Keep calling tools until Claude answers without one, up to a cap. + MAX_TOOL_TURNS.times do + tool_uses = [] + response.content.each do |block| + case block when Anthropic::Models::TextBlock - { type: "text", text: content_block.text } + final_text << block.text when Anthropic::Models::ToolUseBlock - { type: "tool_use", id: content_block.id, name: content_block.name, input: content_block.input } + tool_uses << block end end - messages << { role: "assistant", content: assistant_content } - end - response.content.each_with_object([]) do |content, response_parts| - case content - when Anthropic::Models::TextBlock - response_parts << content.text - when Anthropic::Models::ToolUseBlock - # Execute tool call via MCP. - result = @mcp_client.call_tool(name: content.name, arguments: content.input) - response_parts << "[Calling tool #{content.name} with args #{content.input.to_json}]" + return final_text.join("\n") if tool_uses.empty? + + # 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}]" # structured_content is data the application can use directly; when a # tool returns an array, count its items rather than re-reading prose. + is_error = result.dig("result", "isError") == true structured = result.dig("result", "structuredContent") - unless result.dig("result", "isError") - @output_schemas[content.name]&.validate_result(structured) - response_parts << "[#{content.name} returned #{structured.length} items]" if structured.is_a?(Array) + unless is_error + @output_schemas[tool_use.name]&.validate_result(structured) + final_text << "[#{tool_use.name} returned #{structured.length} items]" if structured.is_a?(Array) end - # content is what the model reads. + # 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") @@ -119,30 +123,45 @@ def process_query(query) tool_result_content.to_s end - messages << { - role: "user", - content: [{ - type: "tool_result", - tool_use_id: content.id, - content: result_text - }] - } + { type: "tool_result", tool_use_id: tool_use.id, content: result_text, is_error: is_error } + end - # Get next response from Claude. - response = chat(messages) + # Append the assistant's response and all tool results, in one user + # message, to the history. Every tool_use needs a matching tool_result. + messages << { role: "assistant", content: assistant_content(response) } + messages << { role: "user", content: tool_results } - response.content.each do |content_block| - response_parts << content_block.text if content_block.is_a?(Anthropic::Models::TextBlock) - end - end - end.join("\n") + response = chat(messages, available_tools) + end + + # The turn cap was hit. Keep the text of the last response; drop its tool calls. + response.content.each do |block| + final_text << block.text if block.is_a?(Anthropic::Models::TextBlock) + end + final_text << "[Stopped after #{MAX_TOOL_TURNS} tool-use turns]" + final_text.join("\n") end - def chat(messages, tools: nil) - params = { model: ANTHROPIC_MODEL, max_tokens: 1000, messages: messages } - params[:tools] = tools if tools + # Convert a response's content blocks back into request parameters. + def assistant_content(response) + response.content.filter_map do |block| + case block + when Anthropic::Models::TextBlock + { type: "text", text: block.text } + when Anthropic::Models::ToolUseBlock + { type: "tool_use", id: block.id, name: block.name, input: block.input } + end + end + end - anthropic_client.messages.create(**params) + # Tools are passed on every call so Claude can keep calling them across turns. + def chat(messages, tools) + anthropic_client.messages.create( + model: ANTHROPIC_MODEL, + max_tokens: MAX_TOKENS, + messages: messages, + tools: tools + ) end def anthropic_client diff --git a/mcp-client-rust/src/main.rs b/mcp-client-rust/src/main.rs index 7c30c382..cedc8d92 100644 --- a/mcp-client-rust/src/main.rs +++ b/mcp-client-rust/src/main.rs @@ -1,7 +1,8 @@ use anyhow::{Context, Result, bail}; use genai::Client; use genai::chat::{ - ChatMessage, ChatRequest, ChatResponse, ContentPart, Tool as GenaiTool, ToolResponse, + ChatMessage, ChatOptions, ChatRequest, ChatResponse, ContentPart, Tool as GenaiTool, + ToolResponse, }; use jsonschema::Validator; use rmcp::model::{CallToolRequestParams, CallToolResult, Tool as McpTool}; @@ -13,6 +14,10 @@ use tokio::io::{self, AsyncBufReadExt, BufReader}; use tokio::process::Command; const MODEL_ANTHROPIC: &str = "claude-sonnet-5"; +/// Sonnet 5 thinks adaptively unless told otherwise, and max_tokens caps thinking +/// plus the reply, so leave room for both. +const MAX_TOKENS: u32 = 10000; +const MAX_TOOL_TURNS: usize = 10; struct MCPClient { anthropic: Client, @@ -103,20 +108,20 @@ impl MCPClient { let mut final_text = Vec::new(); // Initial Claude API call with tools - let mut chat_req = ChatRequest::new(messages.clone()).with_tools(self.tools.clone()); - let mut chat_rsp = self.request_model(&chat_req).await?; + let mut chat_rsp = self.request_model(&messages).await?; - // Process response content - collect text and handle tool calls - for text in chat_rsp.texts() { - final_text.push(text.to_string()); - } + // Keep calling tools until Claude answers without one, up to a cap. + for _ in 0..MAX_TOOL_TURNS { + for text in chat_rsp.texts() { + final_text.push(text.to_string()); + } - let tool_calls = chat_rsp.tool_calls(); - if !tool_calls.is_empty() { - // Append assistant's response to message history - messages.push(ChatMessage::assistant(chat_rsp.content.clone())); + let tool_calls = chat_rsp.tool_calls(); + if tool_calls.is_empty() { + return Ok(final_text.join("\n")); + } - // Execute each tool call and collect responses + // Execute every tool call in this response and collect the results let mut tool_results = Vec::new(); for tool_call in tool_calls { // Add information about the tool call to final text @@ -150,39 +155,51 @@ impl MCPClient { } // content is a list of block types; forward only the text ones. - let payload = tool_result + let mut payload = tool_result .content .iter() .filter_map(|block| block.as_text().map(|text| text.text.as_str())) .collect::>() .join("\n"); + // genai's ToolResponse cannot set Anthropic's `is_error` flag, so + // an error result is marked in the text instead. + if tool_result.is_error.unwrap_or(false) { + payload = format!("Error: {payload}"); + } + tool_results.push(ContentPart::ToolResponse(ToolResponse::new( tool_call.call_id.clone(), payload, ))); } - // Append tool responses to message history + // Append the assistant's response and all tool results, in one user + // message, to the history. Every tool call needs a matching result. + messages.push(ChatMessage::assistant(chat_rsp.content.clone())); messages.push(ChatMessage::user(tool_results)); - // Build the next request and query model - chat_req = ChatRequest::new(messages.clone()); - chat_rsp = self.request_model(&chat_req).await?; + chat_rsp = self.request_model(&messages).await?; + } - // Collect text from response - for text in chat_rsp.texts() { - final_text.push(text.to_string()); - } + // The turn cap was hit. Keep the text of the last response; drop its tool calls. + for text in chat_rsp.texts() { + final_text.push(text.to_string()); } + final_text.push(format!("[Stopped after {MAX_TOOL_TURNS} tool-use turns]")); Ok(final_text.join("\n")) } - async fn request_model(&self, chat_req: &ChatRequest) -> Result { + /// Call Claude with the current history. Tools are passed on every call so + /// Claude can keep calling them across turns. + async fn request_model(&self, messages: &[ChatMessage]) -> Result { + let chat_req = ChatRequest::new(messages.to_vec()).with_tools(self.tools.clone()); + let options = ChatOptions::default().with_max_tokens(MAX_TOKENS); + let response = self .anthropic - .exec_chat(MODEL_ANTHROPIC, chat_req.clone(), None) + .exec_chat(MODEL_ANTHROPIC, chat_req, Some(&options)) .await .context("Anthropic chat request failed")?; diff --git a/mcp-client-typescript/index.ts b/mcp-client-typescript/index.ts index ccea35cc..809ca510 100644 --- a/mcp-client-typescript/index.ts +++ b/mcp-client-typescript/index.ts @@ -9,6 +9,9 @@ import dotenv from "dotenv"; dotenv.config({ quiet: true }); // load environment variables from .env const ANTHROPIC_MODEL = "claude-sonnet-5"; +// Sonnet 5 thinks adaptively unless told otherwise, and max_tokens caps thinking +// plus the reply, so leave room for both. +const MAX_TOKENS = 10000; const MAX_TOOL_TURNS = 10; class MCPClient { @@ -92,7 +95,7 @@ class MCPClient { let response = await this.anthropic.messages.create({ model: ANTHROPIC_MODEL, - max_tokens: 1000, + max_tokens: MAX_TOKENS, messages, tools: this.tools, }); @@ -133,11 +136,15 @@ class MCPClient { ); } - // content is what the model reads. + // 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 as Anthropic.ToolResultBlockParam["content"], + content: result.content + .filter((block) => block.type === "text") + .map((block) => block.text) + .join("\n"), is_error: result.isError === true, }); } @@ -150,12 +157,18 @@ class MCPClient { response = await this.anthropic.messages.create({ model: ANTHROPIC_MODEL, - max_tokens: 1000, + max_tokens: MAX_TOKENS, messages, tools: this.tools, }); } + // The turn cap was hit. Keep the text of the last response; drop its tool calls. + for (const block of response.content) { + if (block.type === "text") { + finalText.push(block.text); + } + } finalText.push(`[Stopped after ${MAX_TOOL_TURNS} tool-use turns]`); return finalText.join("\n"); } diff --git a/tests/README.md b/tests/README.md index 507bd57b..6851ddca 100644 --- a/tests/README.md +++ b/tests/README.md @@ -8,6 +8,7 @@ The smoke tests verify: - **Servers**: Each weather server (Python, TypeScript, Rust, Go, Ruby) can start, respond to MCP protocol requests, and honour the output schemas it advertises - **Clients**: Each MCP client (Python, TypeScript, Ruby, Go, Rust) can connect to a mock server and list tools +- **Tool loop**: Each client (Python, TypeScript, Ruby, Go) runs a query through a scripted tool-use loop against a fake Anthropic API and forwards tool results correctly ## Structured content @@ -20,6 +21,22 @@ The array case is the one worth guarding. A server that advertises `{"type": "ar Tool calls reach the live NWS API. When it is unreachable the tools return an error result, which the test reports as a skip rather than a failure — someone else's outage should not fail the build. +## Tool loop + +Connecting and listing tools does not exercise the chat loop, so each client is also driven through one scripted query 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 fake API scripts one conversation — a response with two `tool_use` blocks, then one for a tool the mock server lacks, then a final answer — and checks every request the client sends: + +- `tools` are passed on every call, not only the first; +- `max_tokens` is 10000, leaving room for the model's adaptive thinking; +- every `tool_use` gets a `tool_result` in a single following user message, with matching `tool_use_id`s; +- an MCP `isError` result is forwarded as `is_error`; +- the client makes exactly three calls, exits 0, and prints the final answer. + +A client that does one tool round and stops, drops `tools` on the follow-up call, or sends one `tool_result` per message passes the no-key test and fails this one. + +The Rust client is not covered yet. Its `genai` crate reads no environment variable for the endpoint, and it negotiates protocol `2025-11-25`, under which the mock's array-rooted tool is refused at call time. + ## Running Tests ```bash @@ -94,6 +111,22 @@ It advertises two tools whose output schemas cover both shapes a structured resu 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, pipes in one query followed by `quit`, and reports which checks failed along with the client's output. + +**Usage**: + +```bash +node tests/helpers/build/tool-loop-test.js [args...] +``` + +**Example**: + +```bash +node tests/helpers/build/tool-loop-test.js node mcp-client-typescript/build/index.js tests/helpers/build/mock-mcp-server.js +``` + ## CI/CD Integration Tests run automatically on pull requests via GitHub Actions. See `.github/workflows/ci.yml` for the CI configuration. diff --git a/tests/helpers/tool-loop-test.ts b/tests/helpers/tool-loop-test.ts new file mode 100644 index 00000000..b67c734f --- /dev/null +++ b/tests/helpers/tool-loop-test.ts @@ -0,0 +1,251 @@ +#!/usr/bin/env node +/** + * Tool-loop test for clients + * + * Runs a quickstart client through one scripted query and checks that its + * tool-use loop is right. Two stand-ins replace the real world: + * + * - the mock MCP server (mock-mcp-server.ts), passed as part of the client + * command exactly as in the no-key client test; + * - 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. + * + * The fake API scripts one conversation: + * + * call 1 -> text plus two tool_use blocks in one response (parallel calls) + * call 2 -> one tool_use for a tool the mock server does not have, so the + * tool result is an error + * call 3 -> a final text answer + * + * and checks every request the client sends: tools are passed each time, + * max_tokens leaves room for thinking, every tool_use gets a tool_result in a + * single following user message, ids match, and the error result is flagged. + * A client that does one tool round and stops, drops `tools` on the second + * call, or sends one tool_result per message fails here and passes the no-key + * test. + * + * Usage: node tool-loop-test.js [args...] + */ + +import { spawn } from "node:child_process"; +import http from "node:http"; + +/** Sonnet 5 thinks adaptively; the clients reserve this much for it. */ +const EXPECTED_MAX_TOKENS = 10000; +const EXPECTED_CALLS = 3; +const QUERY = "What is the weather like?\n"; +const CLIENT_TIMEOUT_MS = 120_000; + +type Block = Record & { type: string }; +type Message = { role: string; content: string | Block[] }; +type Request = { + model?: string; + max_tokens?: number; + tools?: unknown[]; + messages: Message[]; +}; + +const failures: string[] = []; +let passed = 0; +let calls = 0; + +function check(condition: boolean, what: string) { + if (condition) { + passed++; + } else { + failures.push(`call ${calls}: ${what}`); + } +} + +function toolResultsOf(message: Message | undefined): Block[] { + if (!message || !Array.isArray(message.content)) return []; + return message.content.filter((block) => block.type === "tool_result"); +} + +/** tool_result content may be a string or a list of text blocks. */ +function textOf(content: unknown): string { + if (typeof content === "string") return content; + if (Array.isArray(content)) { + return content + .filter((block) => block?.type === "text") + .map((block) => String(block.text)) + .join("\n"); + } + return ""; +} + +/** The scripted assistant turn for this call, after checking the request. */ +function respond(request: Request): Block[] { + calls++; + const { messages } = request; + const last = messages[messages.length - 1]; + const results = toolResultsOf(last); + + check( + Array.isArray(request.tools) && request.tools.length === 2, + `tools are passed (got ${request.tools?.length ?? "none"})`, + ); + check( + request.max_tokens === EXPECTED_MAX_TOKENS, + `max_tokens is ${EXPECTED_MAX_TOKENS} (got ${request.max_tokens})`, + ); + + if (calls === 1) { + check(messages.length === 1 && last.role === "user", "history is the user query"); + return [ + { type: "text", text: "Checking two things." }, + { type: "tool_use", id: "tu_1", name: "get_alerts", input: { state: "CA" } }, + { + type: "tool_use", + id: "tu_2", + name: "get_forecast", + input: { latitude: 38.58, longitude: -121.49 }, + }, + ]; + } + + if (calls === 2) { + check(messages.length === 3, `history has 3 messages (got ${messages.length})`); + check(messages[1]?.role === "assistant", "assistant turn is kept in history"); + check( + last.role === "user" && results.length === 2, + `both tool_results arrive in one user message (got ${results.length})`, + ); + const ids = results.map((block) => block.tool_use_id).sort().join(","); + check(ids === "tu_1,tu_2", `tool_use_ids match (got ${ids})`); + check( + results.every((block) => textOf(block.content).length > 0), + "tool_result content carries the tool's text", + ); + check( + results.every((block) => block.is_error !== true), + "successful results are not flagged as errors", + ); + return [{ type: "tool_use", id: "tu_3", name: "no_such_tool", input: {} }]; + } + + if (calls === 3) { + check(messages.length === 5, `history has 5 messages (got ${messages.length})`); + check( + results.length === 1 && results[0].tool_use_id === "tu_3", + "second-step tool_result is present", + ); + // Rust's genai crate has no is_error field, so that client marks the + // text instead; accept either. + const flagged = results[0]?.is_error === true; + const marked = textOf(results[0]?.content).startsWith("Error:"); + check(flagged || marked, "error result is forwarded as an error"); + return [{ type: "text", text: "All done." }]; + } + + check(false, "unexpected extra call"); + return [{ type: "text", text: "extra" }]; +} + +function startFakeApi(): Promise { + const server = http.createServer((req, res) => { + let body = ""; + req.on("data", (chunk) => (body += chunk)); + req.on("end", () => { + let request: Request; + try { + request = JSON.parse(body) as Request; + } catch { + calls++; + check(false, "request body is JSON"); + res.writeHead(400).end(); + return; + } + const content = respond(request); + const stopReason = content.some((block) => block.type === "tool_use") + ? "tool_use" + : "end_turn"; + res.setHeader("content-type", "application/json"); + res.end( + JSON.stringify({ + id: `msg_${calls}`, + type: "message", + role: "assistant", + model: request.model, + content, + stop_reason: stopReason, + stop_sequence: null, + usage: { input_tokens: 1, output_tokens: 1 }, + }), + ); + }); + }); + return new Promise((resolve) => server.listen(0, "127.0.0.1", () => resolve(server))); +} + +function runClient( + command: string, + args: string[], + baseUrl: string, +): Promise<{ code: number | null; stdout: string; stderr: string }> { + return new Promise((resolve) => { + const child = spawn(command, args, { + env: { ...process.env, ANTHROPIC_BASE_URL: baseUrl, ANTHROPIC_API_KEY: "test-key" }, + stdio: ["pipe", "pipe", "pipe"], + }); + let stdout = ""; + let stderr = ""; + child.stdout.on("data", (chunk) => (stdout += chunk)); + child.stderr.on("data", (chunk) => (stderr += chunk)); + const timer = setTimeout(() => { + failures.push(`client did not exit within ${CLIENT_TIMEOUT_MS / 1000}s`); + child.kill(); + }, CLIENT_TIMEOUT_MS); + child.on("error", (error) => { + clearTimeout(timer); + failures.push(`could not start client: ${error.message}`); + resolve({ code: null, stdout, stderr }); + }); + child.on("close", (code) => { + clearTimeout(timer); + resolve({ code, stdout, stderr }); + }); + child.stdin.write(QUERY); + child.stdin.write("quit\n"); + child.stdin.end(); + }); +} + +async function main() { + const [command, ...args] = process.argv.slice(2); + if (!command) { + console.error("Usage: node tool-loop-test.js [args...]"); + process.exit(1); + } + + const api = await startFakeApi(); + const address = api.address(); + if (!address || typeof address === "string") throw new Error("no port"); + const baseUrl = `http://127.0.0.1:${address.port}`; + console.error(`Testing client tool loop: ${command} ${args.join(" ")}`); + + const { code, stdout, stderr } = await runClient(command, args, baseUrl); + api.close(); + + calls = EXPECTED_CALLS; // label the closing checks + check(code === 0, `client exited 0 (got ${code})`); + check(calls === EXPECTED_CALLS, `API was called ${EXPECTED_CALLS} times`); + check(stdout.includes("All done."), "client printed the final answer"); + + if (failures.length === 0) { + console.error(`✓ Tool loop correct (${passed} checks)`); + return; + } + console.error(`✗ Tool loop failed ${failures.length} of ${passed + failures.length} checks:`); + for (const failure of failures) console.error(` - ${failure}`); + console.error("--- client stdout ---"); + console.error(stdout.trim()); + console.error("--- client stderr ---"); + console.error(stderr.trim()); + process.exit(1); +} + +main().catch((error) => { + console.error("✗ Tool loop test crashed:", error); + process.exit(1); +}); diff --git a/tests/smoke-test.sh b/tests/smoke-test.sh index 91352dcc..5cbc1782 100755 --- a/tests/smoke-test.sh +++ b/tests/smoke-test.sh @@ -13,6 +13,7 @@ TESTS_DIR="${PROJECT_ROOT}/tests" # Setup common test variables TEST_CLIENT="${PROJECT_ROOT}/tests/helpers/build/mcp-test-client.js" MOCK_SERVER="${PROJECT_ROOT}/tests/helpers/build/mock-mcp-server.js" +TOOL_LOOP_TEST="${PROJECT_ROOT}/tests/helpers/build/tool-loop-test.js" # Track test results FAILED_TESTS=() @@ -153,6 +154,57 @@ test_mcp_client_rust() { ANTHROPIC_API_KEY= "${client_bin}" node "${MOCK_SERVER}" >/dev/null 2>&1 } +# 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 scripts one query as two +# parallel tool calls, then a failing one, then an answer. 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() { + check_dependency uv || return 1 + local client_dir="${PROJECT_ROOT}/mcp-client-python" + node "${TOOL_LOOP_TEST}" uv --directory "${client_dir}" run python "${client_dir}/client.py" "${MOCK_SERVER}" +} + +# Test: TypeScript MCP client tool loop +test_tool_loop_typescript() { + check_dependency node || return 1 + check_dependency npm || return 1 + local client_dir="${PROJECT_ROOT}/mcp-client-typescript" + ensure_built "${client_dir}" "" || return 1 + node "${TOOL_LOOP_TEST}" node "${client_dir}/build/index.js" "${MOCK_SERVER}" +} + +# Test: Ruby MCP client tool loop +test_tool_loop_ruby() { + check_dependency ruby || return 1 + check_dependency bundle || return 1 + local client_dir="${PROJECT_ROOT}/mcp-client-ruby" + ensure_bundled "${client_dir}" || return 1 + (cd "${client_dir}" && node "${TOOL_LOOP_TEST}" bundle exec ruby client.rb "${MOCK_SERVER}") +} + +# Test: Go MCP client tool loop +test_tool_loop_go() { + check_dependency go || return 1 + local client_dir="${PROJECT_ROOT}/mcp-client-go" + ensure_built "${client_dir}" mcp-client-go || return 1 + + local client_bin + client_bin=$(resolve_binary "${client_dir}/mcp-client-go") || { + print_error "no mcp-client-go binary found in ${client_dir}" + return 1 + } + + node "${TOOL_LOOP_TEST}" "${client_bin}" node "${MOCK_SERVER}" +} + +# The Rust client has no tool-loop test yet. Its genai crate reads no +# environment variable for the API endpoint, so the fake API cannot be reached +# without adding code to the example, and it negotiates protocol 2025-11-25, +# under which the mock's array-rooted tool is refused at call time. + # Run all tests print_header "Running smoke tests" @@ -166,6 +218,10 @@ run_test "mcp-client-typescript" test_mcp_client_typescript run_test "mcp-client-ruby" test_mcp_client_ruby run_test "mcp-client-go" test_mcp_client_go run_test "mcp-client-rust" test_mcp_client_rust +run_test "mcp-client-python tool loop" test_tool_loop_python +run_test "mcp-client-typescript tool loop" test_tool_loop_typescript +run_test "mcp-client-ruby tool loop" test_tool_loop_ruby +run_test "mcp-client-go tool loop" test_tool_loop_go # Print summary echo "" diff --git a/tests/utils.sh b/tests/utils.sh index a480a65c..2a64af06 100644 --- a/tests/utils.sh +++ b/tests/utils.sh @@ -76,11 +76,12 @@ setup_test() { CLIENT_DIR="${PROJECT_ROOT}/${test_name}" TEST_CLIENT="${PROJECT_ROOT}/tests/helpers/build/mcp-test-client.js" MOCK_SERVER="${PROJECT_ROOT}/tests/helpers/build/mock-mcp-server.js" + TOOL_LOOP_TEST="${PROJECT_ROOT}/tests/helpers/build/tool-loop-test.js" } # Ensure test helpers are built ensure_helpers_built() { - if [ ! -f "${TEST_CLIENT}" ] || [ ! -f "${MOCK_SERVER}" ]; then + if [ ! -f "${TEST_CLIENT}" ] || [ ! -f "${MOCK_SERVER}" ] || [ ! -f "${TOOL_LOOP_TEST}" ]; then print_error "Test helpers not built" print_header "Building test helpers..." cd "${PROJECT_ROOT}/tests/helpers" || return 1 From 9596e510ec2cc7032fa509ee14e1ff7f9aef5472 Mon Sep 17 00:00:00 2001 From: a-akimov Date: Wed, 16 Sep 2026 17:58:37 +0200 Subject: [PATCH 2/3] Improvements --- mcp-client-go/main.go | 13 +- mcp-client-python/client.py | 6 +- mcp-client-ruby/client.rb | 8 +- mcp-client-rust/src/main.rs | 7 +- mcp-client-typescript/index.ts | 10 +- tests/README.md | 16 +-- tests/helpers/tool-loop-test.ts | 238 +++++++++++++++++++++++--------- 7 files changed, 213 insertions(+), 85 deletions(-) diff --git a/mcp-client-go/main.go b/mcp-client-go/main.go index d9dd0b85..7358e754 100644 --- a/mcp-client-go/main.go +++ b/mcp-client-go/main.go @@ -243,13 +243,20 @@ func (c *MCPClient) ProcessQuery(ctx context.Context, query string) (string, err } } - // The turn cap was hit. Keep the text of the last response; drop its tool calls. + // The turn cap was hit. Keep the last response's text. If it asked for + // more tools, say they were not run. + wantsTools := false for _, block := range response.Content { - if b, ok := block.AsAny().(anthropic.TextBlock); ok { + switch b := block.AsAny().(type) { + case anthropic.TextBlock: finalText = append(finalText, b.Text) + case anthropic.ToolUseBlock: + wantsTools = true } } - finalText = append(finalText, fmt.Sprintf("[Stopped after %d tool-use turns]", maxToolTurns)) + if wantsTools { + finalText = append(finalText, fmt.Sprintf("[Stopped after %d tool-use turns]", maxToolTurns)) + } return strings.Join(finalText, "\n"), nil } diff --git a/mcp-client-python/client.py b/mcp-client-python/client.py index 86fe46f1..80435c5c 100644 --- a/mcp-client-python/client.py +++ b/mcp-client-python/client.py @@ -118,9 +118,11 @@ async def process_query(self, query: str) -> str: tools=available_tools, ) - # The turn cap was hit. Keep the text of the last response; drop its tool calls. + # The turn cap was hit. Keep the last response's text. If it asked for + # more tools, say they were not run. final_text.extend(content.text for content in response.content if content.type == "text") - final_text.append(f"[Stopped after {MAX_TOOL_TURNS} tool-use turns]") + if any(content.type == "tool_use" for content in response.content): + final_text.append(f"[Stopped after {MAX_TOOL_TURNS} tool-use turns]") return "\n".join(final_text) async def chat_loop(self): diff --git a/mcp-client-ruby/client.rb b/mcp-client-ruby/client.rb index c72a5f47..db86715f 100644 --- a/mcp-client-ruby/client.rb +++ b/mcp-client-ruby/client.rb @@ -53,6 +53,7 @@ def chat_loop loop do print "\nQuery: " + $stdout.flush # a pipe is not line-buffered, so show the prompt before waiting line = $stdin.gets break if line.nil? @@ -134,11 +135,14 @@ def process_query(query) response = chat(messages, available_tools) end - # The turn cap was hit. Keep the text of the last response; drop its tool calls. + # The turn cap was hit. Keep the last response's text. If it asked for + # more tools, say they were not run. response.content.each do |block| final_text << block.text if block.is_a?(Anthropic::Models::TextBlock) end - final_text << "[Stopped after #{MAX_TOOL_TURNS} tool-use turns]" + if response.content.any?(Anthropic::Models::ToolUseBlock) + final_text << "[Stopped after #{MAX_TOOL_TURNS} tool-use turns]" + end final_text.join("\n") end diff --git a/mcp-client-rust/src/main.rs b/mcp-client-rust/src/main.rs index cedc8d92..7623bc29 100644 --- a/mcp-client-rust/src/main.rs +++ b/mcp-client-rust/src/main.rs @@ -182,11 +182,14 @@ impl MCPClient { chat_rsp = self.request_model(&messages).await?; } - // The turn cap was hit. Keep the text of the last response; drop its tool calls. + // The turn cap was hit. Keep the last response's text. If it asked for + // more tools, say they were not run. for text in chat_rsp.texts() { final_text.push(text.to_string()); } - final_text.push(format!("[Stopped after {MAX_TOOL_TURNS} tool-use turns]")); + if !chat_rsp.tool_calls().is_empty() { + final_text.push(format!("[Stopped after {MAX_TOOL_TURNS} tool-use turns]")); + } Ok(final_text.join("\n")) } diff --git a/mcp-client-typescript/index.ts b/mcp-client-typescript/index.ts index 809ca510..935274df 100644 --- a/mcp-client-typescript/index.ts +++ b/mcp-client-typescript/index.ts @@ -163,13 +163,19 @@ class MCPClient { }); } - // The turn cap was hit. Keep the text of the last response; drop its tool calls. + // The turn cap was hit. Keep the last response's text. If it asked for + // more tools, say they were not run. + let wantsTools = false; for (const block of response.content) { if (block.type === "text") { finalText.push(block.text); + } else if (block.type === "tool_use") { + wantsTools = true; } } - finalText.push(`[Stopped after ${MAX_TOOL_TURNS} tool-use turns]`); + if (wantsTools) { + finalText.push(`[Stopped after ${MAX_TOOL_TURNS} tool-use turns]`); + } return finalText.join("\n"); } diff --git a/tests/README.md b/tests/README.md index 6851ddca..d7ea197d 100644 --- a/tests/README.md +++ b/tests/README.md @@ -23,17 +23,15 @@ 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 one scripted query 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. +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. -The fake API scripts one conversation — a response with two `tool_use` blocks, then one for a tool the mock server lacks, then a final answer — and checks every request the client sends: +The fake API picks a script from the query text and checks every request the client sends: -- `tools` are passed on every call, not only the first; -- `max_tokens` is 10000, leaving room for the model's adaptive thinking; -- every `tool_use` gets a `tool_result` in a single following user message, with matching `tool_use_id`s; -- an MCP `isError` result is forwarded as `is_error`; -- the client makes exactly three calls, exits 0, and prints the final answer. +- **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. -A client that does one tool round and stops, drops `tools` on the follow-up call, or sends one `tool_result` per message 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, or gets the turn cap wrong passes the no-key test and fails this one. The Rust client is not covered yet. Its `genai` crate reads no environment variable for the endpoint, and it negotiates protocol `2025-11-25`, under which the mock's array-rooted tool is refused at call time. @@ -113,7 +111,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, pipes in one query followed by `quit`, 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 three 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/tool-loop-test.ts b/tests/helpers/tool-loop-test.ts index b67c734f..336520c1 100644 --- a/tests/helpers/tool-loop-test.ts +++ b/tests/helpers/tool-loop-test.ts @@ -2,7 +2,7 @@ /** * Tool-loop test for clients * - * Runs a quickstart client through one scripted query and checks that its + * Runs a quickstart client through scripted queries and checks that its * tool-use loop is right. Two stand-ins replace the real world: * * - the mock MCP server (mock-mcp-server.ts), passed as part of the client @@ -10,19 +10,22 @@ * - 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. * - * The fake API scripts one conversation: + * Three queries are piped into one client process, and the fake API picks the + * script from the query text: * - * call 1 -> text plus two tool_use blocks in one response (parallel calls) - * call 2 -> one tool_use for a tool the mock server does not have, so the - * tool result is an error - * call 3 -> a final text answer + * - "parallel tools": text plus two tool_use blocks in one response, then one + * tool_use for a tool the mock server lacks (an error result), then an + * answer. Checks that tools are passed on every call, max_tokens leaves + * room for thinking, every tool_use gets a tool_result in a single + * following user message with matching ids, and the error is forwarded. + * - "ten tool turns": exactly MAX_TOOL_TURNS tool calls, then an answer. The + * 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. * - * and checks every request the client sends: tools are passed each time, - * max_tokens leaves room for thinking, every tool_use gets a tool_result in a - * single following user message, ids match, and the error result is flagged. * A client that does one tool round and stops, drops `tools` on the second - * call, or sends one tool_result per message fails here and passes the no-key - * test. + * call, sends one tool_result per message, or gets the turn cap wrong fails + * here and passes the no-key test. * * Usage: node tool-loop-test.js [args...] */ @@ -32,9 +35,25 @@ import http from "node:http"; /** Sonnet 5 thinks adaptively; the clients reserve this much for it. */ const EXPECTED_MAX_TOKENS = 10000; -const EXPECTED_CALLS = 3; -const QUERY = "What is the weather like?\n"; -const CLIENT_TIMEOUT_MS = 120_000; +/** MAX_TOOL_TURNS in every client. */ +const MAX_TOOL_TURNS = 10; +const STOP_NOTICE = `[Stopped after ${MAX_TOOL_TURNS} tool-use turns]`; +/** Every client prints this before reading a query. */ +const PROMPT = "Query: "; +const CLIENT_TIMEOUT_MS = 180_000; + +const QUERIES = { + parallel: "parallel tools", + atLimit: "ten tool turns", + overrun: "endless tool turns", +} as const; +type Scenario = keyof typeof QUERIES; + +const FINAL_ANSWER: Record = { + parallel: "All done.", + atLimit: "Answered at the limit.", + overrun: "never sent", +}; type Block = Record & { type: string }; type Message = { role: string; content: string | Block[] }; @@ -47,13 +66,13 @@ type Request = { const failures: string[] = []; let passed = 0; -let calls = 0; +const calls: Record = { parallel: 0, atLimit: 0, overrun: 0 }; function check(condition: boolean, what: string) { if (condition) { passed++; } else { - failures.push(`call ${calls}: ${what}`); + failures.push(what); } } @@ -62,7 +81,7 @@ function toolResultsOf(message: Message | undefined): Block[] { return message.content.filter((block) => block.type === "tool_result"); } -/** tool_result content may be a string or a list of text blocks. */ +/** Text content may be a string or a list of text blocks. */ function textOf(content: unknown): string { if (typeof content === "string") return content; if (Array.isArray(content)) { @@ -74,72 +93,130 @@ function textOf(content: unknown): string { return ""; } -/** The scripted assistant turn for this call, after checking the request. */ -function respond(request: Request): Block[] { - calls++; +/** 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--) { + const message = messages[i]; + if (message.role !== "user" || toolResultsOf(message).length > 0) continue; + const text = textOf(message.content).trim(); + return (Object.keys(QUERIES) as Scenario[]).find((key) => QUERIES[key] === text); + } + return undefined; +} + +function toolUse(id: string, name: string, input: Record): Block { + return { type: "tool_use", id, name, input }; +} + +function text(value: string): Block { + return { type: "text", text: value }; +} + +/** The scripted assistant turn for the parallel-tools query. */ +function respondParallel(n: number, request: Request): Block[] { + const label = `parallel call ${n}`; const { messages } = request; const last = messages[messages.length - 1]; const results = toolResultsOf(last); - check( - Array.isArray(request.tools) && request.tools.length === 2, - `tools are passed (got ${request.tools?.length ?? "none"})`, - ); - check( - request.max_tokens === EXPECTED_MAX_TOKENS, - `max_tokens is ${EXPECTED_MAX_TOKENS} (got ${request.max_tokens})`, - ); - - if (calls === 1) { - check(messages.length === 1 && last.role === "user", "history is the user query"); + if (n === 1) { + check(messages.length === 1 && last.role === "user", `${label}: history is the user query`); return [ - { type: "text", text: "Checking two things." }, - { type: "tool_use", id: "tu_1", name: "get_alerts", input: { state: "CA" } }, - { - type: "tool_use", - id: "tu_2", - name: "get_forecast", - input: { latitude: 38.58, longitude: -121.49 }, - }, + text("Checking two things."), + toolUse("tu_1", "get_alerts", { state: "CA" }), + toolUse("tu_2", "get_forecast", { latitude: 38.58, longitude: -121.49 }), ]; } - if (calls === 2) { - check(messages.length === 3, `history has 3 messages (got ${messages.length})`); - check(messages[1]?.role === "assistant", "assistant turn is kept in history"); + if (n === 2) { + check(messages.length === 3, `${label}: history has 3 messages (got ${messages.length})`); + check(messages[1]?.role === "assistant", `${label}: assistant turn is kept in history`); check( last.role === "user" && results.length === 2, - `both tool_results arrive in one user message (got ${results.length})`, + `${label}: both tool_results arrive in one user message (got ${results.length})`, ); const ids = results.map((block) => block.tool_use_id).sort().join(","); - check(ids === "tu_1,tu_2", `tool_use_ids match (got ${ids})`); + check(ids === "tu_1,tu_2", `${label}: tool_use_ids match (got ${ids})`); check( results.every((block) => textOf(block.content).length > 0), - "tool_result content carries the tool's text", + `${label}: tool_result content carries the tool's text`, ); check( results.every((block) => block.is_error !== true), - "successful results are not flagged as errors", + `${label}: successful results are not flagged as errors`, ); - return [{ type: "tool_use", id: "tu_3", name: "no_such_tool", input: {} }]; + return [toolUse("tu_3", "no_such_tool", {})]; } - if (calls === 3) { - check(messages.length === 5, `history has 5 messages (got ${messages.length})`); + if (n === 3) { + check(messages.length === 5, `${label}: history has 5 messages (got ${messages.length})`); check( results.length === 1 && results[0].tool_use_id === "tu_3", - "second-step tool_result is present", + `${label}: second-step tool_result is present`, ); // Rust's genai crate has no is_error field, so that client marks the // text instead; accept either. const flagged = results[0]?.is_error === true; const marked = textOf(results[0]?.content).startsWith("Error:"); - check(flagged || marked, "error result is forwarded as an error"); - return [{ type: "text", text: "All done." }]; + check(flagged || marked, `${label}: error result is forwarded as an error`); + return [text(FINAL_ANSWER.parallel)]; + } + + 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 + * script answers on call MAX_TOOL_TURNS + 1; past it, it never stops asking. + */ +function respondTurnLimit(scenario: "atLimit" | "overrun", n: number, request: Request): Block[] { + const label = `${scenario === "atLimit" ? "at-limit" : "overrun"} call ${n}`; + const prefix = scenario === "atLimit" ? "tl" : "ov"; + const last = request.messages[request.messages.length - 1]; + const results = toolResultsOf(last); + + if (n > 1) { + check( + results.length === 1 && results[0].tool_use_id === `${prefix}_${n - 1}`, + `${label}: previous tool_result is present`, + ); } - check(false, "unexpected extra call"); - return [{ type: "text", text: "extra" }]; + // The initial call plus MAX_TOOL_TURNS follow-ups is the most a client makes. + if (n > MAX_TOOL_TURNS + 1) { + check(false, `${label}: client called past the turn limit`); + return [text("extra")]; + } + if (scenario === "atLimit" && n === MAX_TOOL_TURNS + 1) { + return [text(FINAL_ANSWER.atLimit)]; + } + // Distinct tools per scenario, so their "[Calling tool ...]" lines can be + // told apart in the client's output. + return scenario === "atLimit" + ? [toolUse(`${prefix}_${n}`, "get_forecast", { latitude: 40.7, longitude: -74.0 })] + : [toolUse(`${prefix}_${n}`, "get_alerts", { state: "NY" })]; +} + +function respond(request: Request): Block[] { + const scenario = scenarioOf(request.messages); + if (!scenario) { + check(false, "request belongs to a known query"); + return [text("unknown query")]; + } + const n = ++calls[scenario]; + + check( + Array.isArray(request.tools) && request.tools.length === 2, + `${scenario} call ${n}: tools are passed (got ${request.tools?.length ?? "none"})`, + ); + check( + request.max_tokens === EXPECTED_MAX_TOKENS, + `${scenario} call ${n}: max_tokens is ${EXPECTED_MAX_TOKENS} (got ${request.max_tokens})`, + ); + + return scenario === "parallel" ? respondParallel(n, request) : respondTurnLimit(scenario, n, request); } function startFakeApi(): Promise { @@ -151,7 +228,6 @@ function startFakeApi(): Promise { try { request = JSON.parse(body) as Request; } catch { - calls++; check(false, "request body is JSON"); res.writeHead(400).end(); return; @@ -163,7 +239,7 @@ function startFakeApi(): Promise { res.setHeader("content-type", "application/json"); res.end( JSON.stringify({ - id: `msg_${calls}`, + id: `msg_${Object.values(calls).reduce((a, b) => a + b, 0)}`, type: "message", role: "assistant", model: request.model, @@ -190,7 +266,19 @@ function runClient( }); let stdout = ""; let stderr = ""; - child.stdout.on("data", (chunk) => (stdout += chunk)); + // Type each line only once the client shows its prompt, as a user would. + // Lines written ahead of time are dropped by readline-based clients, and + // closing stdin early ends the session before the later queries. + const lines = [...Object.values(QUERIES), "quit"]; + let prompts = 0; + child.stdout.on("data", (chunk) => { + stdout += chunk; + const seen = stdout.split(PROMPT).length - 1; + while (prompts < seen && lines.length > 0) { + prompts++; + child.stdin.write(`${lines.shift()}\n`); + } + }); child.stderr.on("data", (chunk) => (stderr += chunk)); const timer = setTimeout(() => { failures.push(`client did not exit within ${CLIENT_TIMEOUT_MS / 1000}s`); @@ -205,12 +293,36 @@ function runClient( clearTimeout(timer); resolve({ code, stdout, stderr }); }); - child.stdin.write(QUERY); - child.stdin.write("quit\n"); - child.stdin.end(); }); } +/** Checks on the client's output once it has exited. */ +function checkOutput(code: number | null, stdout: string) { + check(code === 0, `client exited 0 (got ${code})`); + check(calls.parallel === 3, `parallel: API was called 3 times (got ${calls.parallel})`); + check(stdout.includes(FINAL_ANSWER.parallel), "parallel: client printed the final answer"); + + const expected = MAX_TOOL_TURNS + 1; + check(calls.atLimit === expected, `at-limit: API was called ${expected} times (got ${calls.atLimit})`); + check(calls.overrun === expected, `overrun: API was called ${expected} times (got ${calls.overrun})`); + + // Output arrives in query order. The at-limit answer is followed by the + // overrun query's first tool call; a stop notice between the two is wrong, + // and exactly one must follow. + const answerAt = stdout.indexOf(FINAL_ANSWER.atLimit); + const overrunAt = answerAt < 0 ? -1 : stdout.indexOf("[Calling tool get_alerts", answerAt); + check(answerAt >= 0, "at-limit: client printed the final answer"); + check(overrunAt > answerAt, "overrun: client called the tool"); + if (answerAt >= 0 && overrunAt > answerAt) { + check( + !stdout.slice(answerAt, overrunAt).includes(STOP_NOTICE), + "at-limit: no stop notice after an answer at the limit", + ); + const notices = stdout.slice(overrunAt).split(STOP_NOTICE).length - 1; + check(notices === 1, `overrun: stop notice printed once (got ${notices})`); + } +} + async function main() { const [command, ...args] = process.argv.slice(2); if (!command) { @@ -226,11 +338,7 @@ async function main() { const { code, stdout, stderr } = await runClient(command, args, baseUrl); api.close(); - - calls = EXPECTED_CALLS; // label the closing checks - check(code === 0, `client exited 0 (got ${code})`); - check(calls === EXPECTED_CALLS, `API was called ${EXPECTED_CALLS} times`); - check(stdout.includes("All done."), "client printed the final answer"); + checkOutput(code, stdout); if (failures.length === 0) { console.error(`✓ Tool loop correct (${passed} checks)`); From a24d085bf279019744ebe598a6e63a3526553e78 Mon Sep 17 00:00:00 2001 From: a-akimov Date: Wed, 16 Sep 2026 19:29:11 +0200 Subject: [PATCH 3/3] Address comments --- mcp-client-ruby/client.rb | 8 +++++++- tests/helpers/tool-loop-test.ts | 29 ++++++++++++++++++++++++----- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/mcp-client-ruby/client.rb b/mcp-client-ruby/client.rb index db86715f..f14d81be 100644 --- a/mcp-client-ruby/client.rb +++ b/mcp-client-ruby/client.rb @@ -146,7 +146,9 @@ def process_query(query) final_text.join("\n") end - # Convert a response's content blocks back into request parameters. + # Convert a response's content blocks back into request parameters. Thinking + # blocks must go back unchanged, signature included, or the API rejects the + # follow-up request. def assistant_content(response) response.content.filter_map do |block| case block @@ -154,6 +156,10 @@ def assistant_content(response) { type: "text", text: block.text } when Anthropic::Models::ToolUseBlock { type: "tool_use", id: block.id, name: block.name, input: block.input } + when Anthropic::Models::ThinkingBlock + { type: "thinking", thinking: block.thinking, signature: block.signature } + when Anthropic::Models::RedactedThinkingBlock + { type: "redacted_thinking", data: block.data } end end end diff --git a/tests/helpers/tool-loop-test.ts b/tests/helpers/tool-loop-test.ts index 336520c1..6cdd761a 100644 --- a/tests/helpers/tool-loop-test.ts +++ b/tests/helpers/tool-loop-test.ts @@ -104,6 +104,13 @@ function scenarioOf(messages: Message[]): Scenario | undefined { return undefined; } +const THINKING: Block = { + type: "thinking", + thinking: "Two lookups are needed.", + signature: "test-signature", +}; +const REDACTED_THINKING: Block = { type: "redacted_thinking", data: "test-redacted-data" }; + function toolUse(id: string, name: string, input: Record): Block { return { type: "tool_use", id, name, input }; } @@ -121,7 +128,10 @@ function respondParallel(n: number, request: Request): Block[] { if (n === 1) { check(messages.length === 1 && last.role === "user", `${label}: history is the user query`); + // Thinking comes before the tool calls, as it does from the real model. return [ + THINKING, + REDACTED_THINKING, text("Checking two things."), toolUse("tu_1", "get_alerts", { state: "CA" }), toolUse("tu_2", "get_forecast", { latitude: 38.58, longitude: -121.49 }), @@ -131,6 +141,19 @@ function respondParallel(n: number, request: Request): Block[] { if (n === 2) { check(messages.length === 3, `${label}: history has 3 messages (got ${messages.length})`); check(messages[1]?.role === "assistant", `${label}: assistant turn is kept in history`); + // The API rejects a follow-up whose assistant turn lost its thinking + // blocks, so they must be echoed back unchanged. + const echoed = Array.isArray(messages[1]?.content) ? messages[1].content : []; + const thinking = echoed.find((block) => block.type === "thinking"); + const redacted = echoed.find((block) => block.type === "redacted_thinking"); + check( + thinking?.thinking === THINKING.thinking && thinking?.signature === THINKING.signature, + `${label}: thinking block is passed back with its signature`, + ); + check( + redacted?.data === REDACTED_THINKING.data, + `${label}: redacted thinking block is passed back`, + ); check( last.role === "user" && results.length === 2, `${label}: both tool_results arrive in one user message (got ${results.length})`, @@ -154,11 +177,7 @@ function respondParallel(n: number, request: Request): Block[] { results.length === 1 && results[0].tool_use_id === "tu_3", `${label}: second-step tool_result is present`, ); - // Rust's genai crate has no is_error field, so that client marks the - // text instead; accept either. - const flagged = results[0]?.is_error === true; - const marked = textOf(results[0]?.content).startsWith("Error:"); - check(flagged || marked, `${label}: error result is forwarded as an error`); + check(results[0]?.is_error === true, `${label}: error result is forwarded as is_error`); return [text(FINAL_ANSWER.parallel)]; }