Skip to content

fix(email): strip compound channel prefix before SMTP To header - #56581

Open
liuhao1024 wants to merge 2 commits into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-56569-email-compound-channel-id
Open

liuhao1024 wants to merge 2 commits into
NousResearch:mainfrom
liuhao1024:liuhao/cron-bugfix-56569-email-compound-channel-id

Conversation

@liuhao1024

Copy link
Copy Markdown
Contributor

What does this PR do?

Strips the email: platform prefix from compound channel IDs before using them as the SMTP To header. When the cron delivery pipeline or session keys carry a compound channel ID like email:user@example.com, strict SMTP servers (Gmail) reject the message with 555 5.5.2 because email:user@example.com is not a valid RFC 5321 address.

Related Issue

Fixes #56569

Type of Change

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

Changes Made

  • plugins/platforms/email/adapter.py: Added _strip_channel_prefix() helper that strips the email: prefix from channel identifiers. Applied it in all 4 SMTP send paths:
    • EmailAdapter._send_email() — adapter reply path
    • EmailAdapter._send_email_with_attachments() — multi-attachment path
    • EmailAdapter._send_email_with_attachment() — single-attachment path
    • _standalone_send() — standalone/cron delivery path
  • tests/gateway/test_email.py: Added TestStripChannelPrefix with 5 tests covering prefix stripping, bare addresses, empty strings, and both SMTP send paths with compound IDs

How to Test

  1. Run python -m pytest tests/gateway/test_email.py::TestStripChannelPrefix -xvs — all 5 tests should pass
  2. Run python -m pytest tests/gateway/test_email.py -x — full email test suite (93 tests) should pass
  3. Verify that msg["To"] receives user@example.com (not email:user@example.com) when a compound channel ID is passed to any SMTP send path

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/gateway/test_email.py -q and all tests pass (93 passed)
  • I've added tests for my changes
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation — or N/A (internal helper, no user-facing docs)
  • I've updated cli-config.yaml.example — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md — or N/A
  • I've considered cross-platform impact — N/A (Python stdlib email, no platform-specific code)
  • I've updated tool descriptions/schemas — or N/A

Screenshots / Logs

tests/gateway/test_email.py::TestStripChannelPrefix::test_send_email_strips_prefix PASSED
tests/gateway/test_email.py::TestStripChannelPrefix::test_standalone_send_strips_prefix PASSED
tests/gateway/test_email.py::TestStripChannelPrefix::test_strip_email_prefix PASSED
tests/gateway/test_email.py::TestStripChannelPrefix::test_strip_empty PASSED
tests/gateway/test_email.py::TestStripChannelPrefix::test_strip_no_prefix PASSED
93 passed, 1 warning in 3.97s

The cron delivery pipeline and session keys sometimes carry a compound
channel ID like email:user@example.com.  All four SMTP send paths
(_send_email, _send_email_with_attachments, _send_email_with_attachment,
_standalone_send) used this value directly as the SMTP To header, causing
strict servers (Gmail) to reject with 555 5.5.2.

Add _strip_channel_prefix() helper and apply it at every SMTP send site.
5 regression tests cover prefix stripping, bare addresses, empty strings,
and both the adapter and standalone send paths.

Fixes NousResearch#56569
@alt-glitch alt-glitch added type/bug Something isn't working platform/email Email (IMAP/SMTP) adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages P2 Medium — degraded but workaround exists labels Jul 1, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related: fixes #56569 (email adapter using the compound email: channel ID as the SMTP To header). Fix PR for that issue — related, not a duplicate.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the localized defensive fix. The stated cron/session-key route is not demonstrated against current main.

Problems

  • Direct target parsing removes the outer email: selector before dispatch (tools/send_message_tool.py:361-369, tools/send_message_tool.py:586-589), and explicit cron targets use the same parser (cron/scheduler.py:1128-1159). Inbound email sources are built from the bare extracted sender address (plugins/platforms/email/adapter.py:872-878). The new tests call the low-level helpers directly, so they do not establish the reported production path.
  • The diff modifies all three adapter SMTP builders, but tests cover only _send_email and _standalone_send; the attachment helpers have no regression assertion.

Suggested changes

  • Add a test that reaches a supported route which actually passes a compound recipient to SMTP, if one exists; otherwise describe this as defensive low-level normalization.
  • Cover both attachment send helpers, or centralize the normalization at their common boundary.

Automated hermes-sweeper review.

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

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(cron/email): email adapter uses compound channel ID as SMTP To: address — Gmail rejects with 555 5.5.2

3 participants