Skip to content

Fix issue 28 with tool use and improve the logic overall#141

Open
a-akimov wants to merge 2 commits into
modelcontextprotocol:mainfrom
a-akimov:fix-issue28-tool-use
Open

Fix issue 28 with tool use and improve the logic overall#141
a-akimov wants to merge 2 commits into
modelcontextprotocol:mainfrom
a-akimov:fix-issue28-tool-use

Conversation

@a-akimov
Copy link
Copy Markdown
Contributor

Motivation and Context

  • Fix process_query in mcp-client-python/client.py so tool-use turns conform to the Anthropic Messages API: append the assistant's full content (preserving the tool_use block) and reply with a proper tool_result block carrying tool_use_id, replacing the previous plain-user-message workaround and the dead hasattr(content, "text") check that the API spec confirms can never be true.
  • Restructure the response handler into a turn-level loop so the client correctly handles parallel tool calls in a single response (executing all tool_use blocks and returning all matching tool_result blocks in one user message) and chained tool calls across turns (passing tools on every follow-up request so the model can keep using them).
  • Add MAX_TOOL_TURNS = 10 cap to prevent unbounded tool-use loops; on cap-hit the client appends a [Stopped after N tool-use turns] notice and returns the accumulated output.
  • Fix chat_loop to exit cleanly on EOFError / KeyboardInterrupt instead of being swallowed by a broad except Exception and respinning forever. This also makes the python client smoke test pass reliably under non-tty stdin regardless of whether ANTHROPIC_API_KEY is set (previously the test hung whenever the key was loaded from .env).

How Has This Been Tested?

  • tests/smoke-test.sh passes locally (all 5 tests green, no hangs) both with and without ANTHROPIC_API_KEY set.
  • Manual: run client.py against weather-server-python with a real ANTHROPIC_API_KEY and ask a query that triggers a single tool call (e.g. "What's
    the weather in NYC?") — verify the model receives the tool result and produces a final answer.
  • Manual: ask a query that triggers multiple sequential tool calls (e.g. "Compare the weather in NYC and SF") — verify all calls execute and the final answer references both results.
  • Manual: verify the MAX_TOOL_TURNS cap path by lowering the constant locally and confirming the [Stopped after N tool-use turns] notice appears.
  • Manual: press Ctrl+D or Ctrl+C at the Query: prompt — verify the client exits cleanly instead of spinning.

Breaking Changes

N/a

Types of changes

  • 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)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Fixes issue #28.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant