fix(mcp): resolve toolset tools by the server's known prefix - #31254
Conversation
Greptile SummaryThis PR fixes toolset tool filtering for MCP servers whose prefix contains a hyphen — such as a hyphenated alias or a UUID
Confidence Score: 5/5Safe to merge — the fix is narrowly scoped to prefix stripping, storage format is unchanged, and the fallback to legacy behavior when the server is not found in the registry preserves existing behavior for any unregistered server. The change replaces a single heuristic split with an exact-prefix lookup keyed on the server object the caller already holds. The new helper is fully invertible with No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/_experimental/mcp_server/utils.py | Adds strip_known_server_prefix that iterates all known server prefixes via iter_known_server_prefixes and strips the exact {prefix}{separator} match — correctly handling hyphenated aliases and UUID server_id fallback prefixes where the old first-separator split failed. |
| litellm/proxy/_experimental/mcp_server/server.py | Replaces split_server_prefix_from_name with strip_known_server_prefix in filter_tools_by_key_team_permissions; also slightly refactors the function to use an early return for the no-restriction case. Change is correct and the fallback when get_mcp_server_by_id returns None is safe. |
| litellm/proxy/_experimental/mcp_server/mcp_server_manager.py | Switches resolve_toolset_tool_permissions to use strip_known_server_prefix; get_mcp_server_by_id is an in-memory registry lookup, and results are cached after the first resolution so the per-tool call overhead is negligible. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_short_mcp_tool_prefix.py | Adds TestStripKnownServerPrefix with direct unit tests using real MCPServer objects (not SimpleNamespace). The _reset_env autouse fixture ensures short-prefix mode is off, so hyphenated-alias and UUID-fallback cases are faithfully reproduced regardless of environment. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_toolset_scope.py | Adds TestToolsetPrefixResolution regression tests using SimpleNamespace with short_prefix=None. These correctly reproduce the failing cases when LITELLM_USE_SHORT_MCP_TOOL_PREFIX is unset (default), but if that env var is set in CI the SimpleNamespace will compute a clean 3-char short prefix and the hyphenated/UUID cases won't exercise the real bug path — the direct unit tests in test_short_mcp_tool_prefix.py close this gap. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server.py | Adds short_prefix=None to existing server mocks to align with the new prefix resolution path, and corrects server.alias from "gitmcp" to "GITMCP" to match mock tool names — necessary because strip_known_server_prefix is case-sensitive where the old first-separator split was not. |
| ui/litellm-dashboard/src/components/mcp_tools/MCPToolsetsTab.tsx | Display-only: renders tool chips as {server-prefix}-{tool} so the same bare tool name on different servers is distinguishable. serverPrefixById memos are independent between MCPToolsetsTab and CreateToolsetModal but both consume the same React Query cache entry so no extra network calls occur. |
Reviews (5): Last reviewed commit: "test(mcp): add focused unit tests for st..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryFixes a toolset tool-filtering bug where tools on MCP servers whose prefix contains the separator character (
Confidence Score: 4/5The production fix is correct and well-scoped; the only concern is that the new regression tests don't actually exercise the hyphenated-alias and UUID-prefix failure paths they document. The core logic change — replacing first-separator guessing with prefix-aware stripping — is sound and handles all described edge cases correctly. The graceful tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_toolset_scope.py — the
|
| Filename | Overview |
|---|---|
| litellm/proxy/_experimental/mcp_server/utils.py | Adds strip_known_server_prefix, which iterates all known prefixes via iter_known_server_prefixes and uses startswith to strip exactly {normalized_prefix}{separator}. Logic is correct and consistent with how add_server_prefix_to_name builds names. |
| litellm/proxy/_experimental/mcp_server/server.py | Replaces the first-separator split in filter_tools_by_key_team_permissions with strip_known_server_prefix; early-returns on None permissions. Change is correct and equivalent for clean-prefix servers, superior for hyphenated/UUID prefix servers. |
| litellm/proxy/_experimental/mcp_server/mcp_server_manager.py | Replaces the first-separator split in resolve_toolset_tool_permissions with strip_known_server_prefix; gracefully falls back to legacy behaviour when server lookup returns None. Change is correct. |
| tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_toolset_scope.py | Adds TestToolsetPrefixResolution with parametrized cases for clean alias, hyphenated alias, and UUID server_id. The server factory sets short_prefix=None, causing all three cases to use a 3-char computed prefix (no hyphens), so the pre-fix code also passes all three cases — the tests don't guard against regression. |
| ui/litellm-dashboard/src/components/mcp_tools/MCPToolsetsTab.tsx | Display-only change: adds displayToolName helper and serverPrefixById map so tool chips show {server-prefix}-{tool} in both the toolset list column and the create/edit modal. No functional impact on stored data. |
Reviews (2): Last reviewed commit: "fix(mcp): resolve toolset tools by the s..." | Re-trigger Greptile
df2ea64 to
054fe90
Compare
054fe90 to
0f58194
Compare
Toolsets store {server_id, bare tool_name} and reconcile that against the
live prefixed tool name at list time. The reconciliation chopped the live
name at the first MCP_TOOL_PREFIX_SEPARATOR with no server context, so a
server whose prefix contains the separator (a hyphenated alias, or the
UUID server_id used as the prefix when a server has no alias) had its
tools silently dropped from /toolset/<name>/mcp while listing fine
everywhere else. Strip the exact known prefix for the tool's server_id
instead of guessing the boundary, on both the resolve and filter sides
Also render toolset tools as {server-prefix}-{tool} in the dashboard
picker result and chips; this is display only, the persisted record
stays {server_id, bare tool_name}
Resolves LIT-3419
0f58194 to
1daaa2c
Compare
|
Addressed the one concern from the review: the dashboard |
Cover the LIT-3419 cases directly on the helper with real MCPServer objects: clean prefix round-trip, hyphenated alias, UUID server_id fallback, unprefixed passthrough, and the server=None legacy fallback
|
Pushed a follow-up commit with direct unit tests for the new helper. @greptileai |
…#31254) * fix(mcp): resolve toolset tools by the server's known prefix Toolsets store {server_id, bare tool_name} and reconcile that against the live prefixed tool name at list time. The reconciliation chopped the live name at the first MCP_TOOL_PREFIX_SEPARATOR with no server context, so a server whose prefix contains the separator (a hyphenated alias, or the UUID server_id used as the prefix when a server has no alias) had its tools silently dropped from /toolset/<name>/mcp while listing fine everywhere else. Strip the exact known prefix for the tool's server_id instead of guessing the boundary, on both the resolve and filter sides Also render toolset tools as {server-prefix}-{tool} in the dashboard picker result and chips; this is display only, the persisted record stays {server_id, bare tool_name} Resolves LIT-3419 * test(mcp): add focused unit tests for strip_known_server_prefix Cover the LIT-3419 cases directly on the helper with real MCPServer objects: clean prefix round-trip, hyphenated alias, UUID server_id fallback, unprefixed passthrough, and the server=None legacy fallback
Relevant issues
Linear ticket
Resolves LIT-3419
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
make test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewCI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
Toolsets persist each tool as
{server_id, bare tool_name}. At list time the bare name is reconciled against the live tool name, which the gateway serves prefixed as{server-prefix}-{tool}. The old reconciliation chopped the live name at the first-, so a server whose prefix contains a-had its tools silently dropped from the toolset. A server gets such a prefix when it has no alias and falls back to its UUIDserver_id, or when it carries a legacy hyphenated alias. Servers with a clean prefix (no separator) were unaffected, which is why some toolsets worked and others returned nothing.Reproduced against a live proxy on
localhost:4099backed by real Postgres, talking to the public DeepWiki MCP server. The server is registered with no alias, so its prefix is its hyphenated UUID.Setup (identical for both runs):
Listing tools through the toolset URL, before and after the change. The global
/mcp/count stays at 3 in both runs, so the server and its tools are present throughout; only the toolset filter differs.Before (current
litellm_internal_staging):After (this PR):
The dashboard change is display only. On the MCP Toolsets tab the "Your Toolset" chips and the toolset list "Tools" column now render each tool as
{server-prefix}-{tool}(for exampledeepwiki-read_wiki_contents) so the same tool name on different servers is distinguishable; the persisted record is unchanged. To see it, openhttp://localhost:4000/ui/?page=mcp-servers, go to the Toolsets tab, create or edit a toolset, and pick a couple of tools.Type
🐛 Bug Fix
Changes
The reconciliation between a toolset's stored bare tool name and the live prefixed tool name used
split_server_prefix_from_name, which removes everything up to the firstMCP_TOOL_PREFIX_SEPARATORwith no knowledge of the server. When the server's prefix itself contains the separator the cut lands inside the prefix, so the stripped name never matches the stored bare name and the tool is filtered out.The toolset already stores the
server_id, so the prefix is knowable. This addsstrip_known_server_prefix(name, server)inutils.py, which removes exactly{known_prefix}{separator}for one of the server's registered prefixes (viaiter_known_server_prefixes) and leaves the name untouched when none match.filter_tools_by_key_team_permissions(the list/filter side) andresolve_toolset_tool_permissions(the stored-name side) both use it now, keyed by theserver_idthey already hold, so the two sides reduce to the same true bare name for hyphenated aliases, UUID prefixes, and the short-prefix mode alike. Storage format is unchanged and nothing is prefixed on write.The same first-separator weakness exists in the server-level
allowed_tools/ display-override paths (_tool_name_matches,apply_tool_overrides); those are a separate feature gated on optional per-server config and are left for a follow-up to keep this change scoped to the toolset bug.Tests extend
test_mcp_toolset_scope.pywith a parametrized regression over a clean alias, a hyphenated alias, and a no-alias UUID prefix, asserting the granted tools survive the filter and that resolution reduces an already-prefixed stored name back to bare. The clean-alias case passes on the old code; the hyphenated and UUID cases fail before this change and pass after.A follow-up commit adds focused unit tests for
strip_known_server_prefixintest_short_mcp_tool_prefix.py, exercising the clean, hyphenated-alias, and UUID server_id prefixes plus the unprefixed-passthrough andserver=Nonefallback paths directly on the helper with realMCPServerobjects