Python: Fail closed on unresolved agent-tool approvals - #8527
Eduard van Valkenburg (eavanvalkenburg) wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
🟡 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.
There was a problem hiding this comment.
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
0c96cb0 to
92b6a6d
Compare
2dffa94 to
8877aa2
Compare
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
ToolApprovalMiddlewarecan evaluate its existingauto_approval_rules.Agent.as_tool()policy with delayed Workflow approval.Related Issue
Fixes #4963
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.