From c0afb6c3257f716a033936098b2223469cc424b5 Mon Sep 17 00:00:00 2001 From: eavanvalkenburg Date: Mon, 21 Sep 2026 12:38:37 +0200 Subject: [PATCH] Python: Preserve empty hosted MCP allowlists --- .../agent_framework_foundry/_chat_client.py | 3 +- .../tests/foundry/test_foundry_chat_client.py | 65 +++++++++++++++++++ .../agent_framework_openai/_chat_client.py | 3 +- .../tests/openai/test_openai_chat_client.py | 53 ++++++++++++++- 4 files changed, 121 insertions(+), 3 deletions(-) diff --git a/python/packages/foundry/agent_framework_foundry/_chat_client.py b/python/packages/foundry/agent_framework_foundry/_chat_client.py index 8e42d56249c..3f0d519c025 100644 --- a/python/packages/foundry/agent_framework_foundry/_chat_client.py +++ b/python/packages/foundry/agent_framework_foundry/_chat_client.py @@ -674,6 +674,7 @@ def get_mcp_tool( description: A description of what the MCP server provides. approval_mode: Tool approval mode ("always_require", "never_require", or dict). allowed_tools: List of allowed tool names from this MCP server. + None omits the filter; an empty list is sent unchanged. headers: HTTP headers to include in requests to the MCP server. project_connection_id: Foundry connection ID for managed MCP connections. **kwargs: Additional arguments passed to the SDK MCPTool constructor. @@ -699,7 +700,7 @@ def get_mcp_tool( mcp["project_connection_id"] = project_connection_id elif headers: mcp["headers"] = headers - if allowed_tools: + if allowed_tools is not None: mcp["allowed_tools"] = allowed_tools if approval_mode: if isinstance(approval_mode, str): diff --git a/python/packages/foundry/tests/foundry/test_foundry_chat_client.py b/python/packages/foundry/tests/foundry/test_foundry_chat_client.py index 7ccc8430797..4ed35a7a66c 100644 --- a/python/packages/foundry/tests/foundry/test_foundry_chat_client.py +++ b/python/packages/foundry/tests/foundry/test_foundry_chat_client.py @@ -811,6 +811,71 @@ def test_get_mcp_tool_with_project_connection_id() -> None: assert "server_url" not in tool_config +@pytest.mark.parametrize("allowed_tools", [None, [], ["read_only"]], ids=["omitted", "empty", "nonempty"]) +@pytest.mark.parametrize( + ("url", "project_connection_id"), + [ + param("https://mcp.example", None, id="url"), + param(None, "conn-123", id="connection"), + param("https://mcp.example", "conn-123", id="url-and-connection"), + ], +) +async def test_get_mcp_tool_preserves_allowed_tools_in_request( + allowed_tools: list[str] | None, url: str | None, project_connection_id: str | None +) -> None: + tool_config = FoundryChatClient.get_mcp_tool( + name="Docs MCP", + url=url, + project_connection_id=project_connection_id, + allowed_tools=allowed_tools, + headers={"Authorization": "Bearer test-token"}, + approval_mode="never_require", + ) + expected: dict[str, Any] = { + "type": "mcp", + "server_label": "Docs_MCP", + "require_approval": "never", + } + if url is not None: + expected["server_url"] = url + if project_connection_id is not None: + expected["project_connection_id"] = project_connection_id + else: + expected["headers"] = {"Authorization": "Bearer test-token"} + if allowed_tools is not None: + expected["allowed_tools"] = allowed_tools + assert dict(tool_config) == expected + + requests: list[dict[str, Any]] = [] + + def handle_request(request: Any) -> Any: + requests.append(json.loads(request.content)) + return _OPENAI_HTTPX.Response( + 200, + json={ + "id": "resp_test", + "object": "response", + "created_at": 0, + "model": "test-model", + "status": "completed", + "output": [], + }, + ) + + async with AsyncOpenAI( + api_key="test-key", + base_url="https://example.test/v1", + http_client=DefaultAsyncHttpxClient(transport=_OPENAI_HTTPX.MockTransport(handle_request)), + ) as async_client: + project_client = MagicMock() + project_client.get_openai_client.return_value = async_client + client = FoundryChatClient(project_client=project_client, model="test-model") + await client.get_response([Message(role="user", contents=["Hello"])], options={"tools": [tool_config]}) + + assert len(requests) == 1 + assert requests[0]["tools"] == [expected] + + def test_get_mcp_tool_requires_url_or_project_connection_id() -> None: """Missing both ``url`` and ``project_connection_id`` is always invalid.""" with pytest.raises(ValueError, match="url.*project_connection_id"): diff --git a/python/packages/openai/agent_framework_openai/_chat_client.py b/python/packages/openai/agent_framework_openai/_chat_client.py index e1fe3cc5cf9..636fd38893c 100644 --- a/python/packages/openai/agent_framework_openai/_chat_client.py +++ b/python/packages/openai/agent_framework_openai/_chat_client.py @@ -1471,6 +1471,7 @@ def get_mcp_tool( or provide a dict with "always_require_approval" and/or "never_require_approval" keys mapping to lists of tool names. allowed_tools: List of tool names that are allowed to be used from this MCP server. + None omits the filter; an empty list is sent unchanged. headers: HTTP headers to include in requests to the MCP server. Returns: @@ -1520,7 +1521,7 @@ def get_mcp_tool( if headers: mcp["headers"] = headers - if allowed_tools: + if allowed_tools is not None: mcp["allowed_tools"] = allowed_tools if approval_mode: diff --git a/python/packages/openai/tests/openai/test_openai_chat_client.py b/python/packages/openai/tests/openai/test_openai_chat_client.py index d30448c1e04..92b68650dac 100644 --- a/python/packages/openai/tests/openai/test_openai_chat_client.py +++ b/python/packages/openai/tests/openai/test_openai_chat_client.py @@ -6,6 +6,7 @@ import os from collections.abc import AsyncGenerator, Iterator, Sequence from datetime import datetime, timezone +from importlib import import_module from pathlib import Path from typing import Annotated, Any, cast from unittest.mock import AsyncMock, MagicMock, patch @@ -41,7 +42,7 @@ ChatClientInvalidRequestException, SettingNotFoundError, ) -from openai import AsyncOpenAI, BadRequestError +from openai import AsyncOpenAI, BadRequestError, DefaultAsyncHttpxClient from openai.types.responses import ResponseFunctionShellToolCall, ResponseFunctionShellToolCallOutput from openai.types.responses.response_reasoning_item import Summary from openai.types.responses.response_reasoning_summary_text_delta_event import ( @@ -64,6 +65,8 @@ from agent_framework_openai._chat_client import OPENAI_LOCAL_SHELL_CALL_ITEM_ID_KEY from agent_framework_openai._exceptions import OpenAIContentFilterException +_OPENAI_HTTPX = cast(Any, import_module(DefaultAsyncHttpxClient.__mro__[1].__module__.partition(".")[0])) + skip_if_openai_integration_tests_disabled = pytest.mark.skipif( os.getenv("OPENAI_API_KEY", "") in ("", "test-dummy-key"), reason="No real OPENAI_API_KEY provided; skipping integration tests.", @@ -5576,6 +5579,54 @@ def test_prepare_tools_for_openai_with_mcp() -> None: assert "require_approval" in mcp +@pytest.mark.parametrize("allowed_tools", [None, [], ["read_only"]], ids=["omitted", "empty", "nonempty"]) +async def test_get_mcp_tool_preserves_allowed_tools_in_request(allowed_tools: list[str] | None) -> None: + tool_config = OpenAIChatClient.get_mcp_tool( + name="Docs MCP", + url="https://mcp.example", + allowed_tools=allowed_tools, + headers={"Authorization": "Bearer test-token"}, + approval_mode="never_require", + ) + expected: dict[str, Any] = { + "type": "mcp", + "server_label": "Docs_MCP", + "server_url": "https://mcp.example", + "headers": {"Authorization": "Bearer test-token"}, + "require_approval": "never", + } + if allowed_tools is not None: + expected["allowed_tools"] = allowed_tools + assert tool_config == expected + + requests: list[dict[str, Any]] = [] + + def handle_request(request: Any) -> Any: + requests.append(json.loads(request.content)) + return _OPENAI_HTTPX.Response( + 200, + json={ + "id": "resp_test", + "object": "response", + "created_at": 0, + "model": "test-model", + "status": "completed", + "output": [], + }, + ) + + async with AsyncOpenAI( + api_key="test-key", + base_url="https://example.test/v1", + http_client=DefaultAsyncHttpxClient(transport=_OPENAI_HTTPX.MockTransport(handle_request)), + ) as async_client: + client = OpenAIChatClient(model="test-model", async_client=async_client) + await client.get_response([Message(role="user", contents=["Hello"])], options={"tools": [tool_config]}) + + assert len(requests) == 1 + assert requests[0]["tools"] == [expected] + + def test_prepare_tools_for_openai_single_function_tool() -> None: """Test that a single FunctionTool (not wrapped in a list) is handled correctly.""" client = OpenAIChatClient(model="test-model", api_key="test-key")