Python: preserve approval context across resume paths - #8433
Evan Mattson (moonbox3) merged 9 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Stateless zero-response mixed resumes and adapter-local default invocation arguments are not handled correctly.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Preserves approval authority and tool runtime context across Python core and AG-UI resume paths, including dependent mixed-batch behavior from #8428.
Changes:
- Marks framework-created sessions as run-local for approval authority.
- Correlates mixed approval/Host pauses and preserves middleware behavior.
- Forwards filtered function invocation context through AG-UI.
File summaries
| File | Description |
|---|---|
python/packages/core/agent_framework/_agents.py |
Marks temporary context-provider sessions as run-local. |
python/packages/core/agent_framework/_tools.py |
Adds mixed-pause classification, persistence, and resolution. |
python/packages/core/agent_framework/_harness/_tool_approval.py |
Preserves mixed user-input batches through middleware. |
python/packages/core/tests/core/test_function_invocation_logic.py |
Tests mixed-batch ordering and correlation. |
python/packages/core/tests/core/test_harness_tool_approval.py |
Tests context-provider and middleware resume paths. |
python/packages/ag-ui/agent_framework_ag_ui/_agent.py |
Exposes function invocation kwargs. |
python/packages/ag-ui/agent_framework_ag_ui/_agent_run.py |
Threads runtime kwargs into approval execution. |
python/packages/ag-ui/AGENTS.md |
Documents AG-UI runtime-context handling. |
python/packages/ag-ui/tests/ag_ui/test_approval_result_event.py |
Tests filtered adapter-local kwargs. |
python/packages/ag-ui/tests/ag_ui/test_agent_wrapper_comprehensive.py |
Tests combined AG-UI/context-provider resume. |
docs/specs/004-python-function-calling-loop.md |
Specifies mixed-pause invariants and regressions. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 2
- 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 (4 commit(s)): 7f90bef3b0b4, 07aec0aa7985, 8b7ea1fa9f96, 59525434d012
Model: gpt-5.6-sol-fast
Overview
The review found 1 verified inline finding(s).
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: python/packages/core/agent_framework/_harness/_tool_approval.py
There was a problem hiding this comment.
MAF Automated Review — Iteration 2
Result: Findings reported
Scope: 13 net-new commit(s): f17597b09b04, c97528b13084, c030fa3582b1, b83b0e3b3b9d, c356df98a6c1, e24a1f2c408a, e1a901a5c73c, 5135f389ca47, ab8299beb0bd, 234236b5bb5a, 8d5f640ce9bf, 73d2e7dd3747, 16a0e1a03eca
Model: gpt-5.6-sol-fast
Overview
The change moves session provenance to invocation scope, preserves AG-UI function context with explicit precedence, and unifies mixed-pause response matching with strong identity, completeness, and ordering checks. Session-backed correlation and the immediate adjacent-assistant replay cases are well guarded by immutable pending state and focused regression tests. The remaining risk is in stateless batch discovery: completed split batches can be replayed, and valid mixed batches can be missed when a later request or non-assistant message interrupts the assistant-message run.
Reviewed the supplied incremental change set across correctness, security/reliability, architecture, and failure behavior.
3 verified findings remained after source verification (1 high, 2 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: python/packages/core/agent_framework/_tools.py
Motivation & Context
Tool approval resume has two context-sensitive gaps. A sessionless
Agent.run(...)silently drops a valid approval when any context provider causes the framework to create a fresh per-run session, while AG-UI adapter-local approval execution loses the documentedfunction_invocation_kwargsruntime context beforeagent.run(...)begins.This work builds on the mixed approval/Host correlation foundation merged in #8428 and includes the follow-up hardening identified during review.
Description & Review Guide
AgentSession, preserving caller-supplied/persistent fail-closed authority. Threadfunction_invocation_kwargsthrough the programmatic AG-UI entry points and merge adapter-local invocation context with wrapped-agent defaults and run options using normal precedence, without leaking unrelated run kwargs. Share one mixed-pause response matcher between session-backed and stateless paths, fail closed on zero/partial stateless responses across adjacent assistant messages, and preserve separately streamed Host input plus later approvals in model order.Related Issue
Builds on merged #8428.
Fixes #8410
Fixes #8132
No other open upstream pull request currently references either issue.
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.