-
Notifications
You must be signed in to change notification settings - Fork 16
adding notifications to python agent #190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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 pull request adds notification handling capabilities to the Python OpenAI sample agent, enabling it to respond to email notifications and Word document comments from Microsoft 365 applications. The implementation follows similar patterns established in other Agent 365 samples.
Changes:
- Added notification handler registration in the generic agent host to process incoming notifications
- Implemented notification processing logic in the agent to handle email notifications (EMAIL_NOTIFICATION) and Word document comments (WPX_COMMENT)
- Integrated observability configuration into the host startup sequence
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| python/openai/sample-agent/host_agent_server.py | Added notification imports, initialized AgentNotification, registered notification handler with authentication and observability support, and added observability configuration to startup |
| python/openai/sample-agent/agent.py | Implemented handle_agent_notification_activity method to process different notification types, added NotificationTypes import, and created helper method to extract results from agent responses |
| # <NotificationHandling> | ||
|
|
||
| async def handle_agent_notification_activity( | ||
| self, notification_activity, auth: Authorization, auth_handler_name: str, context: TurnContext |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The notification_activity parameter lacks type hints. According to the import at line 40, this should be typed as AgentNotificationActivity. This would improve code clarity and enable better IDE support and type checking.
| async def handle_agent_notification_activity( | ||
| self, notification_activity, auth: Authorization, auth_handler_name: str, context: TurnContext | ||
| ) -> str: | ||
| """Handle agent notification activities (email, Word mentions, etc.)""" | ||
| try: | ||
| notification_type = notification_activity.notification_type | ||
| logger.info(f"📬 Processing notification: {notification_type}") | ||
|
|
||
| # Setup MCP servers on first call | ||
| await self.setup_mcp_servers(auth, auth_handler_name, context) | ||
|
|
||
| # Handle Email Notifications | ||
| if notification_type == NotificationTypes.EMAIL_NOTIFICATION: | ||
| if not hasattr(notification_activity, "email") or not notification_activity.email: | ||
| return "I could not find the email notification details." | ||
|
|
||
| email = notification_activity.email | ||
| email_body = getattr(email, "html_body", "") or getattr(email, "body", "") | ||
| message = f"You have received the following email. Please follow any instructions in it. {email_body}" | ||
|
|
||
| result = await Runner.run(starting_agent=self.agent, input=message, context=context) | ||
| return self._extract_result(result) or "Email notification processed." | ||
|
|
||
| # Handle Word Comment Notifications | ||
| elif notification_type == NotificationTypes.WPX_COMMENT: | ||
| if not hasattr(notification_activity, "wpx_comment") or not notification_activity.wpx_comment: | ||
| return "I could not find the Word notification details." | ||
|
|
||
| wpx = notification_activity.wpx_comment | ||
| doc_id = getattr(wpx, "document_id", "") | ||
| comment_id = getattr(wpx, "initiating_comment_id", "") | ||
| drive_id = "default" | ||
|
|
||
| # Get Word document content | ||
| doc_message = f"You have a new comment on the Word document with id '{doc_id}', comment id '{comment_id}', drive id '{drive_id}'. Please retrieve the Word document as well as the comments and return it in text format." | ||
| doc_result = await Runner.run(starting_agent=self.agent, input=doc_message, context=context) | ||
| word_content = self._extract_result(doc_result) | ||
|
|
||
| # Process the comment with document context | ||
| comment_text = notification_activity.text or "" | ||
| response_message = f"You have received the following Word document content and comments. Please refer to these when responding to comment '{comment_text}'. {word_content}" | ||
| result = await Runner.run(starting_agent=self.agent, input=response_message, context=context) | ||
| return self._extract_result(result) or "Word notification processed." | ||
|
|
||
| # Generic notification handling | ||
| else: | ||
| notification_message = notification_activity.text or f"Notification received: {notification_type}" | ||
| result = await Runner.run(starting_agent=self.agent, input=notification_message, context=context) | ||
| return self._extract_result(result) or "Notification processed successfully." | ||
|
|
||
| except Exception as e: | ||
| logger.error(f"Error processing notification: {e}") | ||
| return f"Sorry, I encountered an error processing the notification: {str(e)}" | ||
|
|
||
| def _extract_result(self, result) -> str: | ||
| """Extract text content from agent result""" | ||
| if not result: | ||
| return "" | ||
| if hasattr(result, "final_output") and result.final_output: | ||
| return str(result.final_output) | ||
| elif hasattr(result, "contents"): | ||
| return str(result.contents) | ||
| elif hasattr(result, "text"): | ||
| return str(result.text) | ||
| elif hasattr(result, "content"): | ||
| return str(result.content) | ||
| else: | ||
| return str(result) | ||
|
|
||
| # </NotificationHandling> |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new notification handling functionality should be documented in the AGENT-CODE-WALKTHROUGH.md file. This is a significant feature that demonstrates how to handle email notifications and Word comment notifications. Consider adding a new "Step 6: Notification Handling" section before the current "Step 6: Cleanup and Resource Management" (which would then become Step 7), explaining the handle_agent_notification_activity method, the different notification types supported (EMAIL_NOTIFICATION and WPX_COMMENT), and how the agent processes them.
| configure( | ||
| service_name="OpenAIAgentTracingWithAzureOpenAI", | ||
| service_namespace="OpenAIAgentTesting", | ||
| ) |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This observability configuration will be overwritten when the agent is initialized. The OpenAIAgentWithMCP class already calls configure() in its _setup_observability() method (agent.py line 191) with different service names from environment variables. This duplicate configuration should be removed from here, or the agent's configuration should be removed to avoid conflicting configurations. The agent's configuration is more flexible as it uses environment variables (OBSERVABILITY_SERVICE_NAME and OBSERVABILITY_SERVICE_NAMESPACE) rather than hardcoded values.
|
|
||
| # Setup message handlers | ||
| self._setup_handlers() | ||
| logger.info("✅ Notification handlers registered successfully") |
Copilot
AI
Jan 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This log message is misleading because it appears right after _setup_handlers() is called, not after notification handlers are registered. The actual notification handler is registered inside _setup_handlers() at line 191. This log message should be moved inside _setup_handlers() after the notification handler decorator (after line 244), or the message should be updated to reflect that it's logging general handler setup completion, not specifically notification handlers.
|
@abdulanu0 would you attach ss of of the testing done |
No description provided.