feat: preliminary imap_mail integration closes issue #3202 #3213

waleedalzarooni · 2025-09-26T10:06:01Z

Description

IMAP integration into toolkit

Checklist

Go over all the following points, and put an x in all the boxes that apply.

[X ] I have read the CONTRIBUTION guide (required)
[X ] I have linked this PR to an issue using the Development section on the right sidebar or by adding Fixes #issue-number in the PR description (required)
[ X] I have checked if any dependencies need to be added or updated in pyproject.toml and uv lock
[ X] I have updated the tests accordingly (required for a bug fix or a new feature)
[ X] I have updated the documentation if needed:
[ X] I have added examples if this is a new feature

If you are unsure about any of these, don't hesitate to ask. We are here to help!

coderabbitai · 2025-09-26T10:06:09Z

Important

Review skipped

Auto reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

✨ Finishing touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch imap-mail-toolkit

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

waleedalzarooni · 2025-09-26T10:06:39Z

first draft still needs some tweaks!

Wendong-Fan

thanks @waleedalzarooni , left some initial comments

camel/toolkits/imap_mail_toolkit.py

Wendong-Fan · 2025-09-26T12:36:15Z

camel/toolkits/imap_mail_toolkit.py

+        # Validate required parameters
+        if not self.imap_server:
+            raise ValueError(
+                "IMAP server must be provided or set via IMAP_SERVER"
+                "environment variable"
+            )
+        if not self.smtp_server:
+            raise ValueError(
+                "SMTP server must be provided or set via SMTP_SERVER"
+                "environment variable"
+            )
+        if not self.username:
+            raise ValueError(
+                "Username must be provided or set via EMAIL_USERNAME"
+                "environment variable"
+            )
+        if not self.password:
+            raise ValueError(
+                "Password must be provided or set via EMAIL_PASSWORD"
+                "environment variable"
+            )


we can use the @api_keys_required decorator make the code more tidy

seems not updated?

Wendong-Fan · 2025-09-26T12:38:34Z

camel/toolkits/imap_mail_toolkit.py

+        if not self.imap_server or not self.username or not self.password:
+            raise ValueError(
+                "IMAP server, username, and password must be provided"
+            )


this check seems redundant, we already checked in __init__

got it, removed

Wendong-Fan · 2025-09-26T12:39:59Z

camel/toolkits/imap_mail_toolkit.py

+    def _get_smtp_connection(self) -> smtplib.SMTP:
+        r"""Establish SMTP connection.
+
+        Returns:
+            smtplib.SMTP: Connected SMTP client
+        """
+        if not self.smtp_server or not self.username or not self.password:
+            raise ValueError(
+                "SMTP server, username, and password must be provided"
+            )
+
+        try:
+            smtp = smtplib.SMTP(self.smtp_server, self.smtp_port)
+            smtp.starttls()
+            smtp.login(self.username, self.password)
+            logger.info(
+                "Successfully connected to SMTP server %s", self.smtp_server
+            )
+            return smtp
+        except Exception as e:
+            logger.error("Failed to connect to SMTP server: %s", e)
+            raise


would be better to do the connect in __init__? otherwise we would need to connect eachtime the function is called

If I only make the connection in __init__ then there will be an issue when sequential function calls occur. After each function call the connection must be closed, unless the connection is opened at each function call this is not possible.

@Wendong-Fan Would you prefer that I integrated the connections into the __init__ method then implemented a separate close_connections for the agent to call to close all connections at the end of an interaction (more risky as it may be missed by the agent)

i think we can implement automatic connection management with timeouts for this

Wendong-Fan · 2025-09-26T12:47:45Z

camel/toolkits/imap_mail_toolkit.py

+
+    def fetch_emails(
+        self,
+        folder: str = "INBOX",


for the folder, how many types do we support? would be better to use Literal to list all supported type?

My concern with using Literal here is that since we support many different email providers with slightly different naming conventions for folders, e.g. Gmail uses Sent and Outlook uses Sent Items.

Literal would be better compared with string, it would be more hard for llm to fill in free string with slight difference correctly

Wendong-Fan · 2025-09-26T12:52:44Z

camel/toolkits/imap_mail_toolkit.py

+                except imaplib.IMAP4.error:
+                    pass


not good practice to handle errors silently

got it, issue is fixed

LIHUA919 · 2025-09-26T22:22:20Z

examples/toolkits/imap_mail_toolkit.py

+        imap_server="imap.gmail.com",
+        smtp_server="smtp.gmail.com",
+        username="[email protected]",
+        password="your_app_password",


Great Job @waleedalzarooni. Sensitive email credentials are hardcoded in the example, which poses security risks.

got it, will be removed

a7m-1st · 2025-09-29T09:01:34Z

camel/toolkits/imap_mail_toolkit.py

+    - Moving emails to folders
+    - Deleting emails
+
+    Args:


Perhaps we can rename this to Attributes:

I think I'll just remove it, not super necessary

Probably better, bcz in the future sometimes its forgotten to update both of them.

let's keep the docstring here, otherwise it's not user-friendly for the developer to fill in the argument

Saedbhati · 2025-09-29T11:12:16Z

camel/toolkits/imap_mail_toolkit.py

+        try:
+            imap = self._get_imap_connection()


Instead of logging in inside each function, we could handle the login once in the constructor (init). This way, the connection is established when the class is initialized, and functions can directly use it. What do you think?

I mentioned to Wendong there is a slight concern with this approach, if I wanted to open connections in the innit method I would need to create a new close method to close all connections, I will discuss in the meeting today whether I should do this as it would add an extra method the agent would need to call, which it could forget

a7m-1st

For now that is all from me @waleedalzarooni. The example is working all right. The changes I noticed are up for discussion.

Thanks !

a7m-1st · 2025-09-29T09:03:45Z

camel/toolkits/imap_mail_toolkit.py

+        Args:
+            imap_server: IMAP server hostname
+            imap_port: IMAP server port (default: 993)
+            smtp_server: SMTP server hostname
+            smtp_port: SMTP server port (default: 587)
+            username: Email username
+            password: Email password
+            timeout: Timeout for operations
+        """


I think can also use the same as format as attributes above just for consistency.

i feel use Args would be better, args would be more useful information compared with attributes

a7m-1st · 2025-09-29T14:10:29Z

camel/toolkits/imap_mail_toolkit.py

+            logger.info(
+                "Email sent successfully to %s", ", ".join(to_recipients)
+            )
+            return "Email sent successfully. Message ID: Unknown"
+


Can you update the return statement as the model is recieving:

... {'role': 'tool', 'content': 'Email sent successfully. Message ID: Unknown', 'tool_call_id': 'call_YwrH4jMN258NYWyPfXIMcX0i'}]

Wendong-Fan

thanks @waleedalzarooni , left some comments below, also added one enhance PR to update part of the content discussed below here: #3236

camel/toolkits/imap_mail_toolkit.py

Wendong-Fan · 2025-10-02T13:35:44Z

camel/toolkits/imap_mail_toolkit.py

+    - Moving emails to folders
+    - Deleting emails
+
+    Args:


let's keep the docstring here, otherwise it's not user-friendly for the developer to fill in the argument

Wendong-Fan · 2025-10-02T13:37:20Z

camel/toolkits/imap_mail_toolkit.py

+        Args:
+            imap_server: IMAP server hostname
+            imap_port: IMAP server port (default: 993)
+            smtp_server: SMTP server hostname
+            smtp_port: SMTP server port (default: 587)
+            username: Email username
+            password: Email password
+            timeout: Timeout for operations
+        """


i feel use Args would be better, args would be more useful information compared with attributes

Wendong-Fan · 2025-10-02T13:37:44Z

camel/toolkits/imap_mail_toolkit.py

+        # Validate required parameters
+        if not self.imap_server:
+            raise ValueError(
+                "IMAP server must be provided or set via IMAP_SERVER"
+                "environment variable"
+            )
+        if not self.smtp_server:
+            raise ValueError(
+                "SMTP server must be provided or set via SMTP_SERVER"
+                "environment variable"
+            )
+        if not self.username:
+            raise ValueError(
+                "Username must be provided or set via EMAIL_USERNAME"
+                "environment variable"
+            )
+        if not self.password:
+            raise ValueError(
+                "Password must be provided or set via EMAIL_PASSWORD"
+                "environment variable"
+            )


seems not updated?

Wendong-Fan · 2025-10-02T13:47:28Z

camel/toolkits/imap_mail_toolkit.py

+                                )
+                                continue
+
+                        email_dict = {


seems in the fetch_emails method when email_body is neither bytes nor str, the code executes continue. However, outside of this branch, the email_message variable is used, could lead to NameError?

Wendong-Fan · 2025-10-02T13:48:25Z

camel/toolkits/imap_mail_toolkit.py

+                content_type = part.get_content_type()
+                content_disposition = str(part.get("Content-Disposition"))
+
+                # Skip attachments


duplicated comment

Wendong-Fan · 2025-10-02T14:00:41Z

camel/toolkits/imap_mail_toolkit.py

+    def _get_smtp_connection(self) -> smtplib.SMTP:
+        r"""Establish SMTP connection.
+
+        Returns:
+            smtplib.SMTP: Connected SMTP client
+        """
+        if not self.smtp_server or not self.username or not self.password:
+            raise ValueError(
+                "SMTP server, username, and password must be provided"
+            )
+
+        try:
+            smtp = smtplib.SMTP(self.smtp_server, self.smtp_port)
+            smtp.starttls()
+            smtp.login(self.username, self.password)
+            logger.info(
+                "Successfully connected to SMTP server %s", self.smtp_server
+            )
+            return smtp
+        except Exception as e:
+            logger.error("Failed to connect to SMTP server: %s", e)
+            raise


i think we can implement automatic connection management with timeouts for this

Wendong-Fan · 2025-10-02T14:07:21Z

camel/toolkits/imap_mail_toolkit.py

+
+    def fetch_emails(
+        self,
+        folder: str = "INBOX",


Literal would be better compared with string, it would be more hard for llm to fill in free string with slight difference correctly

Wendong-Fan · 2025-10-02T14:08:08Z

camel/toolkits/imap_mail_toolkit.py

+        r"""Fetch emails from a folder with optional filtering.
+
+        Args:
+            folder (str): Email folder to search in (default: "INBOX")


for the default value, please check use format like (default: :obj:True) same for other parts

Wendong-Fan · 2025-10-02T14:08:53Z

camel/toolkits/imap_mail_toolkit.py

+            logger.info(
+                "Email sent successfully to %s", ", ".join(to_recipients)
+            )
+            return "Email sent successfully. Message ID: Unknown"


why here use Message ID: Unknown

imap_mail integration

8dbb57a

waleedalzarooni linked an issue Sep 26, 2025 that may be closed by this pull request

[Feature Request] Integrate Generic Email Support via IMAP #3202

Open

2 tasks

Wendong-Fan reviewed Sep 26, 2025

View reviewed changes

Wendong-Fan requested review from Saedbhati and a7m-1st September 26, 2025 12:53

Merge branch 'master' into imap-mail-toolkit

2e38dc1

LIHUA919 reviewed Sep 26, 2025

View reviewed changes

a7m-1st reviewed Sep 29, 2025

View reviewed changes

Saedbhati reviewed Sep 29, 2025

View reviewed changes

Merge branch 'master' into imap-mail-toolkit

f58927a

a7m-1st requested changes Sep 29, 2025

View reviewed changes

Wendong-Fan reviewed Oct 2, 2025

View reviewed changes

Saedbhati and others added 2 commits October 3, 2025 18:34

Merge branch 'master' into imap-mail-toolkit

8b0d012

enhance: preliminary imap_mail integration PR3213 (#3236)

cbd3514

feat: preliminary imap_mail integration closes issue #3202 #3213

Are you sure you want to change the base?

feat: preliminary imap_mail integration closes issue #3202 #3213

Conversation

waleedalzarooni commented Sep 26, 2025

Description

Checklist

Uh oh!

coderabbitai bot commented Sep 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

waleedalzarooni commented Sep 26, 2025

Uh oh!

Wendong-Fan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

waleedalzarooni Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a7m-1st left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Wendong-Fan left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot commented Sep 26, 2025 •

edited

Loading

waleedalzarooni Sep 29, 2025 •

edited

Loading

Wendong-Fan left a comment •

edited

Loading