fix(mcp): preserve non-MCP tools in semantic filter - #24986
Conversation
When the semantic tool filter finds no matches for a user query, it previously returned all available tools. With many MCP servers this can exceed provider tool limits. Now it drops MCP tools (prefixed) and returns only non-MCP tools on zero matches.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Congrats! CodSpeed is installed 🎉
You will start to see performance impacts in the reports once the benchmarks are run from your default branch.
|
Greptile SummaryThis PR fixes the MCP semantic filter hook to preserve non-MCP tools by introducing a single-pass partition that routes only MCP tools through the semantic router while passing all others through untouched. It also adds Confidence Score: 5/5Safe to merge; no P0 or P1 issues found. The behavior changes are deliberate and correctly handled. All logic is correct, edge cases (empty registry, all-MCP zero-match, client-prefixed names, hyphenated non-MCP tools) are covered by dedicated tests. Only P2 style suggestions remain. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/hooks/mcp_semantic_filter/hook.py | Adds _partition_mcp_tools to split tools using the live registry before semantic filtering; non-MCP tools bypass the filter and are re-appended; all-MCP zero-match falls back to top_k. Logic is correct and well-tested. |
| litellm/proxy/_experimental/mcp_server/semantic_tool_filter.py | Returns [] on zero matches (hook handles non-MCP passthrough), adds _name_matches_canonical for client-prefix suffix matching in _get_tools_by_names. Behavior change is deliberate and handled correctly upstream; docstring update missing. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_semantic_tool_filter.py | Adds autouse fixture to register fake "server" MCP prefix; adds regression tests for non-MCP passthrough, zero-match fallback, and _get_tools_by_names suffix matching. One assertion uses the legacy hyphen heuristic instead of the strict classifier. |
| tests/mcp_tests/test_semantic_tool_filter_e2e.py | Monkeypatches 10 fake server prefixes into the registry and renames tool names to match those prefixes so the hook's strict classifier treats them as MCP. Changes are consistent with the new partitioning logic. |
| litellm/proxy/_experimental/mcp_server/utils.py | is_tool_name_prefixed extended with optional known_server_prefixes for strict registry-based classification; legacy heuristic preserved as fallback. Pre-existing; no changes in this PR. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[async_pre_call_hook] --> B{tools present?}
B -- No --> C[return None]
B -- Yes --> D[_partition_mcp_tools builds known_prefixes from registry]
D --> E{mcp_tools empty?}
E -- Yes --> F[return None - non-MCP tools pass through untouched]
E -- No --> G[filter_tools with mcp_tools only]
G --> H{filtered_mcp_tools empty?}
H -- Yes + non_mcp present --> J[filtered = non_mcp_tools only]
H -- Yes + no non_mcp --> I[fallback: mcp_tools top_k]
H -- No --> K[filtered = filtered_mcp + non_mcp_tools]
I --> L[data tools updated]
J --> L
K --> L
Reviews (9): Last reviewed commit: "refactor(mcp-semantic-filter): extract _..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
The semantic filter treated all tools equally — it didn't distinguish between MCP tools (which it should filter) and non-MCP built-in tools (which should pass through untouched). This caused two failures: - On match: non-MCP tools were dropped, losing built-in functionality - On zero match: all tools returned unfiltered, exceeding provider limits Move MCP/non-MCP separation into the hook. The hook now passes only MCP tools (identified by server-name prefix) to filter_tools() and recombines with non-MCP tools after. filter_tools() returns empty list on zero matches instead of all tools.
klhq
changed the title
…CP tools When all tools are MCP and zero semantic matches are found, return the first top_k MCP tools instead of an empty list. Avoids sending an empty tool list to the LLM.
Agentic Code Review🚨 Needs Changes — 🚨 2 critical
|
Agentic Code Review🚨 Needs Changes — 🚨 2 critical
|
Agentic Code Review🚨 Needs Changes — 🚨 1 critical
|
Agentic Code Review🚨 Needs Changes — 🚨 1 critical
|
Rename the 10 test tool names to hyphen-prefixed form (e.g. gmail-send_email, calendar-create_event) so the hook correctly classifies them as MCP tools. Without the prefix, is_tool_name_prefixed returns False for all tools, the hook early-returns None, and the assertion `result and len(result["tools"]) < len(tools)` fails. Addresses Agentic Reviewer finding on #24986.
…lter hook
Build the known-prefix set from global_mcp_server_manager.get_registry()
and pass it to is_tool_name_prefixed() when classifying tools as MCP vs
non-MCP in the semantic filter hook.
Without this, the hook inherited the hyphen false-positive bug from the
1-line is_tool_name_prefixed() utility (fixed in the previous commit),
and would still misclassify non-MCP tools like "text-to-speech" as MCP.
Also switches the partition to a single-pass loop instead of two list
comprehensions.
Adds test_hook_passes_through_hyphenated_non_mcp_tools regression test
and an autouse fixture that registers a fake "server" MCP prefix so
existing tests using `server-tool_{i}` names continue to classify as MCP
tools with the strict check.
Addresses Agentic Reviewer P1 finding on #24986.
The hook now validates MCP tool prefixes against the live server registry (via `known_server_prefixes`). Without a mock, `get_registry()` returns an empty dict, `known_prefixes` is empty, and all tools are classified as non-MCP — the hook early-returns None and the e2e assertion fails. Register 10 fake MCP servers matching the tool prefixes (`gmail`, `calendar`, `files`, etc.) via monkeypatch so the hyphen-prefixed tool names pass the registry check and actually exercise the semantic filter.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 29203065 | Triggered | JSON Web Token | 4d30243 | tests/test_litellm/proxy/test_litellm_pre_call_utils.py | View secret |
| 29203054 | Triggered | Generic High Entropy Secret | 04b772e | tests/test_litellm/test_secret_redaction.py | View secret |
| 29203053 | Triggered | Generic Password | 4d30243 | .circleci/config.yml | View secret |
| 29203056 | Triggered | Generic High Entropy Secret | 04b772e | tests/test_litellm/test_secret_redaction.py | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
@ShashankFC — replies to your 2026-04-10 Agentic Review findings: Finding 1 — hyphen false-positive (MAJOR) Now fixed via #25085 (landed in main via #25192 on 2026-04-14). This PR's hook update uses Finding 2 — e2e test (MAJOR) Fixed in commits Finding 3 — top-k fallback (CRITICAL) False positive — the fallback is at hook.py:248-256. The bot appears to have read |
|
Re: GitGuardian alert — false positive, artifact of the
Happy to drop the merge commit and rebase onto |
# Conflicts: # tests/mcp_tests/test_semantic_tool_filter_e2e.py # tests/test_litellm/proxy/_experimental/mcp_server/test_semantic_tool_filter.py
Low: No security issues foundThis PR changes the semantic tool filter to separate MCP tools from non-MCP tools, only applying semantic filtering to MCP tools while passing non-MCP tools through untouched. The changes are internal to tool presentation logic (which tools the LLM sees in its context window) and do not affect tool execution authorization, authentication, or any external-facing attack surface. Status: 0 open Posted by Veria AI · 2026-04-23T16:55:18.320Z |
[Infra] Promote internal staging to main
|
Michael Riad Zaky seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Low: No security issues foundThis PR fixes the MCP semantic tool filter to correctly separate MCP tools from non-MCP tools before applying semantic filtering, and returns an empty list (instead of all tools) when no semantic matches are found. The changes are internal to the tool-filtering pipeline with no security-sensitive operations (no injection vectors, no auth changes, no secrets handling). User query is only used for embedding similarity computation. Status: 0 open Posted by Veria AI · 2026-04-24T05:14:16.008Z |
|
please fix the linting error @klhq
|
…R0915 async_pre_call_hook hit ruff PLR0915 (54 > 50 statements). Move the MCP/non-MCP partition block into a dedicated helper — same behavior, fewer statements in the hook entrypoint.
Low: No security issues found — PR adds substantial hardeningThis large PR includes the core MCP semantic filter fix (preserving non-MCP tools) alongside many security improvements: banning additional credential params in request bodies ( Status: 0 open Posted by Veria AI · 2026-04-24T09:28:38.218Z |
|
@krrish-berri-2 Fixed in ab64ae2. extracted the MCP/non-MCP tool partition block into a |
cd88dde to
c014bfa
Compare
6 participants
Relevant issues
Fixes #24984
Builds on #25085 (landed in
mainvia #25192), which added theoptional
known_server_prefixesparameter tois_tool_name_prefixed().Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Type
🐛 Bug Fix
Changes
The semantic filter hook treated all tools equally — it didn't distinguish between MCP tools (which it should filter) and non-MCP built-in tools (which should pass through untouched). This caused non-MCP tools to be dropped on match, and all tools to be returned on zero match (exceeding provider tool limits).
Hook behavior
hook.py) via a single-pass partition — only MCP tools go through the semantic router; non-MCP tools pass through untouchedfilter_tools()returns an empty list on zero matches instead of all tools; the hook adds non-MCP tools back afterwardsCorrect MCP classification
The hook's MCP/non-MCP split needs a reliable classifier. The legacy
is_tool_name_prefixed()did a naive"-" in tool_namecheck, which misclassifies hyphenated non-MCP tools (e.g.text-to-speech,code-review) as MCP. That bug was fixed inutils.pyby #25085 (merged tomainvia #25192 on 2026-04-14), which added an optionalknown_server_prefixesparameter for a strict check against the live registry.known_server_prefixesbuilt fromglobal_mcp_server_manager.get_registry()into the hook'sis_tool_name_prefixed()calls, so hyphenated non-MCP tools are no longer misclassifiedTests
test_semantic_tool_filter.pythat registers a fake"server"MCP prefix, so existing hook tests usingserver-tool_{i}names continue to classify as MCP under the strict checktest_hook_passes_through_hyphenated_non_mcp_tools— regression test provingtext-to-speechandcode-reviewsurvive the filter end-to-endtest_semantic_tool_filter_e2e.py: rename tool names to hyphen-prefixed MCP form andmonkeypatch10 fake servers into the registry so the test still exercises the filter with the new strict classifier