Feature: whatsapp bridge integration

[jeancsil]

Integrate Whatsapp with agent-framework. Limiting messages being seen and sent only from me to myself :)

[jeancsil]

Code Review Summary

Overall assessment: Request Changes

This is a substantial feature addition for WhatsApp integration with a well-structured architecture, but it has several critical security, reliability, and quality issues that need to be addressed before merging.

Issues Found

🔴 Critical:

• Security vulnerability in contact filtering: The _on_message_event method has a critical flaw in handling self-messages from LIDs. It incorrectly processes self-messages (is_from_me=True) from unknown LIDs, which could allow messages from other contacts (e.g., LinkedIn sync) to be processed. This violates the security model that only messages from allowed_contact should be processed.
• Hardcoded test API key: CI configuration contains OPENAI_API_KEY: sk-test which appears to be a test key that should not be committed.
• Missing dependency management: The pydantic dependency is added but not properly integrated into the existing codebase - imports are inconsistent.

🟡 Minor:

• Incomplete error handling: Several async operations lack proper error handling, especially in whatsapp.py message handling.
• Configuration validation issues: The configuration validation doesn't properly validate phone number format in some cases.
• Missing documentation: Some new modules lack docstrings and proper documentation.
• Test coverage gaps: While there are comprehensive tests, some edge cases in the message filtering logic need additional test coverage.

Positive Highlights

• Well-structured architecture: The channel abstraction is clean and extensible.
• Comprehensive test suite: Extensive test coverage for the WhatsApp channel, including security-focused tests.
• Good separation of concerns: Configuration, channel logic, and agent logic are well-separated.
• Nice typing indicators feature: The typing indicator implementation is sophisticated with duration control.
• Security-conscious design: Strong privacy filtering and message deduplication features.

Next Steps

1. Fix security vulnerability: Update the contact filtering logic in _on_message_event to properly handle self-messages from unknown LIDs. The fix should ensure that only messages from the allowed_contact phone number (regardless of LID format) are processed.

2. Remove hardcoded credentials: Remove the test API key from the CI configuration.

3. Fix dependency imports: Ensure consistent use of pydantic across the codebase and remove unused imports.

4. Improve error handling: Add proper error handling for all async operations, especially in the WhatsApp channel's message handling.

5. Add configuration validation: Improve phone number format validation in the configuration.

6. Documentation: Add proper docstrings and usage documentation for new modules.

The implementation shows good engineering practices but needs these critical fixes before it can be safely merged and used.

[jeancsil]

🟡 Minor Issue: Missing error handling for database operations

The _send_typing and _stop_typing methods call self._client.send_chat_presence() but don't handle potential exceptions from the Go backend. This could cause the agent to crash if there are network issues.

Fix: Add try-catch around the send_chat_presence calls and log errors gracefully without crashing the agent.

try:
    self._client.send_chat_presence(...)
except Exception as e:
    logger.warning(f"Failed to send typing indicator: {e}")
    # Don't crash the agent, just log and continue

[jeancsil]

🟡 Minor Issue: Database connection thread safety concern

The code uses thread-local SQLite connections, but there's a potential race condition when multiple threads are accessing the database concurrently.

Fix: The current approach is generally good for avoiding SQLite threading issues, but consider adding additional synchronization or connection pooling if the agent becomes multi-threaded.

For now, the current implementation should work for the single-threaded event loop model, but this is worth noting for future scaling.

[jeancsil]

🟡 Minor Issue: Error message could expose sensitive information

The error handling in _handle_message includes the raw error message in the response to users. This could potentially expose internal implementation details, API keys, or other sensitive information.

Fix: Use a generic error message instead of str(e) to avoid exposing sensitive information.

# Instead of: f"Sorry, something went wrong processing your message: {e}"
# Use:
error_message = OutgoingMessage(
    text="Sorry, something went wrong processing your message. Please try again.",
    recipient_id=incoming.sender_id,
)

[jeancsil]

✅ Good Practice: Excellent configuration validation with pydantic

The configuration validation using pydantic models is well-implemented and provides strong type safety. The custom validators for MCP servers and storage path expansion are particularly good.

The validation ensures that all required configuration fields are present and properly typed before runtime, which prevents many potential issues.

[jeancsil]

✅ Good Practice: Excellent security filtering

The group chat filtering logic (if chat_jid.endswith("@g.us")) is perfect - it absolutely prevents any group chat messages from being processed, which is critical for privacy.

This is exactly the right approach to prevent the agent from accidentally processing messages from group chats that could contain sensitive information or spam.

[jeancsil]

✅ Good Practice: Excellent deduplication implementation

The message deduplication using SQLite with time-based windows is very well-implemented. It:

• Prevents processing of duplicate messages within the time window
• Allows legitimate repeated messages after the window expires
• Cleans up old records to prevent database bloat
• Uses proper thread-safe operations

This is much better than simple hash-based deduplication as it handles the real-world case where users might legitimately send the same message multiple times.

[jeancsil]

🟡 Minor Issue: Database initialization could fail silently

The _init_deduplication_db method calls _cleanup_old_deduplication_records() but if the database connection fails, it logs an error but continues running. This means the agent will run without deduplication.

Fix: Consider having a fallback mode or more graceful degradation when the database is unavailable, or at least make the error more visible to users.

if sqlite3.Error as e:
    logger.error(f"CRITICAL: Failed to initialize deduplication DB: {e}")
    logger.error("Agent will run without message deduplication - this may cause message processing issues")
    # Optionally raise an exception to prevent silent failure

[jeancsil]

🟡 Minor Issue: Media message error handling could be improved

The media message sending code in _send_media has good error handling for the HTTP request, but if the media building fails (e.g., invalid media type), the error might not be handled gracefully.

Fix: Add additional error handling for the media building and sending process to ensure users get appropriate feedback if media fails to send.

try:
    # Build and send media message
    await _build_and_send()
except Exception as media_error:
    logger.error(f"Failed to send media: {media_error}")
    # Send text fallback to user
    fallback_message = OutgoingMessage(
        text="Sorry, I couldn't send that media file. Please try sending it as text.",
        recipient_id=message.recipient_id,
    )
    await self.send(fallback_message)