feat(email): per-thread session routing + configurable display name - #63659
andrewKnown wants to merge 7 commits into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for addressing the real sender-scoped-session limitation: current main does route Email DMs from one sender through one chat_id (plugins/platforms/email/adapter.py:866-878).
Problems
- The PR uses the normalized subject as the entire DM
chat_id(plugins/platforms/email/adapter.py:898-918).build_session_key()uses that value directly for DMs (gateway/session.py:907-915), so two allowed senders with the same subject share a session and overwrite reply context. - The fallback at
plugins/platforms/email/adapter.py:954-959chooses the first stored sender for an unresolved thread. That can deliver a reply to an unrelated recipient. - Attachment paths bypass the resolver:
send_multiple_images()andsend_document()passchat_iddirectly to SMTP helpers (plugins/platforms/email/adapter.py:1100-1105,1177-1184), whoseToheaders receive that value (1120,1200). In thread mode it is a subject, not an address. - The PR adds no tests or documentation updates for the new routing/configuration behavior.
Suggested changes
- Preserve the recipient as
chat_id; use a recipient-scoped, stable thread identifier separately for session isolation. - Fail closed for unresolved reply targets; do not select an arbitrary cached sender.
- Route text and attachment sends through one recipient/context resolver, then add coverage for collisions, unknown threads, attachments, compatibility mode, and display names.
Automated hermes-sweeper review.
| r'^(?:re|fwd|fw):\s*', '', subject.strip(), flags=re.IGNORECASE | ||
| ).strip() | ||
| _chat_id = ( | ||
| _thread_subject |
There was a problem hiding this comment.
A subject alone cannot identify a private email thread: two allowed senders can both send Re: Invoice, and build_session_key() uses this chat_id directly for Email DMs. That merges their agent history and overwrites _thread_context; retain the recipient identity and use a separate recipient-scoped thread identifier.
| to_addr = ctx.get("sender_addr", "") | ||
| # If no sender_addr in context, try matching by scanning all contexts | ||
| if not to_addr: | ||
| for _tc in self._thread_context.values(): |
There was a problem hiding this comment.
This fallback selects the first cached sender for any unresolved chat_id, which can send a response to an unrelated recipient. Fail closed here; only use an explicit home target for proactive delivery.
|
@teknium1 reshaped per your review — no more composite chat_id. What changed: Your four points:
Improvements beyond the review: Message-ID anchor index (replies thread correctly even when a tool sends without thread metadata) and bounded thread-context maps (2000, oldest-half trim) for long-lived gateways. Context resolution order at send time: Migration note: email session key format changes one time (per-sender → per-thread sessions), which is the intended behavior of the feature. |
|
Follow-up found during live end-to-end verification of the reshape on a real gateway: two silent-drop bugs in the pre-dispatch sender gate (this PR's code path):
The failure mode is nasty: IMAP fetch marks the mail |
|
Rebased onto current |
Contribution packaging updateThis is no longer an always-on Iris-only patch. It's an opt-in platform feature with first-class switches:
Default is Recommended for email agents: platforms:
email:
session_routing: thread
display_name: Iris SloaneCore model still matches the Jul 16 review: 109 tests passing. Live-verified on a fleet gateway with real IMAP/SMTP mail. |
|
Hi @teknium1 — just following up on this one. Since the July 16 review I've reworked the PR along all four points and verified it end-to-end:
Also folded in two silent-drop fixes surfaced during live end-to-end verification (allow-all precedence and I believe all review feedback is now addressed — would appreciate a re-review whenever you have a moment. Happy to make any further adjustments. |
The Email platform adapter previously used the sender's email address
as the session key (chat_id), meaning all emails from the same person
shared a single conversation. This caused context pollution when an
email agent handled diverse requests (research, calendar, scheduling)
from the same sender.
Changes:
- Add config option (default: 'thread')
- 'thread': each email subject gets its own session (new default)
- 'sender': one session per sender address (legacy behaviour)
- Add config option for the From: header
- Configurable via platforms.email.display_name or EMAIL_DISPLAY_NAME env var
- Defaults to bare EMAIL_ADDRESS (no display name) if not set
- Update send() to resolve thread-based chat_id back to real recipient
address via thread context, with fallback to EMAIL_HOME_ADDRESS
- Wire EMAIL_SESSION_ROUTING and EMAIL_DISPLAY_NAME env vars in
gateway/config.py
Config example:
platforms:
email:
session_routing: thread # or: sender
display_name: 'Iris Sloane'
Closes the issue where email agents accumulate stale context across
unrelated threads from the same sender.
…er, unified send paths, tests Address PR NousResearch#63659 review feedback by teknium1: 1. Composite chat_id (sender:subject) - two senders with the same subject no longer share a session or overwrite thread context. 2. Fail-closed recipient resolution - _resolve_recipient() is the single source of truth for all send paths. Unknown chat_ids no longer pick an arbitrary cached sender. 3. Unified send paths - send(), send_multiple_images(), and send_document() all route through _resolve_recipient(). 4. Multi-prefix subject normalization - Re: Fwd: Subject strips both prefixes. 5. 13 new tests covering cross-sender isolation, session reuse, subject normalization, and recipient resolution edge cases.
…dback) Addresses teknium1's review by replacing the composite sender:subject chat_id with the platform's native DM session model: - chat_id is ALWAYS the sender's email address; thread_id carries the normalized subject. build_session_key() now isolates sessions per (sender, thread) exactly like Telegram topics / Discord threads. - Thread context keyed '<chat_id>::<thread_id>' with a Message-ID anchor index so replies thread correctly even without send-time metadata. - _context_for_send() resolution order: metadata.thread_id -> Message-ID anchor -> sender-mode key -> latest thread for the same mailbox. The last step can only affect subject/headers for a reply to the SAME address; delivery stays fail-closed (To: is chat_id or explicit EMAIL_HOME_ADDRESS only). - Bounded _thread_context/_msgid_context (2000, oldest-half trim). - All send paths (text, images, documents) resolve via _resolve_recipient and pass explicit ctx; To: is provably always an address. - Docs: Session Routing section + env reference in messaging/email.md. 105 tests passing: cross-sender isolation, same-sender multi-thread isolation, sender-mode legacy, context-leak, bounded trim, metadata and Message-ID send resolution, fail-closed/home fallback, multi-prefix normalization, and a To:-is-always-an-address regression.
…tokens Two silent-drop bugs at the adapter's pre-dispatch sender gate: 1. Allow-all was only consulted when EMAIL_ALLOWED_USERS was EMPTY. A stale allowlist (kept as documentation while EMAIL_ALLOW_ALL_USERS=true is set) silently dropped every sender not exactly listed. The gateway authz layer checks allow-all first (authz_mixin.py); the adapter now matches. 2. @Domain tokens (e.g. '@known.ltd') never matched: membership was exact-address only. Tokens now match any address on the domain; exact addresses still work. Failure mode was invisible: IMAP fetch marked mail seen, dispatch dropped it at a logger.debug gate, no New message log, no reply. Found during live end-to-end verification of the thread-id reshape on a fleet gateway. Regression tests: allow-all wins over stale allowlist, @Domain token matches, @Domain token rejects other domains. 108 tests passing.
Package the per-thread email session feature as an opt-in platform setting with discoverable setup surfaces, not an always-on behavior change. Core (already on this branch): - chat_id is always the sender mailbox; thread_id isolates threads when session_routing=thread (matches Telegram topics / Discord threads). - Fail-closed recipient resolution; Message-ID anchors; bounded context. - Allow-all precedence + @Domain allowlist tokens (silent-drop fix). This commit: - Default session_routing=sender (stock Hermes behaviour — no session-key migration for existing deployments). thread is opt-in. - hermes gateway setup: interactive_setup() for Email — credentials, allowlist/open-access, session routing choice, From display name. Also writes platforms.email.session_routing / display_name in config.yaml. - plugin.yaml optional_env: EMAIL_SESSION_ROUTING, EMAIL_DISPLAY_NAME, EMAIL_ALLOW_ALL_USERS. - Dashboard Channels env editor: labels + email env_vars list expanded. - Docs: Session Routing section documents default/safe-upgrade path and recommended config for email-agent secretaries. - Tests: 109 passing, including default-is-sender regression. Migration note when enabling thread: one-time session key change for that mailbox (per-sender sessions stop resuming). Intentional for email agents.
0b1eca1 to
3bc6129
Compare
Status update (conflicts cleared)Rebased Conflicts resolved
Local verification (canonical
PR should no longer show “This branch has conflicts.” Ready for re-review whenever you have a moment, @teknium1. |
…ster outbound Message-IDs Three related defects meant an email answer that carried files did not look like a person's reply. All three are in the email adapter. 1. Attachment mail built its own reply headers. _send_email_with_attachments / _send_email_with_attachment each resolved context with a bare self._thread_context.get(to_addr) lookup, which ALWAYS misses under session_routing="thread" (keys are "sender::thread_id"). The result was Subject "Re: Hermes Agent", no In-Reply-To, no References and no Cc, so mail clients filed every attachment mail into one unrelated conversation. Subject/Cc/In-Reply-To/References now come from a single _apply_reply_headers() that every send path calls, and References EXTENDS the inbound chain (capped at 20) instead of being reset to just the parent. An empty context now logs a warning, and attachment mail with no parent message-id logs an error, rather than silently emitting a generic subject. 2. A reply to the agent's own mail landed on the wrong thread. _msgid_context only ever indexed INBOUND Message-IDs, so a reply carrying In-Reply-To=<hermes-...> missed and fell through to the most-recent-thread scan in _context_for_send — answering on a different thread, with that thread's Cc list. Outbound Message-IDs are now registered against their thread as they are sent. 3. One turn produced several emails. The gateway dispatches an answer as send() for the body, then send_multiple_images() once for an image batch and send_document() once PER remaining file. On chat those are separate bubbles; on email they are separate messages in the recipient's thread. Outbound parts are now buffered per thread and flushed as a single MIME once the turn goes idle (fold_window_seconds, default 12s — measured body->attachment gaps were 4.1-7.9s; fold_max_wait_seconds caps the total). disconnect() flushes anything still buffered so a restart mid-turn cannot drop a reply. Also: thread state (contexts, message-id index, attachment hashes) persists to <HERMES_HOME>/state/email_thread_state.json, since it was memory-only and every restart made the next reply on a live thread lose its subject and Cc. And identical attachments are not re-sent on later turns of the same thread (content sha256, thread-scoped, so a revised file still goes out). Tests: 78 passed. Existing send tests updated for the buffered contract (they now flush before asserting the wire message) plus new coverage for one-email folding, reply-to-own-message thread resolution with a decoy thread present, and thread state surviving a restart.
|
Pushed The original commits fixed threading for text replies only. Any answer that carried attachments still went out as two messages: the one with the files had no Three fixes:
Thread state also persists to
|
…r successful send Two defects found while running the fold against the wider email suite: 1. _queue_part/_flush_* dereferenced _pending, _fold_window and _fold_max_wait directly. Tests and some proactive paths build the adapter via object.__new__(EmailAdapter), so those attributes may not exist and buffering raised AttributeError — losing the reply entirely. Read them through getattr with the documented defaults instead. 2. Attachment content hashes were recorded BEFORE the SMTP send. A failed flush therefore marked the files as delivered and permanently suppressed them on that thread — presenting exactly like the bug this branch fixes. Hashes now land only after send_message() returns. _flush_now also passes ctx/reply_to as keyword args so existing callers that unpack call_args.args positionally keep working. Tests: both defects are covered by new regression tests, each verified to fail when the fix is reverted. 90 passed across the five email-touching test files.
70f6e94 to
7718f86
Compare
|
+1 for this PR. Can it be added to code? |
|
@andrewKnown Friendly ping on this one 🙂 It closes two long-standing issues (#26277, #27804) and is still the most complete fix for email session/thread isolation. It's been about three weeks since the last status update — is there anything needed from our side to move it toward review? For triage context: my two smaller fixes in the same area are also still open — #56581 (strip compound channel prefix before SMTP To) and #58055 (extract SMTP recipient from session keys). If it helps review, I'm happy to fold either into this branch, or rebase them once this lands. |
Summary
The Email platform adapter previously used the sender's email address as the session key (
chat_id), meaning all emails from the same person shared a single conversation. This caused context pollution when an email agent handled diverse requests (research, calendar, scheduling) from the same sender.Changes
1. Per-thread session routing (
session_routing)New config option with two modes:
thread(default): Each email subject gets its own session. Replies within the same thread (same subject, stripped ofRe:/Fwd:) stay in the same session.sender: One session per sender address (legacy behaviour).The normalized subject is used as
chat_idinbuild_source(). The real recipient address is stored in_thread_contextand resolved back insend()when replying.2. Configurable display name (
display_name)Adds a
display_nameoption for theFrom:header in outgoing emails. Previously theFromheader was the bareEMAIL_ADDRESSwith no display name.From: "Iris Sloane" <iris@known.ltd>From: iris@known.ltd(legacy behaviour)Configuration
Or via environment variables:
Backward compatibility
session_routing: senderpreserves the exact legacy behaviourdisplay_nameunset = bare address (same as before)threadmode by default (the better behaviour)Send method resolution
When
session_routingisthread,chat_idis the subject string (not an email address). Thesend()method resolves the real recipient via:sender_addrin_thread_contextbychat_idsender_addrEMAIL_HOME_ADDRESSenv varchat_iditself if it contains@This ensures replies always go to the right recipient, even for Kanban notification deliveries that don't have thread context set.
Files changed
plugins/platforms/email/adapter.py— session routing logic, display name, send resolutiongateway/config.py— wireEMAIL_SESSION_ROUTINGandEMAIL_DISPLAY_NAMEenv varsFollow-up (commit
52b5410): one threaded reply per turnDogfooding the branch on a live mailbox surfaced three defects in the reply
path that the original commits did not cover. An email answer that carried
attachments did not look like a person's reply: it arrived as two messages, the
one with the files landed in an unrelated conversation, and everyone who had
been cc'd was dropped.
1. Attachment mail built its own reply headers
_send_email_with_attachmentsand_send_email_with_attachmenteach resolvedcontext with a bare
self._thread_context.get(to_addr)lookup. Undersession_routing: threadthat always misses, because keys are"sender::thread_id". So attachment mail went out as:SubjectRe: Hermes AgentRe: <the actual thread subject>In-Reply-ToMessage-IDReferencesCcMail clients had no threading headers to work with, so every attachment mail
collapsed into one unrelated conversation.
Subject/Cc/In-Reply-To/Referencesnow come from a single_apply_reply_headers()that every send path calls — the divergence was thebug, so the fix is one authority rather than three copies.
Referencesnowextends the inbound chain (capped at 20, root + most recent) instead of being
reset to just the parent, which had made long threads drift apart in Outlook.
An empty context logs a warning and attachment mail with no parent message-id
logs an error, rather than silently emitting a generic subject.
2. A reply to the agent's own mail landed on the wrong thread
_msgid_contextonly ever indexed inbound Message-IDs. A reply to one ofthe agent's own mails carries
In-Reply-To: <hermes-...>, which missed theindex and fell through to the most-recent-thread scan in
_context_for_send—answering on a different thread, with that thread's Cc list. Outbound
Message-IDs are now registered against their thread as they are sent, and the
thread's stored anchor advances so the next reply chains correctly.
3. One turn produced several emails
The gateway dispatches one answer as
send()for the body, thensend_multiple_images()once for an image batch andsend_document()onceper remaining file, with
human_delaysleeps in between. On chat those areseparate bubbles; on email each is a separate message in the recipient's thread.
Folding the body into just the first attachment would still leave one email per
subsequent file, so outbound parts are buffered per thread and flushed as a
single MIME once the turn goes idle.
fold_window_seconds(default 12) — idle debounce, re-armed by each new part.Measured body→attachment gaps on real turns were 4.1–7.9s.
fold_max_wait_seconds(default 90) — ceiling from the first part, so a longattachment loop cannot stall a reply.
disconnect()flushes anything still buffered, so a restart mid-turn cannotsilently drop a reply (worse than sending two).
Also in this commit
<HERMES_HOME>/state/email_thread_state.json(atomic write,
0600). It was memory-only, so every restart made the nextreply on a live thread lose its subject and Cc — the reported symptom,
reappearing because of a deploy. Resolved via
get_hermes_home()so it isper-profile and sandboxed under test rather than one shared file.
(content sha256, thread-scoped, so a genuinely revised file still goes out).
Configuration
Tests
78 passedintests/gateway/test_email.py,test_email_robustness.py,test_email_secret_scope.py.Four existing
send/send_documenttests asserted the old synchronouscontract (SMTP called during
send()); they now flush before asserting thewire message. New coverage:
test_one_turn_body_plus_files_is_a_single_email— body + 3 files produceone
send_messagecall carrying the body, all three attachments, and thethread's
Subject/Cc/In-Reply-To; asserts nothing is on the wire beforethe flush.
test_reply_to_own_message_resolves_original_thread— with a newer decoythread present for the same address, a reply to an outbound
<hermes-...>id still resolves the original thread and its Cc list. Without the decoy this
passes by accident, which is how the bug went undetected.
test_thread_state_survives_restart— state written, reloaded into a freshadapter, reply still resolves subject + Cc.
Verified end-to-end against a real Gmail mailbox: a 6-file request produced
exactly one message in
[Gmail]/Sent MailwithSubject: Re: <thread>,In-Reply-Toequal to the inboundMessage-ID,Ccpreserved, all sixattachments and a non-empty body; and a reply to the agent's own message on an
older thread — with a newer thread live for the same sender — stayed on the
correct thread.