Python: Fix: Prevent duplicate MCP tools and prompts (#1876) - #1890
Merged
moonbox3 merged 13 commits intoNov 14, 2025
Conversation
ishanrajsingh
changed the title
github-actions
Bot
changed the title
ishanrajsingh
commented
Nov 4, 2025
ishanrajsingh
left a comment
ishanrajsingh
left a comment
Contributor
Author
There was a problem hiding this comment.
Address review feedback: Prevent multiple calls to load_tools and load_prompts
Contributor
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
ishanrajsingh
commented
Nov 5, 2025
Contributor
Author
There was a problem hiding this comment.
Address review feedback from @eavanvalkenburg : Move flag checks to connect() and remove comment
ishanrajsingh
commented
Nov 5, 2025
Contributor
Author
There was a problem hiding this comment.
The mentioned changes have been comitted and addresses review feedback from @eavanvalkenburg
eavanvalkenburg
approved these changes
Nov 6, 2025
eavanvalkenburg
enabled auto-merge
November 6, 2025 16:04
Member
|
@ishanrajsingh there are some failures in the CI, please rebase from main and then we'll run everything again, thanks! |
- Added deduplication logic in MCPTool.load_tools() method - Added deduplication logic in MCPTool.load_prompts() method - Track existing function names before loading from MCP server - Skip tools/prompts that are already registered in _functions list - Prevents 400 error from Azure AI Foundry caused by duplicate tool names The issue occurred because load_tools() was being called multiple times (during connect() and by notification handlers), causing tools to be appended without duplicate checking. Changes made: 1. In load_tools(): Added existing_names set to track registered functions 2. In load_tools(): Added check to skip tools already in existing_names 3. In load_prompts(): Applied same deduplication pattern Testing: - Created unit test verifying deduplication logic - Confirmed duplicates are skipped correctly - Confirmed new functions are added correctly - Prevents duplicate tool names being sent to LLM Fixes microsoft#1876
…d_prompts - Added _tools_loaded and _prompts_loaded flags to MCPTool class - Modified load_tools() to check if already loaded and return early - Modified load_prompts() to check if already loaded and return early - Moved test cases from test_mcp_fix.py to test_mcp.py - Added tests for multiple call prevention - Deleted separate test_mcp_fix.py file Addresses review feedback from @eavanvalkenburg: - Prevents accidental multiple calls to load_tools() - Prevents accidental multiple calls to load_prompts() - Test file now in proper location (test_mcp.py)
…ments - Removed verbose comments from code - Moved _tools_loaded and _prompts_loaded checks to connect() method - Allows manual calls to load_tools() and load_prompts() for updates - Updated tests to reflect new behavior - connect() now prevents duplicate loading during connection - Users can still manually call load_tools()/load_prompts() to refresh Addresses feedback from @eavanvalkenburg
auto-merge was automatically disabled
November 6, 2025 16:18
Head branch was pushed to by a user without write access
ishanrajsingh
force-pushed
the
fix/mcp-duplicate-tools-1876
branch
from
November 6, 2025 16:18
da5de39 to
b02a941
Compare
- Applied black formatting - Fixed ruff linting issues - All tests passing locally
Member
|
you need to rerun |
Contributor
Author
|
I've re-run |
- Consolidate multi-line type annotations to single line - Remove unnecessary parentheses - Apply ruff format and security checks
Contributor
Author
|
I have run the pre commit checks locally and they have passed. I think the CI checks should pass now and there should not be any failure @eavanvalkenburg . |
TaoChenOSU
approved these changes
Nov 13, 2025
arisng
pushed a commit
to arisng/agent-framework
that referenced
this pull request
Feb 2, 2026
…microsoft#1890) * Fix: Prevent duplicate MCP tools and prompts (microsoft#1876) - Added deduplication logic in MCPTool.load_tools() method - Added deduplication logic in MCPTool.load_prompts() method - Track existing function names before loading from MCP server - Skip tools/prompts that are already registered in _functions list - Prevents 400 error from Azure AI Foundry caused by duplicate tool names The issue occurred because load_tools() was being called multiple times (during connect() and by notification handlers), causing tools to be appended without duplicate checking. Changes made: 1. In load_tools(): Added existing_names set to track registered functions 2. In load_tools(): Added check to skip tools already in existing_names 3. In load_prompts(): Applied same deduplication pattern Testing: - Created unit test verifying deduplication logic - Confirmed duplicates are skipped correctly - Confirmed new functions are added correctly - Prevents duplicate tool names being sent to LLM Fixes microsoft#1876 * Address review feedback: Prevent multiple calls to load_tools and load_prompts - Added _tools_loaded and _prompts_loaded flags to MCPTool class - Modified load_tools() to check if already loaded and return early - Modified load_prompts() to check if already loaded and return early - Moved test cases from test_mcp_fix.py to test_mcp.py - Added tests for multiple call prevention - Deleted separate test_mcp_fix.py file Addresses review feedback from @eavanvalkenburg: - Prevents accidental multiple calls to load_tools() - Prevents accidental multiple calls to load_prompts() - Test file now in proper location (test_mcp.py) * Address review feedback: Move flag checks to connect() and remove comments - Removed verbose comments from code - Moved _tools_loaded and _prompts_loaded checks to connect() method - Allows manual calls to load_tools() and load_prompts() for updates - Updated tests to reflect new behavior - connect() now prevents duplicate loading during connection - Users can still manually call load_tools()/load_prompts() to refresh Addresses feedback from @eavanvalkenburg * Fix: Code quality and formatting issues - Applied black formatting - Fixed ruff linting issues - All tests passing locally * chore: Re-run uv lock per review request * Apply pre-commit formatting: consolidate type annotations - Consolidate multi-line type annotations to single line - Remove unnecessary parentheses - Apply ruff format and security checks
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
5 participants
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.
The issue occurred because load_tools() was being called multiple times (during connect() and by notification handlers), causing tools to be appended without duplicate checking.
Changes made:
Testing:
Fixes #1876