Repository navigation
Python: Honor Foundry Toolbox approval metadata - #10136
Eduard van Valkenburg (eavanvalkenburg) merged 5 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
If prompt discovery fails during reconnect, shallow-copy rollback leaves retained tools with changed approval modes, potentially downgrading them from always requiring approval to never requiring it.
1 open issue
What changed in this PR
This PR makes Python Foundry Toolbox honor discovered approval metadata through a new callable MCP approval policy, while preserving ordinary MCP defaults and explicit caller overrides.
Changes:
- Add callable approval policies and refresh approval modes on retained functions.
- Parse Foundry approval metadata with conservative filter matching and validation.
- Add offline approval-flow regressions and document policy precedence and behavior.
| File | Description |
|---|---|
skills/agent-framework-builder-python/references/hosting-and-deployment.md |
Document Toolbox approval behavior for consumers. |
python/packages/foundry_hosting/tests/test_toolbox.py |
Cover policy precedence, refreshes, and approval flows. |
python/packages/foundry_hosting/tests/foundry_hosting/toolbox_approval_helpers.py |
Add offline MCP and model test helpers. |
python/packages/foundry_hosting/README.md |
Explain metadata filters and caller overrides. |
python/packages/foundry_hosting/pyproject.toml |
Enable pytest importlib mode. |
python/packages/foundry_hosting/agent_framework_foundry_hosting/_toolbox.py |
Parse metadata and expose the approval policy parameter. |
python/packages/core/AGENTS.md |
Document callable policy and discovery semantics. |
python/packages/core/agent_framework/_mcp.py |
Support callable policies and staged approval updates. |
Review effort: Balanced
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): d1cf616874ea
Model: gpt-5.6-sol
Overview
The PR adds callable per-discovery approval policies in Core and uses them to map Foundry Toolbox metadata, with strict result validation, conservative unmatched-filter behavior, staged per-catalog refreshes, and execution-path regression coverage. Three residual risks remain: the Foundry package still supports Core versions that do not evaluate callable policies, a downstream MCP server can self-assert the read-only hint used to bypass approval, and tool approval updates can escape a failed combined tool/prompt reconnect.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
3 verified findings remained after source verification (2 high, 1 medium) across 2 files. Details are attached to the affected lines below.
Affected areas: python/packages/core/agent_framework/_mcp.py, python/packages/foundry_hosting/agent_framework_foundry_hosting/_toolbox.py
There was a problem hiding this comment.
MAF Automated Review — Iteration 2
Result: No findings
Scope: 3 net-new commit(s): 21239115c4b5, 67e30274d06d, a7294b3cc8a0
Model: gpt-5.6-sol
Overview
The incremental change correctly makes failed MCP reconnects restore approval modes on retained function objects, including when prompt discovery fails after tool discovery has already refreshed policy. The restore runs even if connection cleanup raises, and the new regression covers policy changes in both directions plus a subsequent successful reconnect. No new publishable Critical, High, or Medium issue was established in this review range.
Reviewed the supplied incremental change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.

Motivation & Context
Foundry Toolbox discovery exposes approval configuration in each tool's
_meta.tool_configuration.require_approval, but Python ignored that configuration. A live hosted Responses reproduction confirmed that a tool configured withrequire_approval: "always"executed without anmcp_approval_request. An explicit local approval policy paused the same tool correctly.This fixes metadata-based approval selection before local tool execution, without interpreting Foundry-specific metadata for ordinary MCP servers.
Description & Review Guide
approval_modeand compose the Foundry metadata parser through that option rather than an inheritance hook. Callables receive the discoveredmcp.types.Toolormcp.types.Promptand must return"always_require"or"never_require"for every item, including their default decision. Invalid results, includingNone, fail discovery explicitly.FoundryToolbox.approval_modeis now an explicit, typed constructor parameter.Noneselects metadata-based approval. Explicit scalar, dictionary, or callable policies replace metadata handling entirely. Ordinary MCPNoneand static-policy behavior remain unchanged. Toolbox metadata supports"always","never", andalways/neverfilters using remote/original tool names andread_only. Filter criteria are combined,alwayswins overlaps, and unmatched filter-policy tools require approval. Missing metadata preserves the no-approval default; malformed metadata fails discovery instead of silently disabling approval. Callable policies refresh retained functions, with updates staged until the complete discovery snapshot validates. Failed refreshes preserve the previous snapshot and approval modes. Static policies preserve caller customizations on retained functions.Before, applications needed a blanket local override to guarantee an approval pause:
After, the default honors each discovered tool's Foundry configuration:
Use
approval_mode="never_require"to disable local Toolbox approval explicitly. Connection and credential ownership, approval persistence, and the function-loop protocol are unchanged. Streaming and non-streaming execution use the same policy; both pause before tool execution when approval is required. The implementation and callable-based composition were reviewed and approved by the maintainer during the live-reproduction follow-up. The final patch was validated locally; the patched package has not been redeployed to the live reproduction.Local validation:
uv run --directory python poe test -P corepassed (9,400 passed, 20 skipped, 2 xfailed);uv run --directory python poe test -P foundry_hostingpassed (1,092 passed, 36 integration tests deselected).uv run --directory python poe syntax -P core,uv run --directory python poe syntax -P foundry_hosting,uv run --directory python poe pyright -P core,uv run --directory python poe pyright -P foundry_hosting,uv run --directory python poe test-typing -P core, anduv run --directory python poe test-typing -P foundry_hostingall passed. Both test-typing runs passed all five configured checkers.git diff --checkpassed.Related Issue
No linked issue. This draft follows a reproduced bug and maintainer-reviewed implementation; issue linkage remains outstanding. No competing Python Toolbox approval-parsing PR was found.
AI Assistance
AI assistance details: Copilot assisted with investigation, implementation, regression tests, documentation, and this PR description.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.