Python: Add tool approval middleware#6414
Conversation
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
This PR improves Python tool-approval UX and correctness by fixing mixed tool-call batches (so only tools that truly require approval surface prompts) and by introducing an experimental, session-backed ToolApprovalMiddleware to coordinate queued approvals, standing approval rules, and heuristic auto-approval.
Changes:
- Adjusts the core function-invocation loop to hide and session-store approval requests for known non-approval-required tools when a mixed batch contains at least one approval-required tool, then reinjects them as approved responses when the approval flow continues.
- Adds experimental
ToolApprovalMiddleware+ state/rule models and helper APIs for “always approve this tool” / “always approve this tool with these arguments”. - Adds focused core tests plus a new sample demonstrating host/user policy decisions with the middleware.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| python/samples/02-agents/tools/tool_approval_middleware.py | Adds an end-to-end sample showing queued approvals, standing rules, and heuristic auto-approval. |
| python/samples/02-agents/tools/README.md | Documents the new sample. |
| python/packages/core/tests/core/test_harness_tool_approval.py | Adds tests for mixed-batch hiding/replay, queued approvals (streaming + non-streaming), auto-approval callbacks, and standing rules. |
| python/packages/core/AGENTS.md | Documents the new tool-approval harness concepts and APIs. |
| python/packages/core/agent_framework/_tools.py | Implements mixed-batch bypass storage/replay via session-backed state. |
| python/packages/core/agent_framework/_harness/_tool_approval.py | Introduces experimental ToolApprovalMiddleware, state/rule models, and “always approve” response helpers. |
| python/packages/core/agent_framework/init.py | Exposes the new middleware/types/helpers at the package top-level. |
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 90%
✓ Security Reliability
The tool approval middleware implementation is well-designed from a security and reliability perspective. The mixed-batch bypass correctly classifies tools: only tools explicitly NOT in the
approval_toolsset (i.e., notalways_require) and known to the tool_map are auto-approved. Unknown tools, declaration-only tools, and additional tools all require visible approval. Standing rules are added only via explicit user/host action. Session state merging correctly preserves the core's hidden auto-approvable requests across middleware saves. No injection risks, no unsafe deserialization, no secrets in code. The one notable concern is the unboundedwhile Trueloop in the middleware when all approval requests are auto-approved, though this is mitigated by the inner MAX_ITERATIONS cap and the experimental nature of the feature.
✓ Test Coverage
The test coverage for the new ToolApprovalMiddleware is solid for the core flows (queuing, mixed batches, streaming, auto-approval callbacks, and tool-level standing rules). However, the public helper
create_always_approve_tool_with_arguments_responsehas zero test coverage. This leaves the argument-scoped standing rule path—including_arguments_match, argument serialization, and the critical behavior that different arguments for the same tool should NOT be auto-approved—entirely unexercised.
✓ Failure Modes
The tool approval middleware and mixed-batch bypass are well-constructed with sound state management. The
_save_statemerge logic correctly preserves the core loop'sauto_approvable_approval_requestsacross middleware serialization cycles. The streaming and non-streaming paths both handle queuing, auto-approval, and rejection correctly. Object identity tracking in_remove_approval_requestsis safe because the Content references are drawn directly from the message lists. No silent failures, lost errors, or stale-state paths were found.
✗ Design Approach
The session-backed mixed-batch bypass mostly fits the stated goal, but it has one design hole: hidden never-require tool calls are replayed on the next session turn unconditionally, so abandoning the approval prompt and sending an unrelated follow-up can still execute stale hidden tool calls. That contradicts the new documented contract that replay happens only when the visible approval flow resumes.
Flagged Issues
- python/packages/core/agent_framework/_tools.py:250 replays hidden auto-approved requests on any later turn in the session, not specifically when the visible approval flow resumes, so stale hidden tool calls can execute during an unrelated follow-up message.
Automated review by eavanvalkenburg's agents
5d0e44b to
5a31c73
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
304d96b to
fa17528
Compare
Motivation and Context
Python currently surfaces approval prompts for every tool call in a mixed tool-call batch when any one tool requires approval. This can block safe read-only tools behind unnecessary approval prompts and makes host approval UX noisy.
Fixes #6385
Description
ToolApprovalMiddlewarewith standing approval rules, tool+arguments rules, heuristicauto_approval_rules, queued approval prompts, and streaming/non-streaming support.Contribution Checklist