Skip to content

Commit 21769e2

Browse files
giles17dmytrostruk
andauthored
Python: Fix hosted MCP tool approval flow for all session/streaming combinations (microsoft#4054)
* fix openai hosted mcp samples * addressed copilot comments * Update python/samples/02-agents/providers/azure_openai/azure_responses_client_with_hosted_mcp.py Co-authored-by: Dmytro Struk <13853051+dmytrostruk@users.noreply.github.com> --------- Co-authored-by: Dmytro Struk <13853051+dmytrostruk@users.noreply.github.com>
1 parent 988ef6a commit 21769e2

File tree

6 files changed

+242
-12
lines changed

6 files changed

+242
-12
lines changed

python/packages/core/agent_framework/_agents.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -921,6 +921,20 @@ async def _get_stream() -> ResponseStream[ChatResponseUpdate, ChatResponse]:
921921
**ctx["filtered_kwargs"],
922922
)
923923

924+
def _propagate_conversation_id(update: AgentResponseUpdate) -> AgentResponseUpdate:
925+
"""Eagerly propagate conversation_id to session as updates arrive.
926+
927+
This ensures session.service_session_id is set even when the user
928+
only iterates the stream without calling get_final_response().
929+
"""
930+
if session is None:
931+
return update
932+
raw = update.raw_representation
933+
conv_id = getattr(raw, "conversation_id", None) if raw else None
934+
if isinstance(conv_id, str) and conv_id and session.service_session_id != conv_id:
935+
session.service_session_id = conv_id
936+
return update
937+
924938
return (
925939
ResponseStream
926940
.from_awaitable(_get_stream())
@@ -933,6 +947,7 @@ async def _get_stream() -> ResponseStream[ChatResponseUpdate, ChatResponse]:
933947
self._finalize_response_updates, response_format=options.get("response_format") if options else None
934948
),
935949
)
950+
.with_transform_hook(_propagate_conversation_id)
936951
.with_result_hook(_post_hook)
937952
)
938953

