Skip to content

Commit 2d6fea6

Browse files
committed
Filter response.mcp_approval_request.* substream events in _should_filter_mcp_chunk
The helper already handled mcp_approval_request items in the output_item.added and output_item.done branches, but the substream event prefix check only matched response.mcp_call.* and response.mcp_list_tools.*, allowing server-side approval request progress events to leak to clients that cannot handle them. Add the missing response.mcp_approval_request.* prefix check and add unit tests for _should_filter_mcp_chunk covering all three substream event types.
1 parent f02648d commit 2d6fea6

2 files changed

Lines changed: 140 additions & 0 deletions

File tree

src/app/endpoints/responses.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,7 @@ def _should_filter_mcp_chunk(
510510
if event_type and (
511511
event_type.startswith("response.mcp_call.")
512512
or event_type.startswith("response.mcp_list_tools.")
513+
or event_type.startswith("response.mcp_approval_request.")
513514
):
514515
output_index = getattr(chunk, "output_index", None)
515516
if output_index in server_mcp_output_indices:

tests/unit/app/endpoints/test_responses.py

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
from app.endpoints.responses import (
1616
_is_server_mcp_output_item,
17+
_should_filter_mcp_chunk,
1718
handle_non_streaming_response,
1819
handle_streaming_response,
1920
responses_endpoint_handler,
@@ -1425,3 +1426,141 @@ def test_file_search_call_returns_false(self) -> None:
14251426
"""Test file_search_call type returns False."""
14261427
item: dict[str, Any] = {"type": "file_search_call"}
14271428
assert _is_server_mcp_output_item(item, {"my-server"}) is False
1429+
1430+
1431+
class TestShouldFilterMcpChunk:
1432+
"""Tests for _should_filter_mcp_chunk helper."""
1433+
1434+
def test_filters_mcp_call_substream_events(self, mocker: MockerFixture) -> None:
1435+
"""Test that response.mcp_call.* events are filtered for tracked indices."""
1436+
chunk = mocker.Mock()
1437+
chunk.output_index = 5
1438+
server_mcp_output_indices: set[int] = {5}
1439+
assert (
1440+
_should_filter_mcp_chunk(
1441+
chunk,
1442+
"response.mcp_call.in_progress",
1443+
{"server-a"},
1444+
server_mcp_output_indices,
1445+
)
1446+
is True
1447+
)
1448+
1449+
def test_filters_mcp_list_tools_substream_events(
1450+
self, mocker: MockerFixture
1451+
) -> None:
1452+
"""Test that response.mcp_list_tools.* events are filtered for tracked indices."""
1453+
chunk = mocker.Mock()
1454+
chunk.output_index = 3
1455+
server_mcp_output_indices: set[int] = {3}
1456+
assert (
1457+
_should_filter_mcp_chunk(
1458+
chunk,
1459+
"response.mcp_list_tools.in_progress",
1460+
{"server-a"},
1461+
server_mcp_output_indices,
1462+
)
1463+
is True
1464+
)
1465+
1466+
def test_filters_mcp_approval_request_substream_events(
1467+
self, mocker: MockerFixture
1468+
) -> None:
1469+
"""Test that response.mcp_approval_request.* events are filtered for tracked indices."""
1470+
chunk = mocker.Mock()
1471+
chunk.output_index = 7
1472+
server_mcp_output_indices: set[int] = {7}
1473+
assert (
1474+
_should_filter_mcp_chunk(
1475+
chunk,
1476+
"response.mcp_approval_request.in_progress",
1477+
{"server-a"},
1478+
server_mcp_output_indices,
1479+
)
1480+
is True
1481+
)
1482+
1483+
def test_does_not_filter_untracked_mcp_approval_request(
1484+
self, mocker: MockerFixture
1485+
) -> None:
1486+
"""Test that mcp_approval_request events for untracked indices pass through."""
1487+
chunk = mocker.Mock()
1488+
chunk.output_index = 7
1489+
server_mcp_output_indices: set[int] = {99}
1490+
assert (
1491+
_should_filter_mcp_chunk(
1492+
chunk,
1493+
"response.mcp_approval_request.in_progress",
1494+
{"server-a"},
1495+
server_mcp_output_indices,
1496+
)
1497+
is False
1498+
)
1499+
1500+
def test_does_not_filter_untracked_mcp_call(self, mocker: MockerFixture) -> None:
1501+
"""Test that mcp_call events for untracked indices pass through."""
1502+
chunk = mocker.Mock()
1503+
chunk.output_index = 10
1504+
server_mcp_output_indices: set[int] = {5}
1505+
assert (
1506+
_should_filter_mcp_chunk(
1507+
chunk,
1508+
"response.mcp_call.completed",
1509+
{"server-a"},
1510+
server_mcp_output_indices,
1511+
)
1512+
is False
1513+
)
1514+
1515+
def test_filters_output_item_added_for_server_mcp(
1516+
self, mocker: MockerFixture
1517+
) -> None:
1518+
"""Test that output_item.added for server MCP items is filtered and tracked."""
1519+
item = mocker.Mock()
1520+
item.type = "mcp_approval_request"
1521+
item.server_label = "server-a"
1522+
chunk = mocker.Mock()
1523+
chunk.item = item
1524+
chunk.output_index = 2
1525+
server_mcp_output_indices: set[int] = set()
1526+
assert (
1527+
_should_filter_mcp_chunk(
1528+
chunk,
1529+
"response.output_item.added",
1530+
{"server-a"},
1531+
server_mcp_output_indices,
1532+
)
1533+
is True
1534+
)
1535+
assert 2 in server_mcp_output_indices
1536+
1537+
def test_filters_output_item_done_for_server_mcp(
1538+
self, mocker: MockerFixture
1539+
) -> None:
1540+
"""Test that output_item.done for server MCP items is filtered and cleaned up."""
1541+
item = mocker.Mock()
1542+
item.type = "mcp_approval_request"
1543+
chunk = mocker.Mock()
1544+
chunk.item = item
1545+
chunk.output_index = 2
1546+
server_mcp_output_indices: set[int] = {2}
1547+
assert (
1548+
_should_filter_mcp_chunk(
1549+
chunk,
1550+
"response.output_item.done",
1551+
{"server-a"},
1552+
server_mcp_output_indices,
1553+
)
1554+
is True
1555+
)
1556+
assert 2 not in server_mcp_output_indices
1557+
1558+
def test_does_not_filter_non_mcp_event(self, mocker: MockerFixture) -> None:
1559+
"""Test that non-MCP events pass through."""
1560+
chunk = mocker.Mock()
1561+
assert (
1562+
_should_filter_mcp_chunk(
1563+
chunk, "response.output_text.delta", {"server-a"}, set()
1564+
)
1565+
is False
1566+
)

0 commit comments

Comments
 (0)