镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

fix(mcp): do not resend a tool call after a timeout or transport error - #8933

Open
Ray (RaycarlLei) wants to merge 1 commit into
crewAIInc:mainfrom
RaycarlLei:fix/mcp-no-tool-call-replay
Open

Ray (RaycarlLei) wants to merge 1 commit into
crewAIInc:mainfrom
RaycarlLei:fix/mcp-no-tool-call-replay

Conversation

@RaycarlLei

@RaycarlLei Ray (RaycarlLei) commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Related issue

Fixes #8932

Summary

A tools/call that timed out or hit a transport error after it was sent was retried up to 3 times. The server may already have run the tool, so one agent action could run a non-idempotent MCP tool three times.

  • MCPClient.call_tool_result now calls _retry_operation with max_attempts=1. _retry_operation gains that optional parameter (default max_retries), so list_tools, list_prompts and get_prompt keep their retries.
  • MCPToolWrapper._execute_tool sets a call_sent flag right before session.call_tool. Any error after that (timeout, cancellation, transport error) raises a private _ToolCallInterruptedError, which _execute_single_attempt reports without retrying. Failures while connecting or initializing, before the call is sent, are retried as before.

The agent now gets: MCP tool 'send_email' timed out after the call was sent. It was not retried because the server may already have run it.

Verification

  • Tests added or updated for the changed behavior
  • Relevant tests and quality checks pass locally

New lib/crewai/tests/mcp/test_tool_call_not_replayed.py:

test main this PR
client: timeout → call_tool awaited once fails (3) passes
client: transport error → awaited once fails (3) passes
client: list_tools still retried max_retries times passes passes
wrapper: timeout after send → awaited once fails (3) passes
wrapper: transport error after send → awaited once fails (3) passes
wrapper: connect failure still retried 3 times passes passes
uv run pytest lib/crewai/tests/mcp lib/crewai/tests/tools/test_tool_failure.py -q   # 251 passed
uv run ruff check / ruff format --check (changed files)                              # clean
uv run mypy lib/crewai/src/crewai/mcp/client.py lib/crewai/src/crewai/tools/mcp_tool_wrapper.py  # no issues

End to end against a FastMCP streamable-http server whose tool sleeps longer than the client timeout (script in the issue): on main the tool ran 3 times; with this change it runs once.

Additional context

  • This changes behavior for tools that relied on the automatic retry after a timeout. If you'd rather keep that available for tools known to be idempotent, I can add an opt-in flag instead of removing it.
  • fix: report MCP isError as ToolFailure on the external URL path #7643 touches the same wrapper functions (return types for ToolFailure); whichever lands second needs a small rebase, with no semantic conflict.
  • AI disclosure: Claude Code (Claude Opus 5.5) wrote this change and its tests and opened this PR on behalf of Ray (@RaycarlLei), who approved publishing it. Please add the llm-generated label.

🤖 Generated with Claude Code


Note

Medium Risk
Changes retry semantics for MCP tool execution (agents no longer auto-retry slow or flaky tools), which is intentional for safety but may affect workflows that depended on transparent retries.

Overview
Fixes duplicate side effects when MCP tools/call fails after the request reaches the server (timeout, reset, etc.). Retrying could run non-idempotent tools multiple times (e.g. send email three times).

MCPClient: call_tool_result now passes max_attempts=1 into _retry_operation, so tool execution is a single attempt. _retry_operation accepts optional max_attempts (default still max_retries); list_tools, list_prompts, and get_prompt keep multi-attempt retries.

MCPToolWrapper: A call_sent flag is set immediately before session.call_tool. Failures after that raise _ToolCallInterruptedError, surfaced to the agent with a clear message and no retry. Connection/setup failures before the call still use the existing 3-attempt backoff.

New tests in test_tool_call_not_replayed.py lock in single-send behavior for client and wrapper while preserving retries for discovery and pre-send connect errors.

Reviewed by Cursor Bugbot for commit e1d9830. Bugbot is set up for automated code reviews on this repo. Configure here.

MCPClient.call_tool_result and MCPToolWrapper retried tools/call up to
three times on timeouts and transport errors. The server may already have
run the tool by then, so a slow non-idempotent tool (send an email, place
an order) could run three times for one agent action.

Send tools/call once. MCPToolWrapper still retries when connecting or
initializing fails, before the call is sent, and tool discovery keeps its
retries.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@strix-security

strix-security Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Strix Security Review

✅ No open security findings.

PR summary: Fixes an MCP reliability/safety bug where a `tools/call` that timed out or hit a transport error after being sent was retried up to three times, so a non-idempotent MCP tool could run three times for one agent action. The client now makes a single tool-call attempt, and the tool wrapper records that the request was sent and reports a non-retryable "interrupted" error instead of retrying.

Review summary

The change stops an MCP tools/call from being replayed: MCPClient.call_tool_result now makes a single attempt, and MCPToolWrapper marks call_sent before dispatching so any post-send failure becomes a private non-retryable error instead of re-entering the retry loop, while discovery operations and pre-send connect failures keep their retries. No vulnerability was confirmed. The review examined the exception and cancellation semantics of the new handler, whether every send path is actually covered, and whether the new error reporting can invert a failure into a success; the residual gaps found (re-invocation by the framework-level tool retry layer on the native MCP path, plain-string error reporting that failure policies ignore, the wrapper not consulting isError) are all present on the base branch and are unchanged by this diff, which strictly reduces the number of times a side-effecting tool can execute.

Ruled out (7)
  • Does the new catch tuple let a cancellation or error-group escape and leave a sent tool call unguarded or unreported?
    The tuple is a strict superset of what the base branch caught, and anyio excludes CancelledError from TaskGroup error collection, so the group raised is an ExceptionGroup that the handler catches.
  • Can the outer 60-second wait_for fire above the call_sent guard and cause the tool call to be sent a second time?
    Both timeouts use the same 60-second deadline, so the outer timer cancels the inner await inside `_execute_tool`'s try block where the guard converts it to a non-retryable error; 18,000 jittered trials across Python 3.11 and 3.14 produced no replay.
  • Can an MCP tool still be executed more than once because the framework re-invokes it after a failure?
    The re-invocation exists on the native MCP path through the unchanged tool-usage retry loop, but that code is untouched by this diff and the change strictly reduces executions (from up to nine to up to three).
  • Does an interrupted call get reported and accounted as a successful tool execution?
    The wrapper has always returned errors as plain strings rather than structured failures, so failure-policy behaviour is unchanged; the new message explicitly states the outcome is unknown instead of implying completion.
  • Can a completed tool call be lost or misreported because `call_sent` is set before the request is transmitted?
    The reported wording is conservative, never claiming success or that the tool did not run, and the previous behaviour for the same failure was a retried call with no duplicate-execution warning at all.
  • Does `attempts = max_attempts or self.max_retries` let a caller who passes 0 still retry a tool call?
    The only call site passes 1, `max_retries` is a module constant never overridden by any configuration path, and the zero-attempt fall-through predates this diff and raises rather than silently succeeding.
  • Does the wrapper surface a server-reported `isError` result as a success?
    The success-return path is byte-identical to the base branch and the diff does not modify it, so the omission of the `isError` check is pre-existing rather than introduced here.

Review 1 on this PR · e1d9830 · requested Nov 2 05:48 UTC when the PR was opened


Reviewed by Strix
Re-run review · Configure security review settings

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] MCP tool calls are sent again after a timeout, so one action can run a tool up to 3 times

1 participant