Repository navigation
Python: Isolate run-level tool approval state in agent tools - #10226
Conversation
Agent.as_tool(propagate_session=True) keeps the session state of the parent's ToolApprovalMiddleware instances out of the delegated agent. It derived those state keys only from the middleware configured on the agent, so a middleware instance passed to a single run was not isolated. ToolApprovalMiddleware now claims its source_id for the run, and the agent unions that claim with the keys declared by its own middleware. A run-level middleware therefore gets the same isolation, and the same distinct source_id requirement, as a constructor-configured one. Claims only add isolated keys and are scoped to one invocation. Adds regression tests for streaming and non-streaming runs, parent failure after delegation, pending approval requests, and per-invocation ownership, and updates the function-calling loop specification row.
GitHub Actions
Inactive
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open issues
What changed in this PR
This PR fixes Agent.as_tool(propagate_session=True) session-sharing behavior so that ToolApprovalMiddleware passed per-run (not just configured on the agent) has its approval-state key treated as run-owned/isolated and properly participates in distinct-source_id enforcement.
Changes:
- Add run-scoped claiming of
ToolApprovalMiddleware.source_idinto the existing agent-tool ownership plumbing. - Union per-run claimed approval source IDs into the agent’s declared approval ownership during
_prepare_run_context. - Expand specs/docs and add regression tests for streaming/non-streaming and edge cases (abort after delegation, pending approvals, per-invocation scoping).
| File | Description |
|---|---|
| python/packages/core/agent_framework/_harness/_tool_approval.py | Run-level ToolApprovalMiddleware now “claims” its source_id into function-invocation kwargs for downstream isolation. |
| python/packages/core/agent_framework/_agents.py | _prepare_run_context consumes and unions the per-run claim with agent-declared approval ownership, keeping the raw claim out of general kwargs. |
| python/packages/core/tests/core/test_agents.py | Adds/extends regression tests covering run-level approval isolation across streaming modes and failure/pending scenarios. |
| docs/specs/004-python-function-calling-loop.md | Updates the spec to document run-level middleware source_id distinctness and isolation behavior. |
Review effort: Lite
GitHub Actions
Inactive
agent-framework-automation
Bot
added
documentation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: No findings
Scope: full PR (1 commit(s)): f82ab6c1e5a4
Model: gpt-5.6-sol
Overview
This PR propagates run-level ToolApprovalMiddleware ownership through a private invocation channel, strips that metadata before MCP and tool-visible kwargs, and unions it with configured middleware ownership before delegated calls. Per-run context copies, pre-execution overlap checks, excluded-state merging, and streaming/non-streaming regression coverage constrain the new behavior. No PR-introduced Critical, High, or Medium defect remains after reconciliation.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
GitHub Actions
Inactive
Normalize the private approval-source claim to a typed frozenset at each reader and combine claims with a frozenset union. Only a frozenset is a claim, as for the other readers of this channel; any other value is no claim. Adds a regression test that caller-supplied claims of any shape neither break a run nor weaken the approval-state isolation of run-level middleware.
GitHub Actions
Inactive
Motivation & Context
Agent.as_tool(propagate_session=True)keeps the session state of the parent'sToolApprovalMiddlewareinstances out of the delegated agent. It derived those state keys only from the middleware configured on the parent agent. AToolApprovalMiddlewarepassed to a singlerun()was therefore not isolated: its state key propagated to the delegated agent like ordinary application state, and the documented requirement of distinctsource_idvalues was not enforced for it.Description & Review Guide
ToolApprovalMiddlewareclaims itssource_idfor the run through the private function-invocation kwarg that already carries parent approval-state ownership toas_tool.RawAgent._prepare_run_contextunions that claim with the keys declared by the agent's own middleware before the function loop passes ownership on. The claim is per invocation, only adds isolated keys, and is removed from the kwargs that tools and MCP header providers can read.as_toolitself is unchanged. Theas_tooldocstring and the function-calling loop specification (one phrase and the "Agent-tool child approval" row) are updated.source_idtest now also covers middleware passed to a single run.source_id(tool_approval) was already isolated.source_idequal to the delegated agent's now fails before the delegated agent runs, as the constructor-configured case already does by contract.as_toolownership code planned for removal in Python: RemoveAgent.as_tool()in vNext #8950 and should be deleted together with it.Related Issue
Relates to #8950 (planned removal of
Agent.as_tool()). This is a minimal interim correctness change to existing behavior; it does not extend or redesignas_tool.AI Assistance
AI assistance details: Implementation, tests, specification update, and this PR description were prepared with GitHub Copilot.
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.