Skip to content

Python: Harden function calling loop and approval replay - #7345

Open
eavanvalkenburg wants to merge 8 commits into
microsoft:mainfrom
eavanvalkenburg:python-approval-resume-contract
Open

Python: Harden function calling loop and approval replay#7345
eavanvalkenburg wants to merge 8 commits into
microsoft:mainfrom
eavanvalkenburg:python-approval-resume-contract

Conversation

@eavanvalkenburg

@eavanvalkenburg eavanvalkenburg commented Jul 27, 2026

Copy link
Copy Markdown
Member

Motivation & Context

Python function approval resume currently has several coupled transcript failures:

  • tools executed while resolving approval do not consistently return or stream their terminal result;
  • approval normalization mutates caller-owned messages and earlier returned responses;
  • reused call_id values can restore duplicate calls or attach results to the wrong logical occurrence;
  • service-side continuation drops the new approval response together with the already-stored request;
  • append-only history can replay stale approval control items;
  • reasoning content bound to an active function call must remain replayable with the call and result;
  • compaction can separate non-adjacent call/result pairs;
  • provider-injected tools can be rejected before before_run registers them;
  • providers can expose unanswered calls or approval requests after an invocation limit;
  • AG-UI confirm_changes snapshots can attach unrelated function results.

These failures can leave UI tool calls without results, produce provider-invalid histories, prevent approved calls
from resuming, or execute stale approval authority on a later turn.

Description & Review Guide

  • What are the major changes?
    • Make approval resume an immutable boundary by normalizing private message copies.
    • Return approved and rejected terminal results before final assistant output in streaming and non-streaming modes.
    • Correlate approval calls/results by ordered occurrence so completed rounds may safely reuse call_id.
    • Keep every content produced by one approved execution in the same occurrence group, including multiple
      user-input requests.
    • Apply the same grouped-result contract in AG-UI and count every executed approval group against
      max_function_calls, including user-input pauses.
    • Consume resolved approval responses during explicit stateless replay and filter approval wrappers from later
      HistoryProvider model replay.
    • Preserve current hosted approval decisions under service-side continuation while continuing to omit stored
      approval requests.
    • Resolve approval responses before checking the model-iteration budget.
    • Simplify FunctionInvocationLayer by splitting approval/model processing, naming the stream/non-stream loops,
      removing dead argument plumbing and closures, and centralizing shared loop policy.
    • Keep non-adjacent reasoning/call/result groups atomic during compaction.
    • Defer provider-injected approvals to in-run execution, preserving exactly one AG-UI result event.
    • Remove calls and approval requests returned after max_function_calls or max_iterations, while preserving
      metadata-only stream updates.
    • Preserve declaration-only request metadata without replaying streamed function arguments into the final response.
    • Correlate AG-UI confirm_changes snapshots to the original function_call_id without joining unrelated results.
    • Add the Python function-calling-loop specification with provider-neutral diagrams, a scenario-to-test matrix,
      the remaining service-owned follow-up, and core-team coordination requirements.
  • What is the impact of these changes?
    • Approved tools execute exactly once; rejected tools execute zero times and return one rejection result.
    • Approval-time user-input pauses return every request produced by the tool without another model call.
    • Caller messages and prior responses remain unchanged.
    • Streaming, streaming finalization, and non-streaming responses expose the same logical approval turn.
    • Local/stateless history retains reasoning, calls, and results as provider-valid logical groups.
    • Compaction and AG-UI snapshots retain the correct call/result relationship.
    • Post-limit provider output cannot create an orphaned call or approval prompt.
    • No public API signatures are changed.
  • What do you want reviewers to focus on?
    • Occurrence-aware approval correlation and placeholder replacement.
    • Approval result ordering and budget/error-limit behavior in both response modes.
    • History filtering versus immediate hosted-service approval transmission.
    • Non-adjacent compaction grouping and post-limit output filtering.
    • Provider-injected approval execution and confirm_changes snapshot correlation.
    • Whether the specification covers every scenario that should gate future changes.

Related Issue

Fixes #7241
Fixes #7304
Fixes #7125
Fixes #7212
Fixes #7043
Fixes #7045
Fixes #6828
Fixes #6973

This PR supersedes #7243, #7326, #7133, #7244, #7091, #7334, #7316, and #7110 by consolidating their fixes with
rejected/non-streaming parity, immutable inputs, occurrence-aware multi-round replay, history semantics, and the
shared validation specification. It preserves the substantive contributions from #7244 and #7091, incorporates the
intent of #7334 for both invocation limits, and corrects #7316 by selecting only the result identified by the
original function_call_id. It also incorporates #7110's metadata-only declaration streaming approach and preserves
the contributor's attribution in the implementation commit.

