Skip to content

Python: Fail closed on unresolved agent-tool approvals - #8527

Open
Eduard van Valkenburg (eavanvalkenburg) wants to merge 7 commits into
microsoft:mainfrom
eavanvalkenburg:fix-agent-as-tool-approval-policy
Open

Eduard van Valkenburg (eavanvalkenburg) wants to merge 7 commits into
microsoft:mainfrom
eavanvalkenburg:fix-agent-as-tool-approval-policy

Conversation

@eavanvalkenburg

Copy link
Copy Markdown
Member

Motivation & Context

When an agent exposed through Agent.as_tool() reaches a child tool that requires approval, the child approval cannot be safely resumed as part of the parent agent's function-calling loop. This change establishes a small, explicit boundary: delegated tools may use an immediate child-side approval policy, while approvals requiring interaction or a durable wait belong in a Workflow.

Description & Review Guide

  • What are the major changes?
    • Give each delegated invocation a child session so ToolApprovalMiddleware can evaluate its existing auto_approval_rules.
    • Fail closed with actionable guidance when a child function approval remains unresolved.
    • Preserve existing propagation for non-approval user-input requests.
    • Document the contract and add a sample comparing immediate Agent.as_tool() policy with delayed Workflow approval.
  • What is the impact of these changes?
    • Applications can approve delegated child tools through policy evaluated when the runtime function call occurs, including tools added dynamically by integrations such as MCP or Skills.
    • Unresolved child approvals no longer escape into a parent continuation that cannot safely resume them.
    • Interactive, delayed, durable, or externally coordinated approvals use Workflow pause and resume.
  • What do you want reviewers to focus on?
    • The boundary between immediate middleware policy and Workflow approval, and whether the focused streaming, non-streaming, and same-name regression coverage captures that contract.

Related Issue

Fixes #4963

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Propagated sessions retain unresolved child approval state, undermining the fail-closed boundary.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Establishes fail-closed approval handling for delegated Python agents.

Changes:

  • Adds private child sessions and immediate middleware approval policies.
  • Rejects unresolved child approvals with actionable guidance.
  • Adds regression tests, documentation, and a comparison sample.
File summaries
File Description
python/samples/03-workflows/tool-approval/agent_as_tool_vs_workflow_approval.py Demonstrates immediate versus workflow approval.
python/samples/03-workflows/README.md Lists the new sample.
python/packages/core/tests/core/test_agents.py Tests approval resolution and isolation.
python/packages/core/agent_framework/exceptions.py Clarifies user-input exception behavior.
python/packages/core/agent_framework/_agents.py Implements child-session and fail-closed handling.
docs/specs/004-python-function-calling-loop.md Documents the approval boundary and coverage.
Review details
  • Files reviewed: 6/6 changed files
  • Comments generated: 1
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread python/packages/core/agent_framework/_agents.py Outdated

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

MAF Automated Review — Iteration 1

Result: Findings reported
Scope: full PR (1 commit(s)): e8c71cf08398
Model: gpt-5.6-sol-fast

Overview

The change gives every delegated agent invocation a session, allowing child-side approval middleware to make immediate policy decisions, and it explicitly blocks unresolved child approvals from entering the normal parent transcript. The new default-private-session and same-name tests provide strong coverage for the non-propagating path. However, propagate_session=True still aliases core approval state into the parent and permits cross-agent approval confusion, while mixed approval and non-approval request batches lose the otherwise supported non-approval request.

Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (1 high, 1 medium) across 1 file. Details are attached to the affected lines below.

Affected areas: python/packages/core/agent_framework/_agents.py

Comment thread python/packages/core/agent_framework/_agents.py Outdated
Comment thread python/packages/core/agent_framework/_agents.py Outdated
Comment thread python/packages/core/agent_framework/_agents.py
Comment thread python/packages/core/agent_framework/_agents.py Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Usage: [Issues, PRs], Target: documentation in the code base and learn docs python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: Cannot approve tool usage from sub-agents

3 participants