Skip to content

fix(gateway): key email thread context by root Message-ID instead of sender address - #11185

Open
buzz-code wants to merge 1 commit into
NousResearch:mainfrom
buzz-code:fix/email-thread-id-keying
Open

buzz-code wants to merge 1 commit into
NousResearch:mainfrom
buzz-code:fix/email-thread-id-keying

Conversation

@buzz-code

Copy link
Copy Markdown

What changed and why

_thread_context was keyed by sender_addr, so all emails from the same
person shared a single thread entry. If Alice sent two unrelated emails,
the second would clobber the first's context — replies would use the wrong
subject, In-Reply-To, and References headers, breaking email threading
in most clients.

Introduce _extract_thread_id() to derive the canonical thread root per
RFC 2822:

  1. First Message-ID in the References chain
  2. In-Reply-To if no References
  3. The email's own Message-ID for new threads
  4. A local UUID fallback for malformed emails with no headers

_thread_context is now keyed by this thread root. The full References
chain is accumulated on each inbound message so outbound replies carry the
complete header. thread_id is propagated through send(),
send_document(), and send_image() via metadata.

get_chat_info() no longer performs a context lookup — the
chat_id → thread_id mapping is one-to-many and get_chat_info has no
external callers, so it now returns a static response with subject: "".

How to test

  1. Configure an email adapter and send two unrelated emails from the same address
  2. Reply to each — verify each reply has the correct Subject, In-Reply-To,
    and References headers matching its own thread (not the other)
  3. Send a reply chain 3+ deep — verify References accumulates all
    Message-IDs in order

Unit tests: pytest tests/gateway/test_email.py -v (79 tests, all passing)

Platforms tested

Linux

…sender address

Previously _thread_context was keyed by sender_addr, so all emails from
the same person shared one thread context regardless of subject. This
caused replies to reference the wrong In-Reply-To/References headers when
a sender had multiple active threads.

Changes:
- Add _extract_thread_id() to derive the canonical thread root from the
  References chain, falling back to In-Reply-To then Message-ID, with a
  local UUID for malformed emails missing all three headers
- Key _thread_context by thread_id (root Message-ID) instead of sender_addr
- Accumulate the full References chain in thread context so multi-hop
  replies carry the complete header
- Thread thread_id through send(), send_document(), send_image() via metadata
- Simplify get_chat_info() — drop the stale ctx lookup, return subject: ""
Copilot AI review requested due to automatic review settings April 16, 2026 18:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes incorrect email threading by keying the EmailAdapter’s internal thread context by the canonical thread root (root Message-ID) rather than by sender address, preventing unrelated emails from the same sender from clobbering each other’s reply headers.

Changes:

  • Added _extract_thread_id() to compute a stable thread root from References → In-Reply-To → Message-ID → local UUID fallback.
  • Updated inbound email parsing/dispatch to compute and propagate thread_id, store per-thread context, and accumulate References.
  • Updated outbound send paths (send, send_image, send_document) to accept metadata.thread_id and use it for correct subject + threading headers; updated/added tests accordingly.

Reviewed changes

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

File Description
gateway/platforms/email.py Implements thread_id extraction/propagation and switches reply context to per-thread storage keyed by root Message-ID.
tests/gateway/test_email.py Adds/updates unit tests to validate thread ID derivation, context keying, References accumulation, and metadata-based thread routing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 576 to 580
caption: Optional[str] = None,
file_name: Optional[str] = None,
reply_to: Optional[str] = None,
metadata: Optional[Dict[str, Any]] = None,
) -> SendResult:

Copilot AI Apr 16, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

send_document() accepts reply_to but never uses it (it isn't forwarded to _send_email_with_attachment()), so attachment replies can't set In-Reply-To/References based on an explicit reply_to like send() does. Consider threading reply_to through to _send_email_with_attachment (and using it to set original_msg_id), so document sends behave consistently with plain-text sends.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-existing gap - _send_email_with_attachment never accepted reply_to_msg_id before this PR either. Out of scope here; tracked as a follow-up.

Comment on lines +264 to 266
# Map thread_id (root Message-ID) -> {subject, message_id, references}
# Each email reply-chain gets its own entry, enabling per-thread sessions.
self._thread_context: Dict[str, Dict[str, str]] = {}

Copilot AI Apr 16, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that _thread_context is keyed by per-thread Message-ID roots, it can grow unbounded as new threads arrive (previously it effectively bounded to sender addresses). To avoid long-lived adapters accumulating memory, consider adding an eviction policy (LRU/TTL/max-size) similar to other platform caches, or periodically trimming old thread entries.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid. The old sender-keyed dict was implicitly bounded by unique senders; Message-ID keying does grow faster. Other adapters handle this with inline eviction (no shared utility exists). Will address in a follow-up — not blocking this fix.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/email Email (IMAP/SMTP) adapter labels Apr 25, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for isolating a real email-threading problem. Current main still keys _thread_context by sender at plugins/platforms/email/adapter.py:867 and resolves it by recipient at :933, so the stated clobbering scenario remains valid.

Problems

  • The PR modifies the former gateway/platforms/email.py; commit 560010547 moved the live adapter to plugins/platforms/email/adapter.py. The implementation and imports need to be ported to that plugin surface.
  • Current native multi-image sends remain outside the proposed propagation: plugins/platforms/email/adapter.py:1023-1031 does not forward metadata to _send_email_with_attachments, which still looks up context by sender at :1047.
  • The added tests do not send replies for two distinct roots from the same sender, so they do not directly prove the reported overwrite is prevented.

Suggested changes

  • Port the thread-root and metadata changes to plugins/platforms/email/adapter.py, including native multi-image attachments.
  • Add a two-thread same-sender regression that asserts each outbound Subject, In-Reply-To, and References chain.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

Summary

Three PRs address the same-sender email-thread overwrite: #11185 introduces canonical root Message-ID context and References-chain propagation but targets the retired gateway adapter, #11418 stores multiple contexts without plumbing identity into outbound sends and also targets the retired adapter, while #73189 updates the live plugin and propagates explicit reply targets through text and attachment paths.

Related pull requests

  • #11185 related — (+208/-26) — superseded implementation direction: The canonical thread-root key and accumulated References chain directly address context clobbering, but the diff modifies the retired gateway/platforms/email.py surface and omits the live plugin's native multi-image path. Despite the keep_open review on #11185, #73189 is the better consolidation base because its diff changes the currently registered adapter; #11185's root-ID and References-chain design should be carried over before closure.
  • #11418 related — (+174/-13) — duplicate with an incomplete fix: The tuple-keyed store preserves concurrent contexts, but outbound methods still call lookup without a thread identifier and therefore select the latest message from the sender, reproducing the reported mis-threading. Despite the keep_open review on #11418, the diff neither updates the live plugin nor proves distinct outbound headers, so it should not remain the merge candidate.
  • #73189 related — (+85/-13) — preferred live-surface base, requiring regression coverage: The diff updates plugins/platforms/email/adapter.py, preserves multiple Message-ID contexts per sender, and forwards reply_to through text, document, and native multi-image attachment sends, directly covering the active clobber paths. It should additionally test two same-sender replies end to end and adopt #11185's stable root-ID/full References-chain handling so deeper replies remain associated with one conversation.

Duplicates

#11185, #11418, and #73189 substantially overlap on replacing the single context per sender; #11418 is the weakest duplicate because its outbound fallback still cannot identify the intended conversation, while #11185 contains useful root-thread semantics for incorporation into #73189.

Suggested consolidation

Merge #73189 after adding a two-thread same-sender outbound regression and incorporating #11185's canonical root Message-ID plus full References-chain propagation. Then close #11185 as superseded by the live-plugin implementation and #11418 as a duplicate whose latest-per-sender outbound lookup does not resolve the root cause.

Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 43 kB of PR diffs, 7 kB of issue/PR text, 4 kB of discussion (4 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/email Email (IMAP/SMTP) adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants