[Frontend] Add tool filtering support to ToolServer (#29224)
Signed-off-by: Daniel Salib <danielsalib@meta.com> Co-authored-by: Chauncey <chaunceyjiang@gmail.com>
This commit is contained in:
@@ -4,6 +4,9 @@
|
||||
import pytest
|
||||
import pytest_asyncio
|
||||
from openai import OpenAI
|
||||
from openai_harmony import ToolDescription, ToolNamespaceConfig
|
||||
|
||||
from vllm.entrypoints.tool_server import MCPToolServer
|
||||
|
||||
from ...utils import RemoteOpenAIServer
|
||||
|
||||
@@ -111,6 +114,48 @@ async def test_mcp_tool_env_flag_enabled(mcp_enabled_client: OpenAI, model_name:
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("model_name", [MODEL_NAME])
|
||||
async def test_mcp_tool_with_allowed_tools_star(
|
||||
mcp_enabled_client: OpenAI, model_name: str
|
||||
):
|
||||
"""Test MCP tool with allowed_tools=['*'] to select all available tools.
|
||||
|
||||
This E2E test verifies that the "*" wildcard works end-to-end.
|
||||
See test_serving_responses.py for detailed unit tests of "*" normalization.
|
||||
"""
|
||||
response = await mcp_enabled_client.responses.create(
|
||||
model=model_name,
|
||||
input=(
|
||||
"Execute the following code: "
|
||||
"import random; print(random.randint(1, 1000000))"
|
||||
),
|
||||
instructions=(
|
||||
"You must use the Python tool to execute code. Never simulate execution."
|
||||
),
|
||||
tools=[
|
||||
{
|
||||
"type": "mcp",
|
||||
"server_label": "code_interpreter",
|
||||
"server_url": "http://localhost:8888",
|
||||
# Using "*" to allow all tools from this MCP server
|
||||
"allowed_tools": ["*"],
|
||||
}
|
||||
],
|
||||
extra_body={"enable_response_messages": True},
|
||||
)
|
||||
assert response is not None
|
||||
assert response.status == "completed"
|
||||
# Verify tool calls work with allowed_tools=["*"]
|
||||
tool_call_found = False
|
||||
for message in response.output_messages:
|
||||
recipient = message.get("recipient")
|
||||
if recipient and recipient.startswith("python"):
|
||||
tool_call_found = True
|
||||
break
|
||||
assert tool_call_found, "Should have found at least one Python tool call with '*'"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("model_name", [MODEL_NAME])
|
||||
async def test_mcp_tool_env_flag_disabled(mcp_disabled_client: OpenAI, model_name: str):
|
||||
@@ -159,3 +204,58 @@ async def test_mcp_tool_env_flag_disabled(mcp_disabled_client: OpenAI, model_nam
|
||||
assert message.get("author").get("role") != "developer", (
|
||||
"No developer messages should be present without a valid tool"
|
||||
)
|
||||
|
||||
|
||||
def test_get_tool_description():
|
||||
"""Test MCPToolServer.get_tool_description filtering logic.
|
||||
|
||||
Note: The wildcard "*" is normalized to None by
|
||||
_extract_allowed_tools_from_mcp_requests before reaching this layer,
|
||||
so we only test None and specific tool filtering here.
|
||||
See test_serving_responses.py for "*" normalization tests.
|
||||
"""
|
||||
pytest.importorskip("mcp")
|
||||
|
||||
server = MCPToolServer()
|
||||
tool1 = ToolDescription.new(
|
||||
name="tool1", description="First", parameters={"type": "object"}
|
||||
)
|
||||
tool2 = ToolDescription.new(
|
||||
name="tool2", description="Second", parameters={"type": "object"}
|
||||
)
|
||||
tool3 = ToolDescription.new(
|
||||
name="tool3", description="Third", parameters={"type": "object"}
|
||||
)
|
||||
|
||||
server.harmony_tool_descriptions = {
|
||||
"test_server": ToolNamespaceConfig(
|
||||
name="test_server", description="test", tools=[tool1, tool2, tool3]
|
||||
)
|
||||
}
|
||||
|
||||
# Nonexistent server
|
||||
assert server.get_tool_description("nonexistent") is None
|
||||
|
||||
# None (no filter) - returns all tools
|
||||
result = server.get_tool_description("test_server", allowed_tools=None)
|
||||
assert len(result.tools) == 3
|
||||
|
||||
# Filter to specific tools
|
||||
result = server.get_tool_description(
|
||||
"test_server", allowed_tools=["tool1", "tool3"]
|
||||
)
|
||||
assert len(result.tools) == 2
|
||||
assert result.tools[0].name == "tool1"
|
||||
assert result.tools[1].name == "tool3"
|
||||
|
||||
# Single tool
|
||||
result = server.get_tool_description("test_server", allowed_tools=["tool2"])
|
||||
assert len(result.tools) == 1
|
||||
assert result.tools[0].name == "tool2"
|
||||
|
||||
# No matching tools - returns None
|
||||
result = server.get_tool_description("test_server", allowed_tools=["nonexistent"])
|
||||
assert result is None
|
||||
|
||||
# Empty list - returns None
|
||||
assert server.get_tool_description("test_server", allowed_tools=[]) is None
|
||||
|
||||
@@ -17,6 +17,7 @@ from vllm.entrypoints.context import ConversationContext
|
||||
from vllm.entrypoints.openai.protocol import ErrorResponse, ResponsesRequest
|
||||
from vllm.entrypoints.openai.serving_responses import (
|
||||
OpenAIServingResponses,
|
||||
_extract_allowed_tools_from_mcp_requests,
|
||||
extract_tool_types,
|
||||
)
|
||||
from vllm.entrypoints.tool_server import ToolServer
|
||||
@@ -254,3 +255,98 @@ class TestValidateGeneratorInput:
|
||||
# Should return an ErrorResponse
|
||||
assert result is not None
|
||||
assert isinstance(result, ErrorResponse)
|
||||
|
||||
|
||||
class TestExtractAllowedToolsFromMcpRequests:
|
||||
"""Test class for _extract_allowed_tools_from_mcp_requests function"""
|
||||
|
||||
def test_extract_allowed_tools_basic_formats(self):
|
||||
"""Test extraction with list format, object format, and None."""
|
||||
from openai.types.responses.tool import McpAllowedToolsMcpToolFilter
|
||||
|
||||
tools = [
|
||||
# List format
|
||||
Mcp(
|
||||
type="mcp",
|
||||
server_label="server1",
|
||||
allowed_tools=["tool1", "tool2"],
|
||||
),
|
||||
# Object format
|
||||
Mcp(
|
||||
type="mcp",
|
||||
server_label="server2",
|
||||
allowed_tools=McpAllowedToolsMcpToolFilter(
|
||||
tool_names=["tool3", "tool4"]
|
||||
),
|
||||
),
|
||||
# None (no filter)
|
||||
Mcp(
|
||||
type="mcp",
|
||||
server_label="server3",
|
||||
allowed_tools=None,
|
||||
),
|
||||
]
|
||||
result = _extract_allowed_tools_from_mcp_requests(tools)
|
||||
assert result == {
|
||||
"server1": ["tool1", "tool2"],
|
||||
"server2": ["tool3", "tool4"],
|
||||
"server3": None,
|
||||
}
|
||||
|
||||
def test_extract_allowed_tools_star_normalization(self):
|
||||
"""Test that '*' wildcard is normalized to None (select all tools).
|
||||
|
||||
This is the key test requested by reviewers to explicitly demonstrate
|
||||
that the "*" select-all scenario is handled correctly.
|
||||
"""
|
||||
from openai.types.responses.tool import McpAllowedToolsMcpToolFilter
|
||||
|
||||
tools = [
|
||||
# Star in list format
|
||||
Mcp(
|
||||
type="mcp",
|
||||
server_label="server1",
|
||||
allowed_tools=["*"],
|
||||
),
|
||||
# Star mixed with other tools in list
|
||||
Mcp(
|
||||
type="mcp",
|
||||
server_label="server2",
|
||||
allowed_tools=["tool1", "*"],
|
||||
),
|
||||
# Star in object format
|
||||
Mcp(
|
||||
type="mcp",
|
||||
server_label="server3",
|
||||
allowed_tools=McpAllowedToolsMcpToolFilter(tool_names=["*"]),
|
||||
),
|
||||
]
|
||||
result = _extract_allowed_tools_from_mcp_requests(tools)
|
||||
# All should be normalized to None (allows all tools)
|
||||
assert result == {
|
||||
"server1": None,
|
||||
"server2": None,
|
||||
"server3": None,
|
||||
}
|
||||
|
||||
def test_extract_allowed_tools_filters_non_mcp(self):
|
||||
"""Test that non-MCP tools are ignored during extraction."""
|
||||
tools = [
|
||||
Mcp(
|
||||
type="mcp",
|
||||
server_label="server1",
|
||||
allowed_tools=["tool1"],
|
||||
),
|
||||
LocalShell(type="local_shell"), # Non-MCP tool should be ignored
|
||||
Mcp(
|
||||
type="mcp",
|
||||
server_label="server2",
|
||||
allowed_tools=["tool2"],
|
||||
),
|
||||
]
|
||||
result = _extract_allowed_tools_from_mcp_requests(tools)
|
||||
# Non-MCP tools should be ignored
|
||||
assert result == {
|
||||
"server1": ["tool1"],
|
||||
"server2": ["tool2"],
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user