Skip to content

Python: Add support for function approval flow in Foundry hosted agent#5666

Merged
TaoChenOSU merged 4 commits intomainfrom
feature/python-hosted-agent-function-approval-flow
May 7, 2026
Merged

Python: Add support for function approval flow in Foundry hosted agent#5666
TaoChenOSU merged 4 commits intomainfrom
feature/python-hosted-agent-function-approval-flow

Conversation

@TaoChenOSU
Copy link
Copy Markdown
Contributor

@TaoChenOSU TaoChenOSU commented May 5, 2026

Motivation and Context

Supersede #5537

The current implementation of Foundry Hosting lacks the function approval flow. This PR adds this feature.

Description

When a tool gets invoked but requires approval, agents in Agent Framework will return a function_approval_request and expect a function_approval_response. These two types do not map to any type in the Foundry Hosted Agent layer.

This PR maps function_approval_request and function_approval_response to mcp_approval_request and mcp_approval_response respectively, allowing tools that require approval to work when the agent is hosted.

The flow goes like:

  1. MAF returns a function_approval_request
  2. The hosted agent integration translates it to a mcp_approval_request and saves the original request for round trip look up.
  3. Client responds with a mcp_approval_response with the same ID as the request.
  4. The hosted agent integration uses the ID to find the original request.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • The PR follows the Contribution Guidelines
  • All unit tests pass, and I have added new tests where possible
  • Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.

@TaoChenOSU TaoChenOSU self-assigned this May 5, 2026
Copilot AI review requested due to automatic review settings May 5, 2026 23:26
@github-actions github-actions Bot changed the title Add support for function approval flow in Foundry hosted agent Python: Add support for function approval flow in Foundry hosted agent May 5, 2026
@moonbox3 moonbox3 added the documentation Improvements or additions to documentation label May 5, 2026
@moonbox3
Copy link
Copy Markdown
Contributor

moonbox3 commented May 5, 2026

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/foundry_hosting/agent_framework_foundry_hosting
   _responses.py61411581%177–180, 248–249, 258–259, 263, 268, 287, 316, 371–373, 375–377, 379–381, 385–388, 400–406, 416–417, 435–437, 442, 444, 451, 453–454, 456, 458, 464–467, 469–471, 475, 478, 483–489, 492–493, 495–496, 504–509, 786, 799, 1265–1267, 1269, 1316–1317, 1319–1320, 1322–1323, 1325–1326, 1331, 1340, 1343–1345, 1347, 1365, 1368, 1410–1411, 1413, 1417–1421, 1423, 1430–1431, 1433–1434, 1440, 1442–1446, 1453, 1459, 1481, 1487
TOTAL33651390688% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
6566 30 💤 0 ❌ 0 🔥 1m 45s ⏱️

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds a function/tool approval round-trip flow to the Foundry-hosted Responses server by persisting approval requests and resolving mcp_approval_* items back into Agent Framework function_approval_* content when requests return.

Changes:

  • Added ApprovalStorage plus in-memory and file-based implementations, and wired approval persistence into ResponsesHostServer message/output conversion.
  • Updated the hosted tools sample to demonstrate always_require approval mode and documented the approval request/response messages.
  • Expanded/updated unit + HTTP round-trip tests to cover approval storage and conversion paths, and converted relevant conversion tests to async.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
python/samples/04-hosting/foundry-hosted-agents/responses/02_tools/README.md Documents the tool approval flow and shows example requests/responses.
python/samples/04-hosting/foundry-hosted-agents/responses/02_tools/main.py Switches the run_bash tool to always_require to exercise the approval flow.
python/packages/foundry_hosting/tests/test_responses.py Adds approval storage + conversion + round-trip tests and updates conversion tests for async APIs.
python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py Implements approval storage, resolves mcp_approval_* items via storage, and persists approval requests emitted by the agent.

Comment thread python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py Outdated
Comment thread python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py Outdated
Comment thread python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py Outdated
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated Code Review

Reviewers: 4 | Confidence: 77%

✓ Correctness

The PR introduces an AprovalStorage mechanism for persisting function approval requests across turns. The implementation is correct for the non-workflow agent path. However, the _handle_inner_workflow method does not pass approval_storage to either _items_to_messages or _to_outputs, meaning if a workflow agent emits a function_approval_request content, it will be surfaced to the client as an mcp_approval_request but never saved to storage—causing a ValueError crash when the client sends back an mcp_approval_response.

✓ Security Reliability

The PR adds approval storage classes (in-memory and file-based) and integrates them into the responses flow. Two reliability concerns: (1) the file-based storage has a TOCTOU race in _create_storage_file_if_not_exists_sync that could truncate data written by a concurrent caller, and (2) in _to_outputs, if the infrastructure fails to assign an item.id to the emitted approval request event, the save is silently skipped with no warning, leading to a confusing KeyError on the subsequent approval response turn.

✓ Test Coverage

The test coverage for the new function approval storage and round-trip flow is comprehensive. Both storage implementations are tested for happy paths and error cases. The conversion functions are tested with and without storage. Full end-to-end round-trip tests cover non-streaming approve/reject flows, streaming persistence, and error handling for unknown IDs. Two minor gaps exist: (1) no test for _to_outputs when approval_storage=None with a function_approval_request content (verifying events emit but no save occurs), and (2) no streaming two-turn round-trip test verifying the approval response reaches the agent in streaming mode.

✓ Design Approach

I found two design-level issues in the new approval persistence approach. First, hosted approval state is written to a cwd-relative path instead of following the existing hosted checkpoint-storage pattern, so it is not using the same host-managed persisted location that the server already relies on for cross-turn state. Second, mcp_approval_request replay was changed from a stateless reconstruction to an out-of-band storage lookup even though the response item already contains the full request payload; that couples ordinary previous_response_id history replay to a sidecar file and creates avoidable failure modes when that file is missing or from an older deployment.


Automated review by TaoChenOSU's agents

Comment thread python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py Outdated
@TaoChenOSU TaoChenOSU marked this pull request as ready for review May 6, 2026 00:10
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated Code Review

Reviewers: 4 | Confidence: 83%

✓ Correctness

The PR adds function approval storage (in-memory and file-based) to persist approval requests so they can be resolved on subsequent turns. The implementation is correct: all referenced Content class methods exist and behave as expected, additional_properties is always a dict (never None), the async/await conversions are consistently applied, and the storage implementations properly handle concurrency within a single process. Tests thoroughly cover the round-trip flow. No correctness issues found.

✓ Security Reliability

The PR introduces a well-structured function approval storage mechanism with atomic file writes, exclusive-create mode, and proper thread safety. The main reliability concern is an ordering issue in the streaming path: approval request events are yielded to the client before the approval request is persisted to storage, creating a window where a save failure leaves the client with an unresolvable approval request ID.

✓ Test Coverage

The test coverage for the new approval storage and round-trip functionality is solid. Tests cover both storage implementations (InMemory and FileBased), conversion functions with and without storage, and end-to-end round-trips for non-streaming flows (approve, reject, unknown ID). The streaming path verifies that approval requests are persisted. Assertions are meaningful—they check specific content types, field values, and the preservation of nested function_call data through serialization. The one notable gap is the absence of a test verifying that _to_outputs silently skips saving when approval_storage=None while still emitting the mcp_approval_request to the client, which could lead to confusing failures on subsequent turns in the workflow path.

✗ Design Approach

I found two design-level problems in the approval round-trip changes. First, the new implementation makes mcp_approval_request replay depend on side storage even though the request item already carries the full tool-call payload, which turns what should be a stateless history reconstruction path into a brittle file-backed dependency. Second, the workflow-hosted path was not wired into the new storage scheme, so approval responses for workflow agents still cannot be converted back into the function_approval_response content that WorkflowAgent expects for pending-request continuation.


Automated review by TaoChenOSU's agents

Comment thread python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py Outdated
@TaoChenOSU TaoChenOSU added this pull request to the merge queue May 7, 2026
Merged via the queue into main with commit 213491d May 7, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation python

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants