Skip to content

Python: stream tool results for approval-resolution execution#7243

Open
he-yufeng wants to merge 3 commits into
microsoft:mainfrom
he-yufeng:fix/approval-tool-result-streaming
Open

Python: stream tool results for approval-resolution execution#7243
he-yufeng wants to merge 3 commits into
microsoft:mainfrom
he-yufeng:fix/approval-tool-result-streaming

Conversation

@he-yufeng

Copy link
Copy Markdown
Contributor

Fixes #7241.

Problem

When a tool is executed while resolving an approval (rather than in the model's normal tool-call turn), its function_result never reaches streaming consumers. The approval-resolution branch of _process_function_requests runs the approved calls and splices results into the local history for the next model call, but returns update_role=None / function_call_results=None — and the first _process_function_requests call site in the streaming loop has no yield gate. The approved tool fires its side effect invisibly: no TOOL_CALL_RESULT from the AG-UI emitter, and the result is absent from MESSAGES_SNAPSHOT, asymmetric with statically executed approvals (which AG-UI emits explicitly via _make_approval_tool_result_events).

Fix

  • The approval branch now returns update_role="tool" and the executed approved_function_results whenever calls actually ran (rejected-only approvals still stream nothing).
  • The first call site mirrors the yield gate the second call site already had, so approval-resolution results stream as a role="tool" update just like normal tool results.

Normal (non-approval) tool streaming is untouched.

Verification

  • New regression test test_tool_approval_resolution_streams_tool_results: approves a guarded tool over a streaming run and asserts the function_result appears in the updates. Fails without the fix (assert []), passes with it.
  • pytest packages/core/tests/core/test_harness_tool_approval.py packages/core/tests/core/test_tools.py packages/core/tests/core/test_function_invocation_logic.py — 211 passed, 2 skipped.
  • AG-UI approval tests (test_run.py -k approval) — 6 passed.
  • ruff check / format clean, pyright (package config) no new errors.

The approval-resolution branch of _process_function_requests executed
the approved calls and spliced their results into the local history,
but returned update_role=None / function_call_results=None, and the
first call site in the streaming loop has no yield gate. A tool
approved and executed in-run therefore ran silently: nothing streamed,
so AG-UI emits no TOOL_CALL_RESULT and the result is absent from
MESSAGES_SNAPSHOT, asymmetric with statically executed approvals.

Populate update_role/function_call_results from the approval branch
when calls were actually executed, and mirror the existing yield gate
at the first call site. Adds a regression test that approves a guarded
tool over a streaming run and asserts the function_result shows up.
Copilot AI review requested due to automatic review settings July 21, 2026 18:53
@giles17 giles17 added the python Usage: [Issues, PRs], Target: Python label Jul 21, 2026

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 fixes a streaming gap in the Python function-invocation layer where tools executed during approval resolution (rather than during the model’s normal tool-call turn) would run but never emit streamed function_result updates, preventing downstream streaming consumers (e.g., AG-UI) from observing TOOL_CALL_RESULT events.

Changes:

  • Return update_role="tool" and function_call_results from _process_function_requests when approval-resolution execution actually runs tools.
  • Add a yield gate after the first _process_function_requests call in the streaming loop so approval-resolution tool results stream like normal tool results.
  • Add a regression test covering approval-resolution tool-result streaming.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
python/packages/core/agent_framework/_tools.py Streams tool results produced during approval-resolution by returning update_role/results and yielding a ChatResponseUpdate in the first streaming loop call site.
python/packages/core/tests/core/test_harness_tool_approval.py Adds a regression test asserting approval-resolution execution produces a streamed function_result update.

Comment on lines +2371 to +2372
"update_role": "tool" if executed_count else None,
"function_call_results": approved_function_results or None,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in a406324. The branch now checks for user-input-request items the same way _handle_function_call_results does, and those go out as assistant updates instead of being filtered out by the function_result count. Covered by a new regression test that drives a tool raising UserInputRequiredException through approval resolution and asserts the update lands with the assistant role.

Comment on lines +598 to +606
tool_results = [
content
for update in second_updates
if update.role == "tool"
for content in update.contents
if content.type == "function_result"
]
assert tool_results, "approval-resolution execution must stream its function_result"
assert calls == 1

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tightened in a406324. The test now asserts exactly one streamed function_result with call_id == "call_guarded" and result == "guarded result", so a wrong or duplicated result would fail it.

@moonbox3

Copy link
Copy Markdown
Contributor

@he-yufeng please have a look at Copilot's comments.

…pdates

The approval-resolution branch labeled the stream by result count only, so a user-input request raised mid-execution was dropped from the stream. Mirror _handle_function_call_results: those items go out as assistant updates. Also tighten the streaming regression test to assert call_id, result, and count.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Python: [Bug]: Tool results from approval-resolution execution are never streamed, so provider-injected approvals emit no TOOL_CALL_RESULT

4 participants