Skip to content

Fix email reply References threading - #55841

Open
plarson wants to merge 1 commit into
NousResearch:mainfrom
plarson:fix/email-threading-references
Open

plarson wants to merge 1 commit into
NousResearch:mainfrom
plarson:fix/email-threading-references

Conversation

@plarson

@plarson plarson commented Jun 30, 2026

Copy link
Copy Markdown

Summary

  • Preserve inbound email References when sending gateway replies
  • Append the current inbound Message-ID to the outbound References chain
  • Cover the behavior with focused email threading header tests

Test

  • uv run --with pytest pytest -q tests/test_email_threading_headers.py
  • python -m py_compile plugins/platforms/email/adapter.py tests/test_email_threading_headers.py

Copilot AI review requested due to automatic review settings June 30, 2026 19:25

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 improves email reply threading in the gateway’s Email platform adapter by carrying forward inbound References headers into outbound replies and extending the References chain with the message being replied to, with new focused tests to validate the behavior.

Changes:

  • Capture inbound References in _fetch_new_messages() and persist it into _thread_context during dispatch.
  • Build outbound References by combining stored inbound References with the current Message-ID being replied to (for plain and attachment sends).
  • Add focused tests asserting In-Reply-To and References behavior for replies with/without prior References.

Reviewed changes

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

File Description
plugins/platforms/email/adapter.py Stores inbound References and uses it to build outbound reply threading headers.
tests/test_email_threading_headers.py Adds targeted tests validating outbound In-Reply-To/References header construction.

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

Comment thread plugins/platforms/email/adapter.py Outdated
Comment on lines +908 to +911
references = " ".join(
part for part in [ctx.get("references", ""), original_msg_id] if part
)
msg["References"] = references
Comment thread plugins/platforms/email/adapter.py Outdated
Comment on lines +1024 to +1027
references = " ".join(
part for part in [ctx.get("references", ""), original_msg_id] if part
)
msg["References"] = references
Comment thread plugins/platforms/email/adapter.py Outdated
Comment on lines +1107 to +1110
references = " ".join(
part for part in [ctx.get("references", ""), original_msg_id] if part
)
msg["References"] = references
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have platform/email Email (IMAP/SMTP) adapter comp/plugins Plugin system and bundled plugins labels Jun 30, 2026
@alt-glitch

Copy link
Copy Markdown

This was generated by AI during triage.

Email-threading cluster: this PR is the narrow outbound References-chain fix. Broader OPEN siblings cover overlapping ground — #27508 (References-root Message-ID as thread_id + outbound References, most standards-complete), #23999 (Gmail Message-ID/In-Reply-To/References preservation), #11185 (rekey thread_context by root Message-ID). Related, not duplicate (subset of #27508). Flagging the cluster for a maintainer to pick the canonical approach.

@plarson
plarson force-pushed the fix/email-threading-references branch from 6eddd51 to aded28b Compare June 30, 2026 19:37
@plarson

plarson commented Jun 30, 2026

Copy link
Copy Markdown
Author

Addressed the review feedback about re-emitting inbound References verbatim.

Update:

  • Added helper logic to unfold/sanitize inbound References values before reuse.
  • Deduplicates message IDs while preserving order.
  • Uses the helper in all three outbound send paths.
  • Added regression coverage for folded References headers and duplicate current message IDs.

Verification:

  • uv run --with pytest pytest -q tests/test_email_threading_headers.py → 3 passed
  • python -m py_compile plugins/platforms/email/adapter.py tests/test_email_threading_headers.py
  • git diff --check

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for addressing the folded-header and duplicate-ID feedback; the new helper in plugins/platforms/email/adapter.py:71-108 is a clear improvement over re-emitting inbound header text.

Problems

  • plugins/platforms/email/adapter.py:881 adds references to context that is still keyed only by sender_addr. Current main reads that context by recipient address in all three send paths (:933, :1047, :1127). If one sender has two active email threads, the later inbound message overwrites the earlier thread's References chain. The gateway passes the individual reply anchor (gateway/platforms/base.py:5028-5033), but _send_email combines it with that overwritten context.

Suggested changes

  • Carry a stable email-thread key through dispatch and reply delivery, then key context by that value; add a two-threads/same-sender regression test.
  • Add attachment-path coverage for the two additional changed send methods.

Automated hermes-sweeper review.

self._thread_context[sender_addr] = {
"subject": subject,
"message_id": msg_data["message_id"],
"references": msg_data.get("references", ""),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This value is still stored under sender_addr, while every send path later retrieves context by recipient address. A second, unrelated thread from the same sender overwrites this References chain before the first thread's response is sent. Please key/route the context by a stable email-thread identity and add a same-sender/two-threads regression.

@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:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026

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:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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.

4 participants