Skip to content

fix(email): extract SMTP recipient from session keys - #58055

Open
tianma-if wants to merge 1 commit into
NousResearch:mainfrom
tianma-if:codex/fix-email-compound-recipient
Open

tianma-if wants to merge 1 commit into
NousResearch:mainfrom
tianma-if:codex/fix-email-compound-recipient

Conversation

@tianma-if

@tianma-if tianma-if commented Jul 4, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Defensively normalize SMTP recipients at the email adapter boundary, so a compound chat/session identifier cannot be written verbatim into a To: header.
  • Apply the same normalization to ordinary replies, standalone sends, single-file attachments, and multi-file attachments.
  • Preserve thread-context lookup by preferring the normalized address and falling back to the raw identifier.

Explicit email delivery currently strips supported email:user@example.com identifiers before reaching SMTP. This change intentionally hardens the low-level builders as well, rather than claiming a currently supported high-level route always forwards a compound key.

Tests

  • .venv/bin/python -m pytest tests/gateway/test_email.py -q (91 passed)
  • .venv/bin/python -m ruff check plugins/platforms/email/adapter.py tests/gateway/test_email.py
  • git diff --check

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/email Email (IMAP/SMTP) adapter P2 Medium — degraded but workaround exists sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 4, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Competing with #56581 for the same issue (#56569). This PR uses a regex to extract the last RFC5321 address from a raw session key (handles the deeper agent:main:email:dm:user@example.com form, applied to _send_email + _standalone_send); #56581 strips only a literal email: prefix but covers all four SMTP send paths. Same goal, different mechanism -> related, not duplicate. A maintainer should pick the approach or reconcile coverage (broader input handling vs. all send paths).

@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.

Thanks for the focused defensive normalization.

Problems

  • The diff fixes only _send_email() and _standalone_send(). The two attachment SMTP builders still copy raw to_addr into To on current main: plugins/platforms/email/adapter.py:1045 and plugins/platforms/email/adapter.py:1125.
  • The new regression calls EmailAdapter.send() directly with a synthetic session key. Current explicit email delivery parses email:user@example.com to a bare address in tools/send_message_tool.py:586-589; cron delivery uses the same parser at cron/scheduler.py:1132-1159. The PR does not establish a supported current route that forwards the compound key to SMTP.

Suggested changes

  • Normalize at all SMTP construction paths and add coverage for the two attachment builders.
  • Add a test through a verified compound-key routing path, if one exists; otherwise frame this as defensive low-level normalization.

Automated hermes-sweeper review.

@@ -925,12 +935,15 @@ def _send_email(
reply_to_msg_id: Optional[str] = None,

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 normalizes only the plain-body SMTP path. The separate attachment builders still assign raw to_addr to msg["To"] on current main (plugins/platforms/email/adapter.py:1045 and :1125); please centralize or apply the same normalization there and add coverage.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Jul 15, 2026
@tianma-if
tianma-if force-pushed the codex/fix-email-compound-recipient branch 2 times, most recently from b5bc09d to 7b3e8bc Compare July 16, 2026 11:17

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/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 type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants