Uh oh!
There was an error while loading. Please reload this page.
Python: Add support for function approval flow in Foundry hosted agent - #5666
Merged
Tao Chen (TaoChenOSU) merged 4 commits intoMay 7, 2026
Merged
Conversation
Contributor
Python Test Coverage Report •
Python Unit Test Overview
| ||||||||||||||||||||||||||||||
Contributor
There was a problem hiding this comment.
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
ApprovalStorageplus in-memory and file-based implementations, and wired approval persistence intoResponsesHostServermessage/output conversion. - Updated the hosted tools sample to demonstrate
always_requireapproval 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. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Contributor
There was a problem hiding this comment.
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_workflowmethod does not passapproval_storageto either_items_to_messagesor_to_outputs, meaning if a workflow agent emits afunction_approval_requestcontent, it will be surfaced to the client as anmcp_approval_requestbut never saved to storage—causing aValueErrorcrash when the client sends back anmcp_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_syncthat could truncate data written by a concurrent caller, and (2) in_to_outputs, if the infrastructure fails to assign anitem.idto 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_outputswhenapproval_storage=Nonewith afunction_approval_requestcontent (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_requestreplay 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 ordinaryprevious_response_idhistory 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
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Tao Chen (TaoChenOSU)
marked this pull request as ready for review
May 6, 2026 00:10
Contributor
There was a problem hiding this comment.
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_outputssilently skips saving whenapproval_storage=Nonewhile still emitting themcp_approval_requestto 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_requestreplay 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 thefunction_approval_responsecontent thatWorkflowAgentexpects for pending-request continuation.
Automated review by TaoChenOSU's agents
Uh oh!
There was an error while loading. Please reload this page.
Ben Thomas (alliscode)
approved these changes
May 6, 2026
Uh oh!
There was an error while loading. Please reload this page.
Evan Mattson (moonbox3)
approved these changes
May 7, 2026
Uh oh!
There was an error while loading. Please reload this page.
Eduard van Valkenburg (eavanvalkenburg) added a commit
to eavanvalkenburg/agent-framework
that referenced
this pull request
May 18, 2026
…, microsoft#5666) Brings the FoundryHostedAgentHistoryProvider refactor branch back into sync with the foundry_hosting changes that have landed on upstream main since PR-1 was opened: * microsoft#5851 (path traversal in checkpoint storage, CWE-22). The workflow-host code in ``_responses.py`` builds a ``FileCheckpointStorage`` from a caller-controlled ``context_id`` (``previous_response_id`` / ``conversation_id`` / ``response_id``). Switch both call sites to route through ``_checkpoint_storage_for_context``, which rejects separators, NUL bytes, drive letters, absolute paths, and all-dot segments, and enforces ``is_relative_to(root)`` before any directory is created. * microsoft#5666 (function approval flow). Make the SDK-Item → AF-Message conversion helpers in ``_shared.py`` async and accept an optional ``approval_storage`` keyword: - ``_items_to_messages`` / ``_item_to_message`` / ``_item_to_message_inner`` - ``_output_items_to_messages`` / ``_output_item_to_message`` / ``_output_item_to_message_inner`` For ``mcp_approval_request`` / ``mcp_approval_response`` items the helpers now load the original function-call Content from the approval storage (via ``ApprovalStorage.load_approval_request``) instead of synthesising a placeholder. This matches upstream semantics and lets approval round-trips reconstruct the real payload. The ``ApprovalStorage`` Protocol moves to ``_shared.py`` so the conversion helpers can reference it without pulling in ``_responses.py`` (which would create a circular import). The concrete ``InMemoryFunctionApprovalStorage`` and ``FileBasedFunctionApprovalStorage`` stay in ``_responses.py`` next to the host that owns them, and re-export ``ApprovalStorage`` from ``_shared`` for compatibility. The workflow-host streaming path passes its own ``self._approval_storage`` into ``_to_outputs`` so approval requests are saved at emit time. * Bump ``_history_provider.FoundryHostedAgentHistoryProvider.get_messages`` to ``await`` the now-async ``_output_items_to_messages`` call. No public API change beyond the new keyword-only ``approval_storage`` parameter on the four conversion entry points. Validation: - uv run poe check-packages -P foundry_hosting (lint + pyright clean) - uv run poe mypy -P foundry_hosting (clean) - uv run poe test -P foundry_hosting (183 passed, 1 skipped) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Eduard van Valkenburg (eavanvalkenburg) added a commit
to eavanvalkenburg/agent-framework
that referenced
this pull request
May 22, 2026
…, microsoft#5666) Brings the FoundryHostedAgentHistoryProvider refactor branch back into sync with the foundry_hosting changes that have landed on upstream main since PR-1 was opened: * microsoft#5851 (path traversal in checkpoint storage, CWE-22). The workflow-host code in ``_responses.py`` builds a ``FileCheckpointStorage`` from a caller-controlled ``context_id`` (``previous_response_id`` / ``conversation_id`` / ``response_id``). Switch both call sites to route through ``_checkpoint_storage_for_context``, which rejects separators, NUL bytes, drive letters, absolute paths, and all-dot segments, and enforces ``is_relative_to(root)`` before any directory is created. * microsoft#5666 (function approval flow). Make the SDK-Item → AF-Message conversion helpers in ``_shared.py`` async and accept an optional ``approval_storage`` keyword: - ``_items_to_messages`` / ``_item_to_message`` / ``_item_to_message_inner`` - ``_output_items_to_messages`` / ``_output_item_to_message`` / ``_output_item_to_message_inner`` For ``mcp_approval_request`` / ``mcp_approval_response`` items the helpers now load the original function-call Content from the approval storage (via ``ApprovalStorage.load_approval_request``) instead of synthesising a placeholder. This matches upstream semantics and lets approval round-trips reconstruct the real payload. The ``ApprovalStorage`` Protocol moves to ``_shared.py`` so the conversion helpers can reference it without pulling in ``_responses.py`` (which would create a circular import). The concrete ``InMemoryFunctionApprovalStorage`` and ``FileBasedFunctionApprovalStorage`` stay in ``_responses.py`` next to the host that owns them, and re-export ``ApprovalStorage`` from ``_shared`` for compatibility. The workflow-host streaming path passes its own ``self._approval_storage`` into ``_to_outputs`` so approval requests are saved at emit time. * Bump ``_history_provider.FoundryHostedAgentHistoryProvider.get_messages`` to ``await`` the now-async ``_output_items_to_messages`` call. No public API change beyond the new keyword-only ``approval_storage`` parameter on the four conversion entry points. Validation: - uv run poe check-packages -P foundry_hosting (lint + pyright clean) - uv run poe mypy -P foundry_hosting (clean) - uv run poe test -P foundry_hosting (183 passed, 1 skipped) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Eduard van Valkenburg (eavanvalkenburg) added a commit
to eavanvalkenburg/agent-framework
that referenced
this pull request
May 22, 2026
…, microsoft#5666) Brings the FoundryHostedAgentHistoryProvider refactor branch back into sync with the foundry_hosting changes that have landed on upstream main since PR-1 was opened: * microsoft#5851 (path traversal in checkpoint storage, CWE-22). The workflow-host code in ``_responses.py`` builds a ``FileCheckpointStorage`` from a caller-controlled ``context_id`` (``previous_response_id`` / ``conversation_id`` / ``response_id``). Switch both call sites to route through ``_checkpoint_storage_for_context``, which rejects separators, NUL bytes, drive letters, absolute paths, and all-dot segments, and enforces ``is_relative_to(root)`` before any directory is created. * microsoft#5666 (function approval flow). Make the SDK-Item → AF-Message conversion helpers in ``_shared.py`` async and accept an optional ``approval_storage`` keyword: - ``_items_to_messages`` / ``_item_to_message`` / ``_item_to_message_inner`` - ``_output_items_to_messages`` / ``_output_item_to_message`` / ``_output_item_to_message_inner`` For ``mcp_approval_request`` / ``mcp_approval_response`` items the helpers now load the original function-call Content from the approval storage (via ``ApprovalStorage.load_approval_request``) instead of synthesising a placeholder. This matches upstream semantics and lets approval round-trips reconstruct the real payload. The ``ApprovalStorage`` Protocol moves to ``_shared.py`` so the conversion helpers can reference it without pulling in ``_responses.py`` (which would create a circular import). The concrete ``InMemoryFunctionApprovalStorage`` and ``FileBasedFunctionApprovalStorage`` stay in ``_responses.py`` next to the host that owns them, and re-export ``ApprovalStorage`` from ``_shared`` for compatibility. The workflow-host streaming path passes its own ``self._approval_storage`` into ``_to_outputs`` so approval requests are saved at emit time. * Bump ``_history_provider.FoundryHostedAgentHistoryProvider.get_messages`` to ``await`` the now-async ``_output_items_to_messages`` call. No public API change beyond the new keyword-only ``approval_storage`` parameter on the four conversion entry points. Validation: - uv run poe check-packages -P foundry_hosting (lint + pyright clean) - uv run poe mypy -P foundry_hosting (clean) - uv run poe test -P foundry_hosting (183 passed, 1 skipped) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Eduard van Valkenburg (eavanvalkenburg) added a commit
to eavanvalkenburg/agent-framework
that referenced
this pull request
May 22, 2026
…, microsoft#5666) Brings the FoundryHostedAgentHistoryProvider refactor branch back into sync with the foundry_hosting changes that have landed on upstream main since PR-1 was opened: * microsoft#5851 (path traversal in checkpoint storage, CWE-22). The workflow-host code in ``_responses.py`` builds a ``FileCheckpointStorage`` from a caller-controlled ``context_id`` (``previous_response_id`` / ``conversation_id`` / ``response_id``). Switch both call sites to route through ``_checkpoint_storage_for_context``, which rejects separators, NUL bytes, drive letters, absolute paths, and all-dot segments, and enforces ``is_relative_to(root)`` before any directory is created. * microsoft#5666 (function approval flow). Make the SDK-Item → AF-Message conversion helpers in ``_shared.py`` async and accept an optional ``approval_storage`` keyword: - ``_items_to_messages`` / ``_item_to_message`` / ``_item_to_message_inner`` - ``_output_items_to_messages`` / ``_output_item_to_message`` / ``_output_item_to_message_inner`` For ``mcp_approval_request`` / ``mcp_approval_response`` items the helpers now load the original function-call Content from the approval storage (via ``ApprovalStorage.load_approval_request``) instead of synthesising a placeholder. This matches upstream semantics and lets approval round-trips reconstruct the real payload. The ``ApprovalStorage`` Protocol moves to ``_shared.py`` so the conversion helpers can reference it without pulling in ``_responses.py`` (which would create a circular import). The concrete ``InMemoryFunctionApprovalStorage`` and ``FileBasedFunctionApprovalStorage`` stay in ``_responses.py`` next to the host that owns them, and re-export ``ApprovalStorage`` from ``_shared`` for compatibility. The workflow-host streaming path passes its own ``self._approval_storage`` into ``_to_outputs`` so approval requests are saved at emit time. * Bump ``_history_provider.FoundryHostedAgentHistoryProvider.get_messages`` to ``await`` the now-async ``_output_items_to_messages`` call. No public API change beyond the new keyword-only ``approval_storage`` parameter on the four conversion entry points. Validation: - uv run poe check-packages -P foundry_hosting (lint + pyright clean) - uv run poe mypy -P foundry_hosting (clean) - uv run poe test -P foundry_hosting (183 passed, 1 skipped) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Eduard van Valkenburg (eavanvalkenburg) added a commit
that referenced
this pull request
May 22, 2026
…5637) * refactor(foundry_hosting): build FoundryHostedAgentHistoryProvider on azure.ai.agentserver SDK Rebuilds the Foundry hosted-agent history provider on top of ``azure.ai.agentserver``'s ``FoundryStorageProvider`` instead of the in-house ``_HttpStorageBackend``. Splits the monolithic ``_responses.py`` into focused modules: - ``_history_provider.py`` — new ``FoundryHostedAgentHistoryProvider`` that talks to the SDK's ``FoundryStorageProvider``, threads ``response_id`` / ``previous_response_id`` through ``ContextVar``s via ``bind_request_context``, and lifts host-bound isolation keys (``x-agent-{user,chat}-isolation-key``) from the optional ``agent_framework_hosting`` package into a provider-local ``IsolationContext`` so the storage layer carries the correct partition keys without channels having to know about them. - ``_shared.py`` — extracts all SDK ``Item`` / ``OutputItem`` ↔ framework ``Message`` conversion helpers into one place so both ``_responses.py`` and the new history provider can share them. Restores ``_convert_file_data`` for inline ``input_file`` payloads, and the hosted-MCP routing for ``custom_tool_call_output`` items whose ``call_id`` carries the ``mcp_*`` prefix. - ``_ids.py`` — shared id helpers. - ``_responses.py`` — shrinks ~700 lines, re-exports converters for back-compat with existing tests. - ``tests/test_history_provider.py`` — exercises the new provider against a fake SDK backend; the host-isolation test is gated on the optional ``agent_framework_hosting`` import. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(foundry_hosting): add local_storage_root for file-based dev history Adds an optional `local_storage_root: str | Path | None` parameter to `FoundryHostedAgentHistoryProvider`. When set and the provider is running outside a Foundry Hosted Agent container, conversations are persisted to JSONL files via `agent_framework.FileHistoryProvider` laid out as: {root}/{user_key or '~none'}/{chat_key or '~none'}/{session_id}.jsonl Hosted mode (FOUNDRY_HOSTING_ENVIRONMENT set) ignores the option with a one-time INFO log so Foundry storage always wins on the platform. The in-memory fallback is unchanged when the option is omitted. Path safety: isolation segments are validated against the same character allowlist FileHistoryProvider uses for session-id stems and base64-url-encoded with a reserved "~iso-" prefix when unsafe. "~none" sentinel for missing keys can never collide with a real isolation key (real keys starting with "~" are encoded). The resolved target dir is also re-checked to be inside the configured root. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(foundry_hosting): address PR-1 review comments - _shared.py:_capture_raw narrows `except Exception` to `except TypeError` and emits a WARNING with traceback so the lossy fallback to a synthesized round-trip is observable. Mirrors the reviewer suggestion. - _history_provider.py:save_messages narrows `except Exception` to `except FoundryStorageError` so only storage-validation failures (4xx/5xx, opaque server errors) are swallowed. Network / TLS / auth / payload-builder bugs propagate so the caller can retry / alert. Adds an instance-level `failed_writes` counter operators can poll for silent-drop visibility. - _history_provider.py id-stamping loop: drops the `contextlib.suppress(AttributeError, TypeError)` around `item.id = new_id` so SDK contract changes surface in the test suite instead of silently corrupting the chain (the storage backend rejects the entire `create_response` with HTTP 500 when synthetic prefix-based ids leak through). `import contextlib` removed. - tests: * Unit-cover `foundry_response_id` / `foundry_response_id_factory` / `foundry_item_id` so SDK `IdGenerator` contract changes are caught locally. * Cover the `save_messages` wire payload: required-by-storage fields (`background`, `parallel_tool_calls`, `instructions`, `agent_reference`), env-var-driven stamping (`FOUNDRY_AGENT_NAME` / `FOUNDRY_AGENT_VERSION` / `FOUNDRY_AGENT_SESSION_ID` / `MODEL_DEPLOYMENT_NAME` with `AZURE_AI_MODEL_DEPLOYMENT_NAME` fallback), and the rule that `model` / `agent_session_id` / `agent_reference.version` are omitted (not stamped to `None`) when their env vars are unset. * Cover the `FOUNDRY_AGENT_SESSION_ID` last-resort chain anchor on both the get and save paths, including the prefix gate that blocks non-`caresp_*`/`resp_*` values from reaching storage, and the precedence rule that a host binding wins over the env. * Replace the old `test_save_messages_swallows_backend_errors` with two tests asserting the new contract: storage errors are swallowed and bump `failed_writes`; everything else propagates and leaves the counter at zero. 141 unit tests pass; mypy + pyright + ruff clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(foundry_hosting): address PR-1 round-2 review comments - Hosted detection now delegates to AgentConfig.from_env().is_hosted so a future Foundry SDK rename of FOUNDRY_HOSTING_ENVIRONMENT propagates automatically; drop the local _ENV_FOUNDRY_HOSTING_ENVIRONMENT constant. - Drop the FOUNDRY_AGENT_SESSION_ID fallback in both get_messages and save_messages: per the SDK it identifies the *container instance*, not the conversation, so chaining off it would silently merge unrelated conversations across container restarts. The host-bound previous_response_id (set by ResponsesChannel) is the only authoritative anchor; the env value is still stamped into the persisted envelope's agent_session_id for operator correlation. - Update module docstring + replace TestFoundryAgentSessionIdAnchor with assertions for the new contract (env var ignored as anchor, still stamped onto persisted envelope, host binding wins). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor(foundry_hosting): reconcile with upstream main (#5851, #5666) Brings the FoundryHostedAgentHistoryProvider refactor branch back into sync with the foundry_hosting changes that have landed on upstream main since PR-1 was opened: * #5851 (path traversal in checkpoint storage, CWE-22). The workflow-host code in ``_responses.py`` builds a ``FileCheckpointStorage`` from a caller-controlled ``context_id`` (``previous_response_id`` / ``conversation_id`` / ``response_id``). Switch both call sites to route through ``_checkpoint_storage_for_context``, which rejects separators, NUL bytes, drive letters, absolute paths, and all-dot segments, and enforces ``is_relative_to(root)`` before any directory is created. * #5666 (function approval flow). Make the SDK-Item → AF-Message conversion helpers in ``_shared.py`` async and accept an optional ``approval_storage`` keyword: - ``_items_to_messages`` / ``_item_to_message`` / ``_item_to_message_inner`` - ``_output_items_to_messages`` / ``_output_item_to_message`` / ``_output_item_to_message_inner`` For ``mcp_approval_request`` / ``mcp_approval_response`` items the helpers now load the original function-call Content from the approval storage (via ``ApprovalStorage.load_approval_request``) instead of synthesising a placeholder. This matches upstream semantics and lets approval round-trips reconstruct the real payload. The ``ApprovalStorage`` Protocol moves to ``_shared.py`` so the conversion helpers can reference it without pulling in ``_responses.py`` (which would create a circular import). The concrete ``InMemoryFunctionApprovalStorage`` and ``FileBasedFunctionApprovalStorage`` stay in ``_responses.py`` next to the host that owns them, and re-export ``ApprovalStorage`` from ``_shared`` for compatibility. The workflow-host streaming path passes its own ``self._approval_storage`` into ``_to_outputs`` so approval requests are saved at emit time. * Bump ``_history_provider.FoundryHostedAgentHistoryProvider.get_messages`` to ``await`` the now-async ``_output_items_to_messages`` call. No public API change beyond the new keyword-only ``approval_storage`` parameter on the four conversion entry points. Validation: - uv run poe check-packages -P foundry_hosting (lint + pyright clean) - uv run poe mypy -P foundry_hosting (clean) - uv run poe test -P foundry_hosting (183 passed, 1 skipped) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Eduard van Valkenburg (eavanvalkenburg) added a commit
that referenced
this pull request
Jun 17, 2026
…5637) * refactor(foundry_hosting): build FoundryHostedAgentHistoryProvider on azure.ai.agentserver SDK Rebuilds the Foundry hosted-agent history provider on top of ``azure.ai.agentserver``'s ``FoundryStorageProvider`` instead of the in-house ``_HttpStorageBackend``. Splits the monolithic ``_responses.py`` into focused modules: - ``_history_provider.py`` — new ``FoundryHostedAgentHistoryProvider`` that talks to the SDK's ``FoundryStorageProvider``, threads ``response_id`` / ``previous_response_id`` through ``ContextVar``s via ``bind_request_context``, and lifts host-bound isolation keys (``x-agent-{user,chat}-isolation-key``) from the optional ``agent_framework_hosting`` package into a provider-local ``IsolationContext`` so the storage layer carries the correct partition keys without channels having to know about them. - ``_shared.py`` — extracts all SDK ``Item`` / ``OutputItem`` ↔ framework ``Message`` conversion helpers into one place so both ``_responses.py`` and the new history provider can share them. Restores ``_convert_file_data`` for inline ``input_file`` payloads, and the hosted-MCP routing for ``custom_tool_call_output`` items whose ``call_id`` carries the ``mcp_*`` prefix. - ``_ids.py`` — shared id helpers. - ``_responses.py`` — shrinks ~700 lines, re-exports converters for back-compat with existing tests. - ``tests/test_history_provider.py`` — exercises the new provider against a fake SDK backend; the host-isolation test is gated on the optional ``agent_framework_hosting`` import. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * feat(foundry_hosting): add local_storage_root for file-based dev history Adds an optional `local_storage_root: str | Path | None` parameter to `FoundryHostedAgentHistoryProvider`. When set and the provider is running outside a Foundry Hosted Agent container, conversations are persisted to JSONL files via `agent_framework.FileHistoryProvider` laid out as: {root}/{user_key or '~none'}/{chat_key or '~none'}/{session_id}.jsonl Hosted mode (FOUNDRY_HOSTING_ENVIRONMENT set) ignores the option with a one-time INFO log so Foundry storage always wins on the platform. The in-memory fallback is unchanged when the option is omitted. Path safety: isolation segments are validated against the same character allowlist FileHistoryProvider uses for session-id stems and base64-url-encoded with a reserved "~iso-" prefix when unsafe. "~none" sentinel for missing keys can never collide with a real isolation key (real keys starting with "~" are encoded). The resolved target dir is also re-checked to be inside the configured root. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(foundry_hosting): address PR-1 review comments - _shared.py:_capture_raw narrows `except Exception` to `except TypeError` and emits a WARNING with traceback so the lossy fallback to a synthesized round-trip is observable. Mirrors the reviewer suggestion. - _history_provider.py:save_messages narrows `except Exception` to `except FoundryStorageError` so only storage-validation failures (4xx/5xx, opaque server errors) are swallowed. Network / TLS / auth / payload-builder bugs propagate so the caller can retry / alert. Adds an instance-level `failed_writes` counter operators can poll for silent-drop visibility. - _history_provider.py id-stamping loop: drops the `contextlib.suppress(AttributeError, TypeError)` around `item.id = new_id` so SDK contract changes surface in the test suite instead of silently corrupting the chain (the storage backend rejects the entire `create_response` with HTTP 500 when synthetic prefix-based ids leak through). `import contextlib` removed. - tests: * Unit-cover `foundry_response_id` / `foundry_response_id_factory` / `foundry_item_id` so SDK `IdGenerator` contract changes are caught locally. * Cover the `save_messages` wire payload: required-by-storage fields (`background`, `parallel_tool_calls`, `instructions`, `agent_reference`), env-var-driven stamping (`FOUNDRY_AGENT_NAME` / `FOUNDRY_AGENT_VERSION` / `FOUNDRY_AGENT_SESSION_ID` / `MODEL_DEPLOYMENT_NAME` with `AZURE_AI_MODEL_DEPLOYMENT_NAME` fallback), and the rule that `model` / `agent_session_id` / `agent_reference.version` are omitted (not stamped to `None`) when their env vars are unset. * Cover the `FOUNDRY_AGENT_SESSION_ID` last-resort chain anchor on both the get and save paths, including the prefix gate that blocks non-`caresp_*`/`resp_*` values from reaching storage, and the precedence rule that a host binding wins over the env. * Replace the old `test_save_messages_swallows_backend_errors` with two tests asserting the new contract: storage errors are swallowed and bump `failed_writes`; everything else propagates and leaves the counter at zero. 141 unit tests pass; mypy + pyright + ruff clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(foundry_hosting): address PR-1 round-2 review comments - Hosted detection now delegates to AgentConfig.from_env().is_hosted so a future Foundry SDK rename of FOUNDRY_HOSTING_ENVIRONMENT propagates automatically; drop the local _ENV_FOUNDRY_HOSTING_ENVIRONMENT constant. - Drop the FOUNDRY_AGENT_SESSION_ID fallback in both get_messages and save_messages: per the SDK it identifies the *container instance*, not the conversation, so chaining off it would silently merge unrelated conversations across container restarts. The host-bound previous_response_id (set by ResponsesChannel) is the only authoritative anchor; the env value is still stamped into the persisted envelope's agent_session_id for operator correlation. - Update module docstring + replace TestFoundryAgentSessionIdAnchor with assertions for the new contract (env var ignored as anchor, still stamped onto persisted envelope, host binding wins). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * refactor(foundry_hosting): reconcile with upstream main (#5851, #5666) Brings the FoundryHostedAgentHistoryProvider refactor branch back into sync with the foundry_hosting changes that have landed on upstream main since PR-1 was opened: * #5851 (path traversal in checkpoint storage, CWE-22). The workflow-host code in ``_responses.py`` builds a ``FileCheckpointStorage`` from a caller-controlled ``context_id`` (``previous_response_id`` / ``conversation_id`` / ``response_id``). Switch both call sites to route through ``_checkpoint_storage_for_context``, which rejects separators, NUL bytes, drive letters, absolute paths, and all-dot segments, and enforces ``is_relative_to(root)`` before any directory is created. * #5666 (function approval flow). Make the SDK-Item → AF-Message conversion helpers in ``_shared.py`` async and accept an optional ``approval_storage`` keyword: - ``_items_to_messages`` / ``_item_to_message`` / ``_item_to_message_inner`` - ``_output_items_to_messages`` / ``_output_item_to_message`` / ``_output_item_to_message_inner`` For ``mcp_approval_request`` / ``mcp_approval_response`` items the helpers now load the original function-call Content from the approval storage (via ``ApprovalStorage.load_approval_request``) instead of synthesising a placeholder. This matches upstream semantics and lets approval round-trips reconstruct the real payload. The ``ApprovalStorage`` Protocol moves to ``_shared.py`` so the conversion helpers can reference it without pulling in ``_responses.py`` (which would create a circular import). The concrete ``InMemoryFunctionApprovalStorage`` and ``FileBasedFunctionApprovalStorage`` stay in ``_responses.py`` next to the host that owns them, and re-export ``ApprovalStorage`` from ``_shared`` for compatibility. The workflow-host streaming path passes its own ``self._approval_storage`` into ``_to_outputs`` so approval requests are saved at emit time. * Bump ``_history_provider.FoundryHostedAgentHistoryProvider.get_messages`` to ``await`` the now-async ``_output_items_to_messages`` call. No public API change beyond the new keyword-only ``approval_storage`` parameter on the four conversion entry points. Validation: - uv run poe check-packages -P foundry_hosting (lint + pyright clean) - uv run poe mypy -P foundry_hosting (clean) - uv run poe test -P foundry_hosting (183 passed, 1 skipped) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_requestand expect afunction_approval_response. These two types do not map to any type in the Foundry Hosted Agent layer.This PR maps
function_approval_requestandfunction_approval_responsetomcp_approval_requestandmcp_approval_responserespectively, allowing tools that require approval to work when the agent is hosted.The flow goes like:
function_approval_requestmcp_approval_requestand saves the original request for round trip look up.mcp_approval_responsewith the same ID as the request.Contribution Checklist