Fix the tool-use loop in the Go, Rust, and Ruby clients and add a tool-loop smoke test (#200)
Some checks failed
Smoke Tests / test (push) Has been cancelled
Some checks failed
Smoke Tests / test (push) Has been cancelled
* 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. * Improvements * Address comments
This commit is contained in:
parent
3844c5b45e
commit
470d1ba328
1 changed files with 695 additions and 140 deletions
|
|
@ -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,54 @@ 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::<Vec<_>>()
|
||||
.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 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());
|
||||
}
|
||||
if !chat_rsp.tool_calls().is_empty() {
|
||||
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<ChatResponse> {
|
||||
/// 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<ChatResponse> {
|
||||
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")?;
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue