mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: Emit TOOL_CALL_RESULT events when resuming after tool approval (#4758)
* Emit TOOL_CALL_RESULT events on approval resume (#4589) When a tool call is approved via the interrupt/resume flow, _resolve_approval_responses executes the tool and injects the result into the messages array, but no TOOL_CALL_RESULT SSE event was yielded to the client. Changes: - _resolve_approval_responses now returns the list of resolved function_result Content objects instead of None - run_agent_stream yields ToolCallResultEvent for each resolved approval result after RunStartedEvent is emitted - Add ToolCallResultEvent to ag_ui.core imports in _agent_run.py Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Apply pre-commit auto-fixes * fix(ag-ui): address PR review feedback for #4589 1. _resolve_approval_responses now returns only approved results (not rejections) so TOOL_CALL_RESULT events are emitted only for executed tools. Rejection results are still written into message history. 2. Emit resolved TOOL_CALL_RESULT events in the no-updates fallback RUN_STARTED path so approval results are never lost. 3. Rewrite tests to use real FunctionTool with func and approval_mode='always_require' via StubAgent default_options, verifying actual tool execution output in TOOL_CALL_RESULT content. Added test for rejection not emitting TOOL_CALL_RESULT. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix #4589: clean up approval resolution and add missing tests - Extract duplicated TOOL_CALL_RESULT emission block into _make_approval_tool_result_events helper to prevent drift - Remove dead rejection_results construction in _resolve_approval_responses; _replace_approval_contents_with_results already handles rejections inline - Pass only approved_results (not all_results) to clarify the contract - Add mixed approve/reject test validating the core splitting logic - Add zero-updates test covering the no-updates fallback emission path - Add direct unit test for _resolve_approval_responses return value Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Apply pre-commit auto-fixes * Fix import sorting lint error in test_approval_result_event.py Add blank line between first-party and third-party import groups to satisfy ruff I001 rule. Fixes #4589 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Evan Mattson
committed by
GitHub
Unverified
GitHub
parent
4afc088f01
commit
cefda44283
@@ -21,6 +21,7 @@ from ag_ui.core import (
|
||||
TextMessageStartEvent,
|
||||
ToolCallArgsEvent,
|
||||
ToolCallEndEvent,
|
||||
ToolCallResultEvent,
|
||||
ToolCallStartEvent,
|
||||
)
|
||||
from agent_framework import (
|
||||
@@ -369,6 +370,24 @@ def _handle_step_based_approval(messages: list[Any]) -> list[BaseEvent]:
|
||||
return events
|
||||
|
||||
|
||||
def _make_approval_tool_result_events(resolved_approval_results: list[Content]) -> list[ToolCallResultEvent]:
|
||||
"""Build TOOL_CALL_RESULT events for tools executed during approval resolution."""
|
||||
events: list[ToolCallResultEvent] = []
|
||||
for resolved in resolved_approval_results:
|
||||
if resolved.call_id:
|
||||
raw = resolved.result if resolved.result is not None else ""
|
||||
result_str = raw if isinstance(raw, str) else json.dumps(make_json_safe(raw))
|
||||
events.append(
|
||||
ToolCallResultEvent(
|
||||
message_id=generate_event_id(),
|
||||
tool_call_id=resolved.call_id,
|
||||
content=result_str,
|
||||
role="tool",
|
||||
)
|
||||
)
|
||||
return events
|
||||
|
||||
|
||||
def _evict_oldest_approvals(registry: dict[str, str], max_size: int = 10_000) -> None:
|
||||
"""Evict the oldest entries from the pending-approvals registry (LRU).
|
||||
|
||||
@@ -391,7 +410,7 @@ async def _resolve_approval_responses(
|
||||
run_kwargs: dict[str, Any],
|
||||
pending_approvals: dict[str, str] | None = None,
|
||||
thread_id: str = "",
|
||||
) -> None:
|
||||
) -> list[Content]:
|
||||
"""Execute approved function calls and replace approval content with results.
|
||||
|
||||
This modifies the messages list in place, replacing function_approval_response
|
||||
@@ -407,10 +426,16 @@ async def _resolve_approval_responses(
|
||||
When provided, every approval response is validated against this
|
||||
registry to prevent bypass, function name spoofing, and replay.
|
||||
thread_id: The conversation thread ID used to scope registry keys.
|
||||
|
||||
Returns:
|
||||
List of approved function_result Content objects only (empty if no
|
||||
approvals). Rejection results are written into the message history
|
||||
but are *not* included in the return value because they should not
|
||||
be emitted as TOOL_CALL_RESULT events.
|
||||
"""
|
||||
fcc_todo = _collect_approval_responses(messages)
|
||||
if not fcc_todo:
|
||||
return
|
||||
return []
|
||||
|
||||
approved_responses = [resp for resp in fcc_todo.values() if resp.approved]
|
||||
rejected_responses = [resp for resp in fcc_todo.values() if not resp.approved]
|
||||
@@ -493,31 +518,23 @@ async def _resolve_approval_responses(
|
||||
logger.exception("Failed to execute approved tool calls; injecting error results: %s", e)
|
||||
approved_function_results = []
|
||||
|
||||
# Build normalized results for approved responses
|
||||
normalized_results: list[Content] = []
|
||||
# Build results for approved responses (used for TOOL_CALL_RESULT event emission)
|
||||
approved_results: list[Content] = []
|
||||
for idx, approval in enumerate(approved_responses):
|
||||
if (
|
||||
idx < len(approved_function_results)
|
||||
and getattr(approved_function_results[idx], "type", None) == "function_result"
|
||||
):
|
||||
normalized_results.append(approved_function_results[idx])
|
||||
approved_results.append(approved_function_results[idx])
|
||||
continue
|
||||
# Get call_id from function_call if present, otherwise use approval.id
|
||||
func_call = approval.function_call
|
||||
call_id = (func_call.call_id if func_call else None) or approval.id or ""
|
||||
normalized_results.append(
|
||||
approved_results.append(
|
||||
Content.from_function_result(call_id=call_id, result="Error: Tool call invocation failed.")
|
||||
)
|
||||
|
||||
# Build rejection results
|
||||
for rejection in rejected_responses:
|
||||
func_call = rejection.function_call
|
||||
call_id = (func_call.call_id if func_call else None) or rejection.id or ""
|
||||
normalized_results.append(
|
||||
Content.from_function_result(call_id=call_id, result="Error: Tool call invocation was rejected by user.")
|
||||
)
|
||||
|
||||
_replace_approval_contents_with_results(messages, fcc_todo, normalized_results) # type: ignore
|
||||
_replace_approval_contents_with_results(messages, fcc_todo, approved_results) # type: ignore
|
||||
|
||||
# Post-process: Convert user messages with function_result content to proper tool messages.
|
||||
# After _replace_approval_contents_with_results, approved tool calls have their results
|
||||
@@ -525,6 +542,8 @@ async def _resolve_approval_responses(
|
||||
# This transformation ensures the message history is valid for the LLM provider.
|
||||
_convert_approval_results_to_tool_messages(messages)
|
||||
|
||||
return approved_results
|
||||
|
||||
|
||||
def _convert_approval_results_to_tool_messages(messages: list[Message]) -> None:
|
||||
"""Convert function_result content in user messages to proper tool messages.
|
||||
@@ -787,7 +806,9 @@ async def run_agent_stream(
|
||||
# Resolve approval responses (execute approved tools, replace approvals with results)
|
||||
# This must happen before running the agent so it sees the tool results
|
||||
tools_for_execution = tools if tools is not None else server_tools
|
||||
await _resolve_approval_responses(messages, tools_for_execution, agent, run_kwargs, pending_approvals, thread_id)
|
||||
resolved_approval_results = await _resolve_approval_responses(
|
||||
messages, tools_for_execution, agent, run_kwargs, pending_approvals, thread_id
|
||||
)
|
||||
|
||||
# Defense-in-depth: replace approval payloads in snapshot with actual tool results
|
||||
# so CopilotKit does not re-send stale approval content on subsequent turns.
|
||||
@@ -851,6 +872,9 @@ async def run_agent_stream(
|
||||
yield StateSnapshotEvent(snapshot=flow.current_state)
|
||||
run_started_emitted = True
|
||||
|
||||
for event in _make_approval_tool_result_events(resolved_approval_results):
|
||||
yield event
|
||||
|
||||
# Feature #4: Detect tool-only messages (no text content)
|
||||
# Emit TextMessageStartEvent to create message context for tool calls
|
||||
if not flow.message_id and _has_only_tool_calls(update.contents):
|
||||
@@ -905,7 +929,8 @@ async def run_agent_stream(
|
||||
if state_schema and flow.current_state:
|
||||
yield StateSnapshotEvent(snapshot=flow.current_state)
|
||||
|
||||
# Process structured output if response_format is set
|
||||
for event in _make_approval_tool_result_events(resolved_approval_results):
|
||||
yield event
|
||||
if response_format is not None and all_updates:
|
||||
from agent_framework import AgentResponse
|
||||
from pydantic import BaseModel
|
||||
|
||||
Reference in New Issue
Block a user