#6851 remains a separate service-owned follow-up because proving that a terminal approval cannot execute again
through previous_response_id requires hosted-service continuation coverage rather than a Python core or adapter
transcript fix.

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 a language prefix) — a workflow keeps the label and title prefix in sync automatically.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 1ee1d250-d1e2-4c6f-8c36-aae0d94fe7a1
@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/ag-ui/agent_framework_ag_ui
   _agent_run.py114511889%184–191, 238–239, 246, 355, 359, 361, 378, 405–406, 519, 533, 537, 541, 544, 549, 554, 563, 566, 573–579, 612, 624, 635, 638, 673, 727–731, 796, 811, 814, 816, 842, 868–870, 928, 930, 932, 935–939, 953, 961–966, 975–976, 1025–1028, 1039, 1047, 1079, 1094, 1108, 1120, 1150, 1154, 1157, 1159, 1199–1201, 1265, 1271–1272, 1277, 1281–1282, 1480, 1488, 1516, 1520, 1565, 1624, 1654–1655, 1777, 1923, 1983, 2000, 2020–2021, 2028, 2136, 2164, 2172, 2174, 2177, 2183, 2238, 2241, 2251–2252, 2259, 2305
packages/core/agent_framework
   _compaction.py8116192%129–130, 138, 174, 268–269, 287–288, 305, 340, 354, 361, 376–377, 431, 438, 454, 504, 539, 541, 561, 603, 659, 665, 667, 686, 730–735, 747, 831, 833, 848, 893, 955, 1081, 1087–1091, 1094–1096, 1104, 1179, 1181, 1197, 1204, 1209, 1224, 1232, 1331, 1354, 1366, 1462, 1489, 1573
   _sessions.py5124191%128–130, 132–133, 150–151, 153–155, 233–234, 367, 648–652, 667, 697, 734–735, 749, 751, 770, 772, 855, 861, 895, 966, 970, 980, 1120, 1136, 1269, 1283–1284, 1307, 1329, 1339, 1381
   _tools.py12827893%231–232, 409, 411, 424, 449–451, 459, 477, 491, 498, 505, 528, 530, 537, 545, 680, 714–716, 719–721, 723, 729, 780–782, 807, 833, 837, 875–877, 881, 1054, 1066, 1073–1076, 1097, 1105, 1119–1121, 1491, 1574, 1602, 1624, 1676–1677, 1733, 1777, 1784–1785, 1873, 1935–1936, 1947, 2048, 2062, 2065, 2078, 2081, 2104, 2111, 2120, 2124, 2149, 2244, 2272, 2312, 2315, 2372, 2498, 3062
   _types.py12329692%61, 70–71, 125, 134, 153, 155, 159, 163, 165, 167, 169, 187, 191, 217, 239, 244, 249, 253, 283, 709–710, 891–892, 1350, 1425, 1460, 1480, 1490, 1684–1686, 1971–1976, 2001, 2056, 2061, 2071, 2079, 2086–2090, 2108, 2181, 2194, 2199, 2312, 2335, 2596, 2620, 2719, 2900–2901, 3003, 3232, 3285, 3304, 3343, 3354, 3356–3360, 3362, 3365–3373, 3383, 3472, 3609, 3614, 3619, 3624, 3628, 3714–3716, 3745, 3833–3837
packages/openai/agent_framework_openai
   _chat_client.py137011791%312, 325, 675–679, 687–690, 696–700, 750–757, 759–761, 768–770, 828, 836, 859, 1076, 1135, 1137, 1139, 1141, 1207, 1221, 1301, 1311, 1316, 1359, 1471–1472, 1487, 1760, 1867, 1872–1873, 1956, 1966, 1993, 1999, 2009, 2015, 2020, 2026, 2031–2032, 2112, 2156, 2159–2162, 2176, 2186–2187, 2199, 2241, 2306, 2323, 2326, 2353–2355, 2394, 2411, 2414, 2476, 2483, 2520–2521, 2556, 2594–2595, 2613–2614, 2786–2787, 2805, 2891–2899, 3077, 3092, 3142, 3152–3154, 3181–3183, 3193–3194, 3200, 3215, 3348–3349
TOTAL45851446690% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
9469 34 💤 0 ❌ 0 🔥 1m 59s ⏱️

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.

Pull request overview

