Skip to content

fix(email): isolate sessions by normalized subject when opted in - #104148

Open
santiagomalter wants to merge 1 commit into
NousResearch:mainfrom
santiagomalter:fix/email-session-by-subject
Open

santiagomalter wants to merge 1 commit into
NousResearch:mainfrom
santiagomalter:fix/email-session-by-subject

Conversation

@santiagomalter

Copy link
Copy Markdown

What does this PR do?

The Email adapter currently omits thread_id and keys _thread_context by sender address alone. Every inbound mail from one person shares a Hermes session, and outbound In-Reply-To / Subject follow that sender's last message. Parallel topics interrupt each other and Gmail (or any client that threads on those headers) piles replies onto the wrong conversation.

This ports closed #26307 onto the live plugin adapter. Default behavior is unchanged. When opted in, chat_id stays the sender address (SMTP To: remains valid) and isolation uses the existing DM thread_id slot in build_session_key(). Reply context is keyed by sender + normalized subject, and metadata.thread_id is plumbed through text, image, multi-image, and document sends.

I have been running this shape in production on generic IMAP/SMTP (not Gmail IMAP): one allowlisted sender, several parallel topics.

Fixes #26277
Related: #27804 (isolation half only — status-mail volume is out of scope)

Duplicate check

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • plugins/platforms/email/adapter.py — opt-in EMAIL_SESSION_BY_SUBJECT / platforms.email.extra.session_by_subject; normalize Re:/Fw:/Fwd:/CJK prefixes; build_source(..., thread_id=...); _new_reply looks up context by sender+thread; all send paths forward metadata.thread_id
  • plugins/platforms/email/plugin.yaml — optional env
  • tests/gateway/test_email.py — prefix stripping, default-off lock, extra-without-env, empty subject, two-subject In-Reply-To, document metadata, restart subject rebuild
  • tests/gateway/test_send_multiple_images.py — multi-image forwards the context key
  • docs: email user-guide subsection, env-var table row, commented cli-config.yaml.example extra

How to Test

  1. Default (flag off): two mails from one sender, different subjects → one session; last subject wins (unchanged).
  2. EMAIL_SESSION_BY_SUBJECT=true or platforms.email.extra.session_by_subject: true: those two mails → two sessions; replies keep the matching Re: / In-Reply-To.
  3. source.chat_id on the event is still user@example.com (never a subject string).
  4. Document and multi-image sends honor metadata.thread_id.
python -m pytest tests/gateway/test_email.py tests/gateway/test_send_multiple_images.py::TestEmailMultiImage -q

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(email): …)
  • I searched for existing PRs (see Duplicate check)
  • My PR contains only changes related to this fix
  • I've run pytest tests/gateway/test_email.py tests/gateway/test_send_multiple_images.py::TestEmailMultiImage -q (47 + 2 passed)
  • I've added tests for my changes
  • I've tested on my platform: macOS 15 (dev) + Linux (CI)

Documentation & Housekeeping

  • I've updated relevant documentation
  • I've updated cli-config.yaml.example
  • N/A — no architecture / CONTRIBUTING / AGENTS.md change
  • Cross-platform: stdlib IMAP/SMTP, no Windows-specific I/O
  • N/A — no tool schema change

@Enough1122

Copy link
Copy Markdown

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

Summary

Adds opt-in per-subject email session isolation (session_by_subject / EMAIL_SESSION_BY_SUBJECT, default off): the session key and SMTP reply threading follow the normalized subject instead of one rolling slot per sender. Well-tested, including restart recovery of the subject from the context key.

Findings (all Non-blocking)

  • adapter.py:34-37 — _SUBJECT_REPLY_PREFIX_RE strips Re/Fw/Fwd plus CJK prefixes iteratively and _normalize_email_subject collapses whitespace. Good; note locale-specific prefixes beyond the listed CJK ones start a new session, which is the safe direction.
  • adapter.py:66-72 — env-wins-only-when-set handling is correct (_esecret_bool on an unset var would pin the flag off). Good.
  • adapter.py:100-105 — \0 separator in the context key cannot collide with a real address/subject. Good.
  • adapter.py:86 — _thread_context now grows per distinct subject with no cap (_seen_uids_max only caps UIDs). A high-volume mailbox with many unique subjects grows this dict unboundedly until restart. Consider an LRU/size cap. Non-blocking.
  • adapter.py:210,226,249 — send_image / send_multiple_images / send_document now thread metadata.thread_id into the SMTP context key. Intended, but any caller passing a non-normalized thread_id silently falls back to the sender-only key (_thread_context_key requires exact match). Fine while the session framework supplies the same normalized value it received.
  • adapter.py:167-174 — _new_reply fallback chain (live ctx → subject-from-key → "Hermes Agent") degrades gracefully after restart. Good.

Verdict

Careful, backward-compatible (default off), thoroughly tested. No blocking issues.

@santiagomalter

Copy link
Copy Markdown
Author

Thanks for the pass.

On the unbounded _thread_context: the sender-keyed dict was already unbounded before this PR (one entry per allowlisted sender, never trimmed). Isolation only multiplies entries by distinct normalized subjects for opted-in mailboxes. An LRU cap would drop live In-Reply-To anchors for older parallel threads — the exact clobber this fix is for — so I am leaving the cap out of this PR. Happy to follow up with a trimmer that mirrors _trim_seen_uids if maintainers want a hard ceiling.

The other notes match the intended contract: extra CJK/locale prefixes fail closed into a new session; outbound metadata.thread_id is the same normalized value build_source stamped on the inbound event.

@alt-glitch alt-glitch added type/feature New feature or request comp/plugins Plugin system and bundled plugins platform/email Email (IMAP/SMTP) adapter area/sessions Session lifecycle, resume, persistence, history sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P3 Low — cosmetic, nice to have labels Sep 6, 2026
@santiagomalter

Copy link
Copy Markdown
Author

On the type/feature / P3 labels: the change is opt-in so existing one-session-per-sender setups do not move, but the unfixed behavior is a bug, not a missing nicety.

Without isolation, two mails from the same allowlisted sender with different subjects share one Hermes session and one _thread_context slot. A new subject hijacks the in-flight turn, pollutes agent context, and outbound In-Reply-To / Subject attach to the sender's last inbound mail — so the far-side client (Gmail, etc.) threads the reply onto the wrong conversation. That is the report in #26277 and the session-isolation half of #27804 (type/bug).

thread_id on build_source is the same session-key mechanism Telegram topics / Slack thread_ts already use; email just never stamped it. Default-off is a compatibility constraint (do not rewrite existing email histories), not a signal that the defect is cosmetic.

Happy to retitle or split if maintainers want this filed strictly as fix / type/bug.

@kiwipaulrob

Copy link
Copy Markdown

Running this exact shape in production — normalized-subject slug as thread_id plus sender+thread reply-context routing, plugin-layer and config-gated as here. It fixed parallel-topic interference for a single allowlisted sender on generic IMAP/SMTP; without it the far-side Gmail client piled replies onto the wrong conversation and all topics shared one agent session. Happy to validate this port against current main once merged. The other half of #27804 (per-tool-call status-email volume) is still outstanding for us — watching for a follow-up there.

@santiagomalter

Copy link
Copy Markdown
Author

Thanks — that matches what this port is for (generic IMAP/SMTP, one allowlisted sender, parallel subjects, Gmail on the far side).

Status-email volume (#27804 part 2) is intentionally out of this PR so isolation can land without mixing display/tool_progress defaults. Happy to review a follow-up for that, and for the extras on #103196 (/new, rotation) once this opt-in path is in.

@alt-glitch alt-glitch added type/bug Something isn't working and removed type/feature New feature or request labels Sep 7, 2026
@santiagomalter

Copy link
Copy Markdown
Author

Duplicate-check addendum: #99814 (fix(email): key sessions and reply threading by Gmail X-GM-THRID, ntg-agent, 31 Aug) is an open plugin-layer implementation of #11422. I had cited the issue, not this PR — missed it in the original list.

It is Gmail-only / always-on (X-GM-THRID when the server advertises X-GM-EXT-1). This PR is opt-in subject isolation on any IMAP provider. Same adapter file, so they need a sequence: this PR first (generic, default-off), then #99814 rebased as the Gmail THRID + RFC 5322 layer. Noted on #26277.

@alt-glitch alt-glitch added type/feature New feature or request and removed type/bug Something isn't working labels Sep 7, 2026
@santiagomalter
santiagomalter force-pushed the fix/email-session-by-subject branch from 51ce142 to 29593d7 Compare September 10, 2026 06:43
Default stays one session per sender. With EMAIL_SESSION_BY_SUBJECT or
platforms.email.extra.session_by_subject, chat_id remains the sender
address and source.thread_id is the normalized subject so parallel
topics no longer share context or clobber In-Reply-To headers.

Fixes NousResearch#26277
@santiagomalter
santiagomalter force-pushed the fix/email-session-by-subject branch from 29593d7 to 0b3c2ee Compare September 12, 2026 17:03

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

area/sessions Session lifecycle, resume, persistence, history comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/email Email (IMAP/SMTP) adapter sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: optional email session isolation by normalized subject

4 participants