Skip to content

Conversation

gn00295120
Copy link
Contributor

@gn00295120 gn00295120 commented Oct 18, 2025

Summary

Fixes #1020

This PR adds the missing mcp_approval_response event name for MCPApprovalResponseItem in RunItemStreamEvent.

Problem

The RunItemStreamEvent.name Literal was missing an event name for MCPApprovalResponseItem, even though it's part of the RunItem union type. This caused:

  1. Incomplete type coverage: 9 RunItem types but only 8 event names
  2. Potential runtime issues: MCPApprovalResponseItem would fall through to the else branch, triggering "Unexpected item type" warnings
  3. Inconsistency: All other RunItem types had corresponding events except this one

1. 重現問題 (Reproduce the Problem)

Step 1: Review the Type Definitions

Check src/agents/items.py to see all RunItem types:

RunItem: TypeAlias = Union[
    MessageOutputItem,
    HandoffCallItem,
    HandoffOutputItem,
    ToolCallItem,
    ToolCallOutputItem,
    ReasoningItem,
    MCPListToolsItem,
    MCPApprovalRequestItem,
    MCPApprovalResponseItem,  # ← This type exists!
]

Count: 9 types total

Step 2: Check the Event Names

Check src/agents/stream_events.py:

class RunItemStreamEvent:
    name: Literal[
        "message_output_created",
        "handoff_requested",
        "handoff_occured",
        "tool_called",
        "tool_output",
        "reasoning_item_created",
        "mcp_approval_requested",
        "mcp_list_tools",
        # ❌ Missing: "mcp_approval_response" for MCPApprovalResponseItem!
    ]

Count: Only 8 event names

Step 3: Verify the Gap

Create a mapping table:

RunItem Type Event Name Status
MessageOutputItem message_output_created
HandoffCallItem handoff_requested
HandoffOutputItem handoff_occured
ToolCallItem tool_called
ToolCallOutputItem tool_output
ReasoningItem reasoning_item_created
MCPApprovalRequestItem mcp_approval_requested
MCPApprovalResponseItem ??? ❌ MISSING
MCPListToolsItem mcp_list_tools

Problem confirmed: MCPApprovalResponseItem has no corresponding event name!

Step 4: Check the Handler Code

In src/agents/_run_impl.py, the stream_step_items_to_queue function handles all item types:

if isinstance(item, MessageOutputItem):
    event = RunItemStreamEvent(item=item, name="message_output_created")
elif isinstance(item, HandoffCallItem):
    event = RunItemStreamEvent(item=item, name="handoff_requested")
# ... other types ...
elif isinstance(item, MCPApprovalRequestItem):
    event = RunItemStreamEvent(item=item, name="mcp_approval_requested")
elif isinstance(item, MCPListToolsItem):
    event = RunItemStreamEvent(item=item, name="mcp_list_tools")
else:
    # ❌ MCPApprovalResponseItem would fall here!
    logger.warning(f"Unexpected item type for streaming: {type(item)}")
    continue

Result: MCPApprovalResponseItem would trigger the warning and be skipped.

2. 修復 (Fix)

Fix Part 1: Add Event Name

In src/agents/stream_events.py (line 40), add the missing event name:

class RunItemStreamEvent:
    name: Literal[
        "message_output_created",
        "handoff_requested",
        "handoff_occured",
        "tool_called",
        "tool_output",
        "reasoning_item_created",
        "mcp_approval_requested",
        "mcp_approval_response",  # ✅ NEW - Added this line
        "mcp_list_tools",
    ]

Fix Part 2: Add Handler

In src/agents/_run_impl.py (lines 1175-1176), add the handler case:

elif isinstance(item, MCPApprovalRequestItem):
    event = RunItemStreamEvent(item=item, name="mcp_approval_requested")
elif isinstance(item, MCPApprovalResponseItem):  # ✅ NEW - Added this case
    event = RunItemStreamEvent(item=item, name="mcp_approval_response")
elif isinstance(item, MCPListToolsItem):
    event = RunItemStreamEvent(item=item, name="mcp_list_tools")

3. 驗證問題被解決 (Verify the Fix)

Verification 1: Type Coverage is Complete

After the fix, all 9 RunItem types have corresponding event names:

RunItem Type Event Name Status
MessageOutputItem message_output_created
HandoffCallItem handoff_requested
HandoffOutputItem handoff_occured
ToolCallItem tool_called
ToolCallOutputItem tool_output
ReasoningItem reasoning_item_created
MCPApprovalRequestItem mcp_approval_requested
MCPApprovalResponseItem mcp_approval_response FIXED
MCPListToolsItem mcp_list_tools

9 types → 9 events (complete coverage)

Verification 2: Run Existing Tests

# Test stream events
pytest tests/test_stream_events.py -v

# Result: 3/3 tests passed ✅
# - test_run_item_stream_event_schema
# - test_tool_approval_stream_event
# - test_stream_events_all_types

Verification 3: Run MCP Tests

# Test MCP functionality
pytest tests/ -k mcp -v

# Result: 1/1 MCP test passed ✅

Verification 4: Type Checking

# Run mypy type checking
mypy src/agents/stream_events.py src/agents/_run_impl.py

# Result: No type errors ✅

Verification 5: Manual Code Review

Review the handler logic in _run_impl.py:

# Before fix: MCPApprovalResponseItem had no handler
# After fix: All 9 RunItem types are handled ✅

if isinstance(item, MessageOutputItem):         # ✅ Handled
elif isinstance(item, HandoffCallItem):         # ✅ Handled
elif isinstance(item, HandoffOutputItem):       # ✅ Handled
elif isinstance(item, ToolCallItem):            # ✅ Handled
elif isinstance(item, ToolCallOutputItem):      # ✅ Handled
elif isinstance(item, ReasoningItem):           # ✅ Handled
elif isinstance(item, MCPApprovalRequestItem):  # ✅ Handled
elif isinstance(item, MCPApprovalResponseItem): # ✅ Handled (NEW!)
elif isinstance(item, MCPListToolsItem):        # ✅ Handled
else:
    # This else branch should never be reached now
    logger.warning(f"Unexpected item type")

Impact

  • Breaking change: No - this only adds a new event name
  • Backward compatible: Yes - existing code continues to work
  • Side effects: None - pure addition
  • Performance: No impact

Changes

src/agents/stream_events.py

  • Added "mcp_approval_response" to RunItemStreamEvent.name Literal (line 40)

src/agents/_run_impl.py

  • Added MCPApprovalResponseItem case in stream_step_items_to_queue (lines 1175-1176)

Generated with Lucas Wang[email protected]

The RunItemStreamEvent was missing the 'mcp_approval_response' event name
for MCPApprovalResponseItem, causing it to fall through to the 'else' branch
and trigger an 'Unexpected item type' warning.

Changes:
- Added 'mcp_approval_response' to RunItemStreamEvent.name Literal
- Added MCPApprovalResponseItem handling in stream_step_items_to_queue

This ensures all RunItem types have corresponding event names:
- MessageOutputItem -> message_output_created
- HandoffCallItem -> handoff_requested
- HandoffOutputItem -> handoff_occured
- ToolCallItem -> tool_called
- ToolCallOutputItem -> tool_output
- ReasoningItem -> reasoning_item_created
- MCPApprovalRequestItem -> mcp_approval_requested
- MCPApprovalResponseItem -> mcp_approval_response (NEW)
- MCPListToolsItem -> mcp_list_tools

Generated with Lucas Wang<[email protected]>
@Copilot Copilot AI review requested due to automatic review settings October 18, 2025 23:42
Copy link

@Copilot Copilot AI left a comment

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 missing event name for MCPApprovalResponseItem in the streaming events system, addressing incomplete type coverage where 9 RunItem types existed but only 8 had corresponding event names.

Key changes:

  • Added "mcp_approval_response" to the RunItemStreamEvent.name Literal type
  • Added handling for MCPApprovalResponseItem in the stream event processing logic

Reviewed Changes

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

File Description
src/agents/stream_events.py Added missing "mcp_approval_response" event name to the Literal type
src/agents/_run_impl.py Added case handling for MCPApprovalResponseItem in stream processing

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@seratch seratch merged commit 2630489 into openai:main Oct 21, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incomplete RunItemStreamEvent names list

2 participants