python/packages/core/agent_framework/_tools.py

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1659,17 +1659,34 @@ def _extract_tools(
16591659
return None
16601660

16611661

1662+
def _is_hosted_tool_approval(content: Any) -> bool:
1663+
"""Check if a function_approval_request/response is for a hosted tool (e.g. MCP).
1664+
1665+
Hosted tool approvals have a server_label in function_call.additional_properties
1666+
and should be passed through to the API untouched rather than processed locally.
1667+
"""
1668+
fc = getattr(content, "function_call", None)
1669+
if fc is None:
1670+
return False
1671+
ap = getattr(fc, "additional_properties", None)
1672+
return bool(ap and ap.get("server_label"))
1673+
1674+
16621675
def _collect_approval_responses(
16631676
messages: list[Message],
16641677
) -> dict[str, Content]:
1665-
"""Collect approval responses (both approved and rejected) from messages."""
1678+
"""Collect approval responses (both approved and rejected) from messages.
1679+
1680+
Hosted tool approvals (e.g. MCP) are excluded because they must be
1681+
forwarded to the API as-is rather than processed locally.
1682+
"""
16661683
from ._types import Message
16671684

16681685
fcc_todo: dict[str, Content] = {}
16691686
for msg in messages:
16701687
for content in msg.contents if isinstance(msg, Message) else []:
1671-
# Collect BOTH approved and rejected responses
1672-
if content.type == "function_approval_response":
1688+
# Collect BOTH approved and rejected responses, but skip hosted tool approvals
1689+
if content.type == "function_approval_response" and not _is_hosted_tool_approval(content):
16731690
fcc_todo[content.id] = content # type: ignore[attr-defined, index]
16741691
return fcc_todo
16751692

@@ -1698,6 +1715,9 @@ def _replace_approval_contents_with_results(
16981715

16991716
for content_idx, content in enumerate(msg.contents):
17001717
if content.type == "function_approval_request":
1718+
# Skip hosted tool approvals — they must pass through to the API unchanged
1719+
if _is_hosted_tool_approval(content):
1720+
continue
17011721
# Don't add the function call if it already exists (would create duplicate)
17021722
if content.function_call.call_id in existing_call_ids: # type: ignore[attr-defined, union-attr, operator]
17031723
# Just mark for removal - the function call already exists
@@ -1706,6 +1726,9 @@ def _replace_approval_contents_with_results(
17061726
# Put back the function call content only if it doesn't exist
17071727
msg.contents[content_idx] = content.function_call # type: ignore[attr-defined, assignment]
17081728
elif content.type == "function_approval_response":
1729+
# Skip hosted tool approvals — they must pass through to the API unchanged
1730+
if _is_hosted_tool_approval(content):
1731+
continue
17091732
if content.approved and content.id in fcc_todo: # type: ignore[attr-defined]
17101733
# Replace with the corresponding result
17111734
if result_idx < len(approved_function_results):

python/packages/core/tests/core/test_agents.py

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,45 @@ async def test_chat_client_agent_update_session_id_streaming_does_not_use_respon
266266
assert session.service_session_id is None
267267

268268

269+
async def test_chat_client_agent_streaming_session_id_set_without_get_final_response(
270+
chat_client_base: SupportsChatGetResponse,
271+
) -> None:
272+
"""Test that session.service_session_id is set during streaming iteration.
273+
274+
This verifies the eager propagation of conversation_id via transform hook,
275+
which is needed for multi-turn flows (e.g. hosted MCP approval) where the
276+
user iterates the stream and then makes a follow-up call without calling
277+
get_final_response().
278+
"""
279+
chat_client_base.streaming_responses = [
280+
[
281+
ChatResponseUpdate(
282+
contents=[Content.from_text("part 1")],
283+
role="assistant",
284+
response_id="resp_123",
285+
conversation_id="resp_123",
286+
),
287+
ChatResponseUpdate(
288+
contents=[Content.from_text(" part 2")],
289+
role="assistant",
290+
response_id="resp_123",
291+
conversation_id="resp_123",
292+
finish_reason="stop",
293+
),
294+
]
295+
]
296+
297+
agent = Agent(client=chat_client_base)
298+
session = agent.create_session()
299+
assert session.service_session_id is None
300+
301+
# Only iterate — do NOT call get_final_response()
302+
async for _ in agent.run("Hello", session=session, stream=True):
303+
pass
304+
305+
assert session.service_session_id == "resp_123"
306+
307+
269308
async def test_chat_client_agent_update_session_messages(client: SupportsChatGetResponse) -> None:
270309
from agent_framework._sessions import InMemoryHistoryProvider
271310

python/packages/core/tests/core/test_function_invocation_logic.py

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1255,6 +1255,152 @@ def local_func(arg1: str) -> str:
12551255
assert response is not None
12561256

12571257

1258+
async def test_hosted_mcp_approval_response_passthrough(chat_client_base: SupportsChatGetResponse):
1259+
"""Test that hosted MCP approval responses pass through without local execution.
1260+
1261+
When an MCP approval response has server_label in function_call.additional_properties,
1262+
the function invocation layer must not intercept it. The approval request/response
1263+
should be forwarded to the API as-is so the service can execute the hosted tool.
1264+
"""
1265+
1266+
@tool(name="local_function")
1267+
def local_func(arg1: str) -> str:
1268+
return f"Local {arg1}"
1269+
1270+
# Simulate an MCP approval request from the service (has server_label)
1271+
mcp_function_call = Content.from_function_call(
1272+
call_id="mcpr_abc123",
1273+
name="microsoft_docs_search",
1274+
arguments='{"query": "azure storage"}',
1275+
additional_properties={"server_label": "Microsoft_Learn_MCP"},
1276+
)
1277+
mcp_approval_request = Content.from_function_approval_request(
1278+
id="mcpr_abc123",
1279+
function_call=mcp_function_call,
1280+
)
1281+
mcp_approval_response = mcp_approval_request.to_function_approval_response(approved=True)
1282+
1283+
# The second call (after approval) should return a final response
1284+
chat_client_base.run_responses = [
1285+
ChatResponse(messages=Message(role="assistant", text="Here are the docs results.")),
1286+
]
1287+
1288+
# Build message list mimicking handle_approvals_without_session:
1289+
# [original query, assistant with approval_request, user with approval_response]
1290+
messages = [
1291+
Message(role="user", text="Search docs for azure storage"),
1292+
Message(role="assistant", contents=[mcp_approval_request]),
1293+
Message(role="user", contents=[mcp_approval_response]),
1294+
]
1295+
1296+
response = await chat_client_base.get_response(
1297+
messages,
1298+
tool_choice="auto",
1299+
tools=[local_func],
1300+
)
1301+
1302+
# The response should succeed without errors
1303+
assert response is not None
1304+
assert response.messages[0].text == "Here are the docs results."
1305+
1306+
# The approval contents should NOT have been mutated by the function invocation layer.
1307+
# The assistant message should still have the original approval_request content.
1308+
assistant_msg = messages[1]
1309+
assert assistant_msg.contents[0].type == "function_approval_request"
1310+
# The user message should still have the original approval_response content.
1311+
user_msg = messages[2]
1312+
assert user_msg.contents[0].type == "function_approval_response"
1313+
1314+
1315+
def test_is_hosted_tool_approval_with_server_label():
1316+
"""Test that _is_hosted_tool_approval returns True for MCP approvals with server_label."""
1317+
from agent_framework._tools import _is_hosted_tool_approval
1318+
1319+
mcp_fc = Content.from_function_call(
1320+
call_id="mcpr_abc",
1321+
name="docs_search",
1322+
arguments="{}",
1323+
additional_properties={"server_label": "Microsoft_Learn_MCP"},
1324+
)
1325+
mcp_request = Content.from_function_approval_request(id="mcpr_abc", function_call=mcp_fc)
1326+
mcp_response = mcp_request.to_function_approval_response(approved=True)
1327+
1328+
assert _is_hosted_tool_approval(mcp_request) is True
1329+
assert _is_hosted_tool_approval(mcp_response) is True
1330+
1331+
1332+
def test_is_hosted_tool_approval_without_server_label():
1333+
"""Test that _is_hosted_tool_approval returns False for regular tool approvals."""
1334+
from agent_framework._tools import _is_hosted_tool_approval
1335+
1336+
regular_fc = Content.from_function_call(call_id="call_1", name="my_func", arguments="{}")
1337+
regular_request = Content.from_function_approval_request(id="call_1", function_call=regular_fc)
1338+
regular_response = regular_request.to_function_approval_response(approved=True)
1339+
1340+
assert _is_hosted_tool_approval(regular_request) is False
1341+
assert _is_hosted_tool_approval(regular_response) is False
1342+
# Also test with None/non-content objects
1343+
assert _is_hosted_tool_approval(None) is False
1344+
assert _is_hosted_tool_approval("not a content") is False
1345+
1346+
1347+
async def test_mixed_local_and_hosted_approval_flow(chat_client_base: SupportsChatGetResponse):
1348+
"""Test that mixed local + hosted MCP approvals are handled correctly.
1349+
1350+
When a response contains both a local tool approval and a hosted MCP approval,
1351+
the local approval should be processed normally while the hosted MCP approval
1352+
should pass through untouched to the API.
1353+
"""
1354+
1355+
@tool(name="local_function", approval_mode="always_require")
1356+
def local_func(arg1: str) -> str:
1357+
return f"Local {arg1}"
1358+
1359+
# Simulate the LLM returning both a local function call and an MCP approval request
1360+
local_fc = Content.from_function_call(call_id="call_local", name="local_function", arguments='{"arg1": "test"}')
1361+
mcp_fc = Content.from_function_call(
1362+
call_id="mcpr_hosted",
1363+
name="microsoft_docs_search",
1364+
arguments='{"query": "azure"}',
1365+
additional_properties={"server_label": "Microsoft_Learn_MCP"},
1366+
)
1367+
mcp_approval_request = Content.from_function_approval_request(id="mcpr_hosted", function_call=mcp_fc)
1368+
1369+
# First response: LLM returns a local function call that needs approval
1370+
chat_client_base.run_responses = [
1371+
ChatResponse(messages=Message(role="assistant", contents=[local_fc])),
1372+
# After local approval + hosted approval, the final response
1373+
ChatResponse(messages=Message(role="assistant", text="Done with both tools.")),
1374+
]
1375+
1376+
# User approves the local function call
1377+
local_approval_response = Content.from_function_approval_response(
1378+
approved=True, id="call_local", function_call=local_fc
1379+
)
1380+
# User also has an MCP approval response (hosted)
1381+
mcp_approval_response = mcp_approval_request.to_function_approval_response(approved=True)
1382+
1383+
messages = [
1384+
Message(role="user", text="Search docs and run local"),
1385+
Message(role="assistant", contents=[local_fc, mcp_approval_request]),
1386+
Message(role="user", contents=[local_approval_response]),
1387+
Message(role="user", contents=[mcp_approval_response]),
1388+
]
1389+
1390+
response = await chat_client_base.get_response(
1391+
messages,
1392+
tool_choice="auto",
1393+
tools=[local_func],
1394+
)
1395+
1396+
assert response is not None
1397+
# The hosted MCP approval contents should NOT have been mutated
1398+
assistant_msg = messages[1]
1399+
assert assistant_msg.contents[1].type == "function_approval_request"
1400+
mcp_user_msg = messages[3]
1401+
assert mcp_user_msg.contents[0].type == "function_approval_response"
1402+
1403+
12581404
async def test_unapproved_tool_execution_raises_exception(chat_client_base: SupportsChatGetResponse):
12591405
"""Test that attempting to execute an unapproved tool raises ToolException."""
12601406

python/samples/02-agents/providers/azure_openai/azure_responses_client_with_hosted_mcp.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,14 @@ async def handle_approvals_with_session_streaming(query: str, agent: "SupportsAg
7070
"""Here we let the session deal with the previous responses, and we just rerun with the approval."""
7171
from agent_framework import Message
7272

73-
new_input: list[Message] = []
73+
new_input: list[Message | str] = [query]
7474
new_input_added = True
7575
while new_input_added:
7676
new_input_added = False
77-
new_input.append(Message(role="user", text=query))
7877
async for update in agent.run(new_input, session=session, options={"store": True}, stream=True):
7978
if update.user_input_requests:
79+
# Reset input to only contain new approval responses for the next iteration
80+
new_input = []
8081
for user_input_needed in update.user_input_requests:
8182
print(
8283
f"User Input Request for function from {agent.name}: {user_input_needed.function_call.name}"
@@ -114,7 +115,8 @@ async def run_hosted_mcp_without_session_and_specific_approval() -> None:
114115
async with Agent(
115116
client=client,
116117
name="DocsAgent",
117-
instructions="You are a helpful assistant that can help with microsoft documentation questions.",
118+
instructions="You are a helpful assistant that uses your MCP tool "
119+
"to help with microsoft documentation questions.",
118120
tools=[mcp_tool],
119121
) as agent:
120122
# First query
@@ -151,7 +153,8 @@ async def run_hosted_mcp_without_approval() -> None:
151153
async with Agent(
152154
client=client,
153155
name="DocsAgent",
154-
instructions="You are a helpful assistant that can help with microsoft documentation questions.",
156+
instructions="You are a helpful assistant that uses your MCP tool "
157+
"to help with Microsoft documentation questions.",
155158
tools=[mcp_tool],
156159
) as agent:
157160
# First query
@@ -186,7 +189,8 @@ async def run_hosted_mcp_with_session() -> None:
186189
async with Agent(
187190
client=client,
188191
name="DocsAgent",
189-
instructions="You are a helpful assistant that can help with microsoft documentation questions.",
192+
instructions="You are a helpful assistant that uses your MCP tool "
193+
"to help with microsoft documentation questions.",
190194
tools=[mcp_tool],
191195
) as agent:
192196
# First query
@@ -222,7 +226,8 @@ async def run_hosted_mcp_with_session_streaming() -> None:
222226
async with Agent(
223227
client=client,
224228
name="DocsAgent",
225-
instructions="You are a helpful assistant that can help with microsoft documentation questions.",
229+
instructions="You are a helpful assistant that uses your MCP tool "
230+
"to help with microsoft documentation questions.",
226231
tools=[mcp_tool],
227232
) as agent:
228233
# First query

python/samples/02-agents/providers/openai/openai_responses_client_with_hosted_mcp.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,15 @@
55

66
from agent_framework import Agent
77
from agent_framework.openai import OpenAIResponsesClient
8+
from dotenv import load_dotenv
89

910
"""
1011
OpenAI Responses Client with Hosted MCP Example
1112
1213
This sample demonstrates integrating hosted Model Context Protocol (MCP) tools with
1314
OpenAI Responses Client, including user approval workflows for function call security.
1415
"""
15-
16+
load_dotenv() # Load environment variables from .env file if present
1617
if TYPE_CHECKING:
1718
from agent_framework import AgentSession, SupportsAgentRun
1819

@@ -69,13 +70,14 @@ async def handle_approvals_with_session_streaming(query: str, agent: "SupportsAg
6970
"""Here we let the session deal with the previous responses, and we just rerun with the approval."""
7071
from agent_framework import Message
7172

72-
new_input: list[Message] = []
73+
new_input: list[Message | str] = [query]
7374
new_input_added = True
7475
while new_input_added:
7576
new_input_added = False
76-
new_input.append(Message(role="user", text=query))
7777
async for update in agent.run(new_input, session=session, stream=True, options={"store": True}):
7878
if update.user_input_requests:
79+
# Reset input to only contain new approval responses for the next iteration
80+
new_input = []
7981
for user_input_needed in update.user_input_requests:
8082
print(
8183
f"User Input Request for function from {agent.name}: {user_input_needed.function_call.name}"

0 commit comments

Comments
 (0)