This PR overhauls the Python function-invocation/approval-resume flow to make approval resume immutable, stream and return terminal tool results consistently, correlate approvals/results by occurrence (even when call_id is reused), and prevent approval control wrappers from being replayed into later model turns. It also updates OpenAI Responses API continuation handling so service-side storage skips replaying approval requests while still sending new approval responses, and adds a normative function-calling loop specification plus expanded regression coverage.

Changes:

  • Refactors the core function-invocation layer to split approval-resolution vs model tool-call processing, enforce immutable normalization, and unify streaming/non-streaming semantics.
  • Adds extensive regression tests for approval resume ordering, replay/occurrence correlation, history filtering, limits/termination behavior, and OpenAI service-side continuation approval serialization.
  • Introduces a Python function-calling loop specification and links it from the Python AGENTS docs.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
python/packages/openai/tests/openai/test_openai_chat_client.py Updates tests to assert approval requests are stripped but approval responses are preserved under service-side storage continuation.
python/packages/openai/AGENTS.md Documents service-side continuation rules for approval request vs response serialization.
python/packages/openai/agent_framework_openai/_chat_client.py Ensures function_approval_response is serialized even when request_uses_service_side_storage=True, while function_approval_request is skipped.
python/packages/core/tests/core/test_harness_tool_approval.py Adds end-to-end approval-resume tests for immutability, reasoning replay grouping, history filtering, user-input pauses, and iteration-budget interactions.
python/packages/core/tests/core/test_function_invocation_logic.py Strengthens assertions around approval processing immutability/inertness and adds targeted unit tests for replay deduplication and occurrence-aware correlation.
python/packages/core/AGENTS.md Documents the new approval-resume invariants and history filtering expectations.
python/packages/core/agent_framework/_tools.py Major refactor: occurrence-aware approval normalization, explicit approval-resolution step, streamlined loop helpers, and shared policy for limits/errors/termination.
python/packages/core/agent_framework/_sessions.py Filters approval request/response wrappers out of history replay before later model calls.
python/packages/ag-ui/agent_framework_ag_ui/_agent_run.py Aligns with updated _try_execute_function_calls signature (removes unused attempt_idx).
python/AGENTS.md Adds guidance that function-calling loop changes must follow the new spec and require extra validation.
docs/specs/004-python-function-calling-loop.md Adds the function-calling loop contract, diagrams, scenario→test matrix, and validation checklist.

Comment thread python/packages/core/agent_framework/_tools.py Outdated
jstar0 and others added 2 commits July 27, 2026 14:32
…ution

When user approves a mix of static and provider-injected tools, partition them
by tool map membership. Execute only static tools (available now). Keep deferred
provider-injected tool approvals in fcc_todo but don't add results for them.

This allows _replace_approval_contents_with_results to find them in fcc_todo
but without results, so they stay as approval responses in messages (not marked
as rejected). The harness then handles them during agent.run() when provider
tools are registered.

The previous approach removed them from fcc_todo, causing them to be treated as
rejected ("Tool call invocation was rejected by user").

Add an end-to-end test that registers a real provider-injected tool via a
ContextProvider and drives the full pause/approve/resume flow, asserting the
approved side effect runs without any rejection or transport-failure result.

Fixes microsoft#7043.

@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.

Automated Code Review

Reviewers: 4 | Confidence: 80% | Result: All clear

Reviewed: Security Reliability, Test Coverage, Failure Modes, Design Approach


Automated review by eavanvalkenburg's agents

Preserve non-adjacent and provider-injected approval behavior while filtering post-limit tool content and correlating AG-UI confirmation snapshots to the original call.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1ee1d250-d1e2-4c6f-8c36-aae0d94fe7a1
Keep every content produced by one approved function execution together through approval occurrence correlation, including multiple user-input requests and reused call IDs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1ee1d250-d1e2-4c6f-8c36-aae0d94fe7a1
@eavanvalkenburg
eavanvalkenburg marked this pull request as ready for review July 27, 2026 13:49

@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.

Automated Code Review

Reviewers: 5 | Confidence: 84%

✓ Correctness

No actionable issues found in this dimension.

✓ Security Reliability

This is a large, well-structured PR that hardens the Python function-calling loop. The security-relevant changes (approval validation, immutable message copies, occurrence-aware correlation, history filtering) are generally sound. The deferred_approval_ids set in the AG-UI transport is populated but never consumed, which is dead code but not a bug since the deferred approvals are implicitly left in-place by the existing _replace_approval_contents_with_results fallback logic. The use of id() for object identity tracking in _collect_approval_responses is safe within the function scope. One minor reliability concern exists around the shallow copy in _copy_messages_for_function_invocation — Content objects are shared between the copied and original message lists, meaning mutations to Content attributes on the 'private copy' (like user_input_request = True) technically leak back. However, in practice this only occurs for freshly-created model response content (not caller-provided), so the stated immutability contract holds for caller inputs.

✓ Test Coverage

This is a well-tested PR with comprehensive coverage for the refactored function-calling loop. The spec, tests, and implementation are well-aligned. The main gap is missing dedicated test coverage for the is_follow_up_request path in _collect_approval_responses, which is a new way to consume approval authority (via user_input_request content in replayed history), distinct from the terminal function_result path that has explicit coverage. All other new behaviors (deferred provider-injected tools, non-adjacent compaction pairing, unexecutable tool content dropping, immutable approval boundaries) have appropriate test coverage through either direct unit tests or integration tests. The PR has extensive test coverage for its core changes: occurrence-based approval correlation, non-adjacent compaction grouping, post-limit output filtering, and history inertness of resolved approvals. The refactored _replace_approval_contents_with_results has dedicated tests for reused call_ids, multi-content groups, placeholder replacement, and deduplication. The new streaming/non-streaming parametrized tests cover middleware termination and error-limit edge cases. One moderate gap exists: the standard streaming approval-resume happy path (approve/reject → results yielded → model text) lacks a dedicated test at the FunctionCallingChatClient level, though it is covered at the Agent/harness level. New behaviors — post-limit content filtering, approval resume immutability, reasoning replay, history filtering, user input propagation, middleware termination, error limits, and the OpenAI approval response serialization change — all have dedicated tests with meaningful assertions. One minor gap: the error-limit approval test does not explicitly assert the model wasn't re-invoked (unlike the middleware-termination test which does), though the assertion on fallback text implicitly guards against it.

✓ Failure Modes

The changes are well-structured and the failure mode handling is generally solid. The occurrence-aware approval correlation, compaction linking, and provider-injected tool deferral all handle their error/edge cases correctly. The only notable dead code is deferred_approval_ids which is populated but never consumed — a minor cleanup omission, not a failure path. The exception-swallowing pattern in AG-UI _resolve_approval_responses (line 1344-1346) is pre-existing and correctly produces per-approval error results downstream. The _filter_approval_control_messages uses shallow copy appropriately.

✓ Design Approach

I found one design-level correctness issue in the AG-UI approval-resume path: it still assumes each approved tool produces exactly one terminal function_result, so an approved tool that pauses again for user input is converted into a synthetic failure instead of surfacing its follow-up request(s). I did not find a second well-supported issue in the files shown here. I found one design issue in the new approval-resume loop: it undercounts executed approved tools when the tool returns user-input request content instead of a function_result, so max_function_calls no longer enforces its documented "total number of individual function invocations" invariant on that path.


Automated review by eavanvalkenburg's agents

Comment thread python/packages/ag-ui/agent_framework_ag_ui/_agent_run.py Outdated
Comment thread python/packages/core/agent_framework/_tools.py Outdated
Keep approval-time follow-up requests grouped through the AG-UI adapter and count every executed result group against the function-call budget in both response modes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1ee1d250-d1e2-4c6f-8c36-aae0d94fe7a1
@eavanvalkenburg

Copy link
Copy Markdown
Member Author

Addressed the latest automated review in af5572f. In addition to the two inline fixes, this adds direct coverage for follow-up requests consuming only their matching approval occurrence, FunctionInvocationLayer streaming approve/reject result ordering, and the approval error-limit path making exactly the required final no-tool model call. The unused AG-UI deferred approval id set was removed. No change was made for the shallow-copy observation because caller-owned Content is not mutated on that path, while deep-copying all Content would add cost and undermine the identity correlation used by approval normalization.

Add focused regressions for grouped execution failure fallback and malformed confirm_changes snapshot metadata, bringing PR-added AG-UI executable lines to full line coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1ee1d250-d1e2-4c6f-8c36-aae0d94fe7a1
@eavanvalkenburg

Copy link
Copy Markdown
Member Author

Added focused AG-UI defensive-path coverage in 19a8134: grouped executor failure fallback, non-list confirm_changes tool metadata, and malformed confirm_changes argument JSON. The PR-added executable lines in agent_framework_ag_ui/_agent_run.py are now 55/55 line-covered.

Emit metadata-only declaration updates after provider argument chunks and preserve request metadata during function-call aggregation so streamed arguments appear exactly once.

Co-authored-by: Kartik Madan <kartikmadan97@gmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1ee1d250-d1e2-4c6f-8c36-aae0d94fe7a1
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

4 participants