Repository navigation
Python: fix(python): prevent duplicate MCP tool execution after ambiguous connection loss - #9247
Open
Suhas Aggarwal (suhasagg) wants to merge 3 commits into
Open
Suhas Aggarwal (suhasagg) wants to merge 3 commits into
Suhas Aggarwal (suhasagg) wants to merge 3 commits into
Conversation
Suhas Aggarwal (suhasagg)
requested review from
SergeyMenshykh,
Tao Chen (TaoChenOSU),
Eduard van Valkenburg (eavanvalkenburg),
Giles Odigwe (giles17),
Jose Alvarez (jpalvarezl),
Evan Mattson (moonbox3),
Roger Barreto (rogerbarreto),
sophia-ramsey and
westey (westey-m)
as code owners
October 10, 2026 10:49
Suhas Aggarwal (suhasagg)
deployed
to
github-app-auth
October 10, 2026 10:49 — with
GitHub Actions
Active
Suhas Aggarwal (suhasagg)
deployed
to
github-app-auth
October 10, 2026 10:49 — with
GitHub Actions
Active
Copilot started reviewing on behalf of
Suhas Aggarwal (suhasagg)
October 10, 2026 10:49
View session
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The safety behavior and regression coverage are sound; only a non-blocking stale test docstring remains.
1 open finding
What changed in this PR
Prevents duplicate MCP tool execution after ambiguous connection loss by removing automatic replay.
Changes:
- Replaces retrying tool calls with single-attempt execution.
- Reports unknown outcomes while preserving underlying exceptions and telemetry.
- Adds regression coverage for connection loss and advisory annotations.
| File | Description |
|---|---|
python/packages/core/agent_framework/_mcp.py |
Implements no-replay behavior and unknown-outcome errors. |
python/packages/core/tests/core/test_mcp.py |
Updates recovery expectations and adds regression tests. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Suhas Aggarwal (suhasagg)
deployed
to
github-app-auth
October 10, 2026 11:35 — with
GitHub Actions
Active
Suhas Aggarwal (suhasagg)
deployed
to
github-app-auth
October 11, 2026 05:58 — with
GitHub Actions
Active
Suhas Aggarwal (suhasagg)
deployed
to
github-app-auth
October 11, 2026 05:59 — with
GitHub Actions
Active
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Motivation & Context
MCP
tools/callrequests may produce non-idempotent side effects, such as creating resources, modifying persistent state, triggering external workflows, or initiating transactions.The existing Python MCP tool implementation can automatically retry a failed
tools/callafter certain connection failures. However, a connection failure does not establish whether the remote MCP server executed the original request.For example:
tools/callrequest.This creates a potential duplicate-execution and data-integrity risk for state-changing MCP tools.
The fundamental problem is that a client cannot reliably distinguish between:
The safe default is therefore to avoid automatically replaying an operation whose execution outcome is unknown.
MCP annotations such as
idempotentHintandreadOnlyHintare advisory metadata supplied by the remote server. They are not a reliable guarantee of replay safety, particularly when tools are misconfigured or implemented by third parties.This PR addresses the duplicate-execution risk described in #9204 by removing automatic replay of ambiguous MCP tool calls and surfacing an explicit unknown-outcome error to the caller.
Description & Review Guide
What are the major changes?
1. Replace automatic tool-call retries with single-attempt execution
_call_tool_with_retries()to_call_tool_once().ClientSession.call_tool().call_tool()attempt.This establishes a conservative no-replay policy for MCP tool execution.
2. Explicitly report ambiguous execution outcomes
For connection failures represented by
ClosedResourceErroror MCP session-termination errors:tools/call.ToolExecutionExceptionexplaining that the remote execution outcome is unknown.inner_exceptionand exception chaining.The error intentionally does not claim that the remote operation failed, because the server may already have completed it.
3. Preserve ordinary MCP error handling
isError=True.4. Add regression tests for ambiguous connection failures
The regression coverage verifies that:
call_tool()invocation.5. Verify advisory MCP annotations cannot authorize replay
Add parameterized tests covering discovered MCP tools advertising:
idempotentHint=TruereadOnlyHint=TrueThe tests use MCP tool discovery through
list_tools()and verify that an ambiguous connection failure does not cause the remote operation to be invoked again.These tests deliberately treat advisory annotations as insufficient evidence of replay safety.
6. Update connection-reset integration expectations
Update the existing connection-reset integration test to:
What is the impact of these changes?
Safety improvement
The framework no longer automatically reissues an ambiguous MCP
tools/callrequest through the removed retry mechanism.This reduces the risk of duplicate side effects for tools that modify external state.
Explicit failure semantics
Callers receive an exception indicating that the remote execution outcome is unknown, rather than receiving an implicit retry that may duplicate the operation.
Behavioral compatibility
This intentionally changes the previous automatic-retry behavior for affected MCP tool calls.
Applications relying on automatic replay after connection loss may need to handle the exception and explicitly reconnect before a subsequent independent operation.
Recovery scope
This PR does not implement automatic session recovery after an ambiguous tool-call failure.
The existing explicit
connect(reset=True)mechanism remains available for reconnecting before a separate operation.Framework-owned session recovery, caller-managed session boundaries, and concurrent in-flight operations require careful lifecycle handling. Those concerns are not claimed as solved by this PR.
The implementation focuses on establishing a safe no-replay baseline without introducing additional reconnection races.
What do you want reviewers to focus on?
I would particularly appreciate maintainer feedback on the following design decisions:
1. Conservative no-replay policy
Is removing automatic replay from
tools/callthe preferred baseline when the client cannot determine whether the remote server executed the request?2. Treatment of MCP tool annotations
The implementation deliberately does not use
idempotentHintorreadOnlyHintto authorize replay after an ambiguous connection failure.This avoids relying on remote advisory metadata as a guarantee of execution safety.
3. Unknown-outcome error semantics
Does
ToolExecutionExceptionwith an explicit unknown-outcome message and preserved underlying exception provide an appropriate caller-facing contract?4. Recovery separation
Would maintainers prefer to keep safe recovery of framework-owned sessions for subsequent independent calls as a follow-up, or include that functionality in this PR?
Any such recovery should avoid replaying the original failed operation and respect caller-managed session ownership and concurrent operations.
5. Compatibility and test coverage
Please review the change in retry behavior and whether additional transport-failure or lifecycle tests would be useful before merging.
Related Issue
Fixes #9204
This PR provides an implementation focused on a strict no-replay policy for ambiguous MCP tool-call failures.
The key distinction is that this implementation does not authorize automatic replay based on advisory
idempotentHintorreadOnlyHintvalues, including when those hints are advertised during MCP tool discovery.The intent is to provide a conservative safety baseline rather than to duplicate or supersede another contributor's work without review.
I welcome maintainer guidance on which approach best fits the framework's desired execution semantics, including whether elements of the approaches should be combined.
Validation
Full MCP test suite
Executed from the
pythondirectory:Results:
Targeted regression tests
uv run --group dev pytest packages/core/tests/core/test_mcp.py \ -k "untrusted_annotations_never_authorize_replay or does_not_retry_after_connection_loss" -vResults:
Lint and formatting
All checks passed.
The full MCP suite reported warnings, including experimental/deprecated API warnings and a coroutine warning in an existing test. No test failures occurred.
AI Assistance
AI assistance details:
AI assistance was used for implementation, tests. All changes were reviewed and tested locally.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.