Skip to content

fix(email): preserve thread recipients and cursor state - #90482

Open
aviyashchin wants to merge 1 commit into
NousResearch:mainfrom
aviyashchin:fix/email-recipient-preservation
Open

aviyashchin wants to merge 1 commit into
NousResearch:mainfrom
aviyashchin:fix/email-recipient-preservation

Conversation

@aviyashchin

Copy link
Copy Markdown

What

  • Persist the IMAP UID cursor only after an inbound message is safely admitted.
  • Preserve same-thread In-Reply-To/References and reply-all recipient sets while removing the bot itself.
  • Support explicit Email subject and CC through the existing send-message tool.
  • Keep configured always-CC recipients without duplicating To/Cc addresses.

Why

Advancing the cursor before processing can lose a message across restart, and reducing a reply to only the sender drops legitimate thread participants. The send path also could not express an explicit subject/CC even though the adapter supports email delivery.

Validation

  • scripts/run_tests.sh tests/gateway/test_email.py tests/gateway/test_email_robustness.py tests/gateway/test_email_charset_fallback.py tests/gateway/test_email_secret_scope.py tests/tools/test_send_message_tool.py tests/tools/test_send_message_target_parse.py
  • 125 passed, 0 failed on macOS arm64
  • Production canary: authenticated Gmail inbound to the live adapter produced a two-message same thread; the reply carried In-Reply-To, preserved the original To and Cc recipients, and returned the exact requested token.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/plugins Plugin system and bundled plugins platform/email Email (IMAP/SMTP) adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Aug 20, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

Reviewed the diff. Solid upgrade on both axes. The durable UID cursor fixes a real restart-safety hole (another client marking mail SEEN while Hermes is down silently swallowed those messages under the UNSEEN search), with write-after-dispatch ordering proven by the failure test, atomic tmp+replace persistence, and an `int(uid) <= cursor` guard covering the `X:*` inclusive-range quirk. Recipient preservation is similarly complete: getaddresses-based extraction with dedup, thread-context carrying To/Cc into replies (self dropped, extras de-duped into Cc), always-cc/reply-all-senders config plus per-send cc/subject plumbed through both the adapter path and the standalone sender, with tests pinning each combination.

Four things worth attention:

  • plugins/platforms/email/adapter.py:~600 (_load_uid_cursor) — the stored `imap_host` is written but never validated on load. UIDs are scoped to a specific server's mailbox; if the same address is later pointed at a different host (migration, provider switch), the stale high-water mark silently suppresses everything below it on the new host. Suggestion: compare `payload["imap_host"]` against `self._imap_host.lower()` and discard the cursor on mismatch (log it).

  • One-time upgrade gap: on the first run after this change `_last_processed_uid` is None and connect bootstraps it to the current newest UID — so mail that arrived while the previous version was down (which UNSEEN would have caught) is skipped exactly once. Worth a line in the changelog; alternatively seed the cursor from the lowest unseen UID on that first bootstrap.

  • Head-of-line blocking is now durable. Since the cursor only advances post-dispatch, one persistently failing message (poison attachment, handler crash) both stops cursor progress and, if `_dispatch_message`'s exception escapes the poll loop unchecked, halts polling altogether. Confirm the poll task wraps `_check_inbox` failures; consider a retry-cap that logs-and-skips a repeatedly failing UID past N attempts so one bad message can't freeze the inbox.

  • Two sources of truth for external correspondents. The adapter accepts `extra.external_correspondents` or `EMAIL_EXTERNAL_CORRESPONDENTS", while the gateway-side isolation (the companion mechanism for skipping memory/persona) reads only the config key. A user configuring via the env var gets the "[External correspondent email]" tag but full operator context. Unify on one lookup path.

No blocking issues found — the cursor work in particular is the right fix.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/email Email (IMAP/SMTP) adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants