Python: Add MCP Host history conversion for AG-UI - #8130
Python: Add MCP Host history conversion for AG-UI#8130Eduard van Valkenburg (eavanvalkenburg) wants to merge 1 commit into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): f0d4e53c5e12
Model: gpt-5.6-sol-fast
Overview
The PR cleanly separates the opt-in Host-history projection from the generic model-safe converter, preserves parallel results and mixed content, and reuses the existing aggregate Host-plus-sidecar budget. Its tests and public exports cover persistence, replay metadata, and model-request isolation well. Two residual risks remain: budget eviction can turn Host-only text into model input for valid non-text projections, and the aggregate cap is enforced only after the full history has been serialized.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
2 verified findings remained after source verification (1 high, 1 medium) across 1 file. Details are attached to the affected lines below.
Affected areas: python/packages/ag-ui/agent_framework_ag_ui/_message_adapters.py
| ) -> list[dict[str, Any]]: | ||
| """Convert Agent Framework messages to bounded AG-UI Host history with replay metadata.""" | ||
| converted = _convert_agent_framework_messages_to_agui(messages, include_host_payload=True) | ||
| return _bound_host_payload_history( |
There was a problem hiding this comment.
When a valid replay sidecar contains only data/URI items and this budget evicts the entry, _bound_host_payload_history falls back to text from the Host payload, removes the MCP marker, and the next inbound conversion sends that Host-only text to the model. This breaks the model-safe separation and can expose private or prompt-injecting UI content. On eviction, derive the fallback only from the model projection, or use a safe model placeholder; never source it from the Host payload.
| max_host_payload_history_size_bytes: int = _MAX_MCP_HOST_PAYLOAD_HISTORY_SIZE_BYTES, | ||
| ) -> list[dict[str, Any]]: | ||
| """Convert Agent Framework messages to bounded AG-UI Host history with replay metadata.""" | ||
| converted = _convert_agent_framework_messages_to_agui(messages, include_host_payload=True) |
There was a problem hiding this comment.
max_host_payload_history_size_bytes is applied only after every Host payload and replay sidecar has been serialized into converted. A history containing many individually allowed near-1-MiB MCP results therefore consumes memory and CPU proportional to the full history before older entries are discarded, so an untrusted MCP server can stall or OOM a shared process despite the 8-MiB output cap. Enforce the aggregate budget while converting newest-first so entries beyond the limit are never materialized.
| if include_host_payload: | ||
| has_host_payload, host_payload = _extract_mcp_tool_result_host_payload(fr) | ||
| if has_host_payload: | ||
| display_values = _extract_tool_result_marker_values(fr, TOOL_RESULT_DISPLAY_KEY) | ||
| tool_message["content"] = _stringify_tool_result( | ||
| display_values[-1] if display_values else host_payload | ||
| ) | ||
| tool_message[_AGUI_MCP_TOOL_RESULT_KEY] = True | ||
| tool_message[_AGUI_TOOL_RESULT_MODEL_CONTENT_KEY] = _model_items_for_agui_replay( | ||
| fr, _stringify_tool_result(model_result) | ||
| ) |
There was a problem hiding this comment.
Could we have one helper build the MCP Host-history fields? This adds a third place that must keep the same rules synchronized: a retained Host marker is required, the last TOOL_RESULT_DISPLAY_KEY overrides the Host payload, content holds that serialized projection, and _agentFrameworkMcpResult plus _agentFrameworkModelContent preserve safe replay. A shared _project_mcp_host_result(content, model_result) returning those fields could serve this converter, live snapshots, and approval-resolved snapshots without changing behavior.
| | AG-UI approval-time follow-up | The full grouped user-input pause remains in message history and emits no synthetic `TOOL_CALL_RESULT`. | `packages/ag-ui/tests/ag_ui/test_approval_result_event.py::test_approval_follow_up_group_remains_in_history_without_live_tool_result` | | ||
| | AG-UI approval execution failure | A grouped executor failure becomes one deterministic terminal error result for the approved call. | `packages/ag-ui/tests/ag_ui/test_approval_result_event.py::test_approval_execution_failure_emits_one_terminal_error_result` | | ||
| | AG-UI no-approval path | Ordinary tool results do not gain an extra approval result event. | `packages/ag-ui/tests/ag_ui/test_approval_result_event.py::test_no_approval_path_emits_no_approval_specific_duplicate_result` | | ||
| | AG-UI MCP Host payload | Core preserves bounded successful and error MCP `CallToolResult` payloads separately from built-in or custom model-facing results; oversized Host payloads are rejected by a bounded preflight without changing the model projection. AG-UI projects retained Host payloads consistently through ordinary and approval-resolved live events, aggregate-bounded messages snapshots, and the supported Host-history converter without changing generic outbound requests or replaying UI-only data to the model. | `packages/core/tests/core/test_mcp.py::test_parse_tool_result_from_mcp_preserves_complete_host_payload_once`, `test_custom_mcp_result_parser_preserves_host_payload_and_model_projection`, `test_oversized_mcp_host_payload_is_omitted_without_changing_model_result`, `test_mcp_host_payload_size_preflight_matches_json_and_aborts_before_dump`, `test_mcp_error_preserves_complete_host_payload_on_function_result`, `packages/ag-ui/tests/ag_ui/test_run_common.py::TestEmitToolResultWithState::test_plain_tool_result_does_not_serialize_replay_items`, `test_mcp_host_payload_routes_to_live_event_and_snapshot`, `test_mcp_snapshot_replays_rich_model_items_without_host_payload_duplication`, `test_messages_snapshot_bounds_cumulative_mcp_host_payloads`, `packages/ag-ui/tests/ag_ui/test_message_adapters.py::test_agent_framework_to_agui_preserves_mcp_host_payload_after_reload`, `test_host_history_conversion_preserves_parallel_results_and_mixed_content`, `test_host_history_conversion_is_public_and_bounds_aggregate_payloads`, `test_mcp_replay_requires_provenance_and_keeps_error_generic` | |
There was a problem hiding this comment.
I still don't understand why this file continues to grow? Can you please educate me?
Motivation & Context
MCP Apps need persisted Agent Framework history to preserve complete Host/UI hydration data without exposing that data to later model requests. This top layer completes the #7971 replacement stack by adding the supported outbound Host-history conversion API on top of the core retention work in #8128 and the AG-UI live/snapshot work in #8129.
Description & Review Guide
agent_framework_messages_to_agui_host_history()toagent_framework_ag_uiand theagent_framework.ag_uinamespace and stub.Messagehistory through the existing aggregate Host-plus-sidecar budget.agent_framework_messages_to_agui()output model-safe while preserving one tool message per function result and mixed-message content in both conversion paths.AGUIChatClientand generic outbound conversion continue to send only model-facing results.Related Issue
Fixes #7959
This is the top/closing layer of the replacement stack for the still-open source PR #7971; it does not modify or close that source PR. It depends on #8129 and #8128 and is orthogonal to #7897.
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.