Fix: Properly await cancelled guardrail tasks during cleanup

[gn00295120]

Summary

Fixes a resource leak bug where guardrail tasks were cancelled but not properly awaited during cleanup, which could lead to unhandled task exception warnings and memory leaks.

Problem

The _cleanup_guardrail_tasks() method in RealtimeSession (src/agents/realtime/session.py:749) was only calling task.cancel() on pending guardrail tasks but not awaiting them. According to Python's asyncio best practices, cancelled tasks should be awaited to properly collect exceptions.

Evidence

1. Test code shows correct pattern: tests/realtime/test_session.py:1199 already demonstrates the proper cleanup:

   await asyncio.gather(*session._guardrail_tasks, return_exceptions=True)

2. Similar pattern used elsewhere: The WebSocket task cleanup in openai_realtime.py:519-523 follows the same pattern of cancel + await.

Solution

1. Changed _cleanup_guardrail_tasks() from sync to async method
2. Added await asyncio.gather(*self._guardrail_tasks, return_exceptions=True) after cancelling to properly collect exceptions
3. Updated the caller (_cleanup()) to await the cleanup method

Testing

New tests in tests/realtime/test_guardrail_cleanup.py:

• ✅ Test 1: Verify cancelled tasks are properly awaited
• ✅ Test 2: Verify exceptions during cleanup are handled
• ✅ Test 3: Verify multiple concurrent tasks are cleaned up

Existing tests: All 8 guardrail tests in test_session.py continue to pass.

Technical Details

Before:

def _cleanup_guardrail_tasks(self) -> None:
    for task in self._guardrail_tasks:
        if not task.done():
            task.cancel()
    self._guardrail_tasks.clear()

After:

async def _cleanup_guardrail_tasks(self) -> None:
    for task in self._guardrail_tasks:
        if not task.done():
            task.cancel()
    
    # Wait for all tasks to complete and collect any exceptions
    if self._guardrail_tasks:
        await asyncio.gather(*self._guardrail_tasks, return_exceptions=True)
    
    self._guardrail_tasks.clear()

Impact

• No breaking changes
• Prevents resource leaks
• Eliminates unhandled task exception warnings
• Follows asyncio best practices

[Copilot]

Pull Request Overview

This PR fixes a resource leak in guardrail task cleanup by ensuring cancelled tasks are properly awaited during session shutdown. The fix prevents unhandled task exception warnings and follows Python asyncio best practices.

Key changes:

• Converted _cleanup_guardrail_tasks() from sync to async method to properly await cancelled tasks
• Added asyncio.gather() with return_exceptions=True to collect exceptions from cancelled tasks
• Implemented comprehensive test coverage for the cleanup behavior

Reviewed Changes

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

File	Description
src/agents/realtime/session.py	Made _cleanup_guardrail_tasks() async and added proper task awaiting with exception handling
tests/realtime/test_guardrail_cleanup.py	Added comprehensive tests verifying proper cleanup of cancelled, failed, and multiple concurrent guardrail tasks
src/agents/agent.py	Added validation to ensure tools list contains only valid Tool objects, preventing runtime AttributeErrors
tests/test_agent_config.py	Added test coverage for tool validation to prevent invalid tool types from passing initialization

---

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

[Copilot]

The validation logic for tool types creates a tuple by concatenating get_args(Tool) with (Handoff,). This approach is fragile because it assumes get_args(Tool) returns a tuple. If Tool is not a Union type or the structure changes, this could fail. Consider using isinstance(get_args(Tool), tuple) to verify the return type, or use a more explicit approach like defining valid_tool_types as a list and extending it.

Suggested change

	valid_tool_types = get_args(Tool) + (Handoff,)
	valid_tool_types = list(get_args(Tool))
	valid_tool_types.append(Handoff)

[Copilot]

This line may fail if any type in valid_tool_types doesn't have a __name__ attribute. While most types have this attribute, it's safer to handle potential AttributeErrors, especially given the dynamic nature of get_args(). Consider wrapping this in a try-except or using getattr(t, '__name__', str(t)) for robustness.

Suggested change

	type_names = ", ".join(t.__name__ for t in valid_tool_types)
	type_names = ", ".join(getattr(t, '__name__', str(t)) for t in valid_tool_types)

[gn00295120]

Resolved the merge conflict by extending the same fix to _cleanup_tool_call_tasks().

Since PR #1984 introduced _cleanup_tool_call_tasks() with the same bug pattern (cancel without await), I applied the async cleanup approach to both methods for consistency:

async def _cleanup(self) -> None:
    await self._cleanup_guardrail_tasks()
    await self._cleanup_tool_call_tasks()  # Also made async

Added 3 tests for tool call cleanup. All 6 tests pass.