fix(cron): prevent delivery loop crash on multi-target email delivery by swissly · Pull Request #47167 · NousResearch/hermes-agent

swissly · 2026-06-16T09:40:31Z

Summary

Fixes two bugs that prevent cron jobs from delivering to multiple email targets:

Delivery loop crash — The standalone path in _deliver_result() had an unhandled exception boundary. When asyncio.run() raised RuntimeError (running event loop), the thread pool fallback could itself raise — but that exception escaped the except chain, crashing the entire delivery loop. All targets after the first failure were silently skipped.
Standalone SMTP_SSL — The standalone _send_email() in send_message_tool.py used smtplib.SMTP() + starttls() for all ports, which fails on port 465 (implicit TLS). This was already fixed in the gateway email adapter (fix(email): use implicit TLS for port 465 #46084) but not in the standalone send path.

Fixes #47163

Changes

cron/scheduler.py: Wrap the thread pool fallback in _deliver_result() in its own try/except so failures are logged and the loop continues to remaining targets.
tools/send_message_tool.py: Use SMTP_SSL for port 465 in the standalone _send_email(), matching the gateway adapter's behavior.

How to Verify

Create a cron job with deliver: "email:user1@example.com,email:user2@example.com"
Trigger the job
Both recipients should receive the email
If one fails, the other should still be delivered and the error logged

Testing

✅ Syntax check passed (py_compile on both files)
✅ Manual verification: delivery loop now continues after per-target failures

Checklist

Only cron/scheduler.py and tools/send_message_tool.py touched (2 files, 15 insertions, 5 deletions)
Conventional Commits format
Fixes the whole bug class (loop crash + standalone SMTP_SSL), not just one site
No tool schema change

The standalone delivery path in _deliver_result() had an unhandled exception boundary: when asyncio.run() raised RuntimeError (running loop), the thread pool fallback could itself raise — but that exception escaped the except chain, crashing the entire delivery loop. Any targets after the first failure were silently skipped. Fix: wrap the thread pool fallback in its own try/except so failures are logged and the loop continues to remaining targets. Also fix the standalone _send_email() in send_message_tool.py to use SMTP_SSL for port 465 (implicit TLS), matching the fix already applied to the gateway email adapter in NousResearch#46084. Fixes NousResearch#47163

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Improves message delivery robustness by handling SMTP implicit TLS on port 465 and adding error handling around async delivery retries in the scheduler.

Changes:

Use SMTP_SSL for implicit TLS on port 465; otherwise use SMTP + STARTTLS.
Wrap threaded asyncio.run(_send_to_platform(...)) retry in a try/except to record delivery failures and continue processing.

Reviewed changes

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

File	Description
tools/send_message_tool.py	Add implicit TLS handling for SMTP port 465 and set socket timeouts.
cron/scheduler.py	Add exception handling around the threaded async retry path and append delivery errors.

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

+        # Port 465 requires implicit TLS (SMTP_SSL); other ports use STARTTLS.
+        if smtp_port == 465:
+            server = smtplib.SMTP_SSL(smtp_host, smtp_port, timeout=30)
+        else:
+            server = smtplib.SMTP(smtp_host, smtp_port, timeout=30)
+            server.starttls(context=ssl.create_default_context())


-        server.starttls(context=ssl.create_default_context())
+        # Port 465 requires implicit TLS (SMTP_SSL); other ports use STARTTLS.
+        if smtp_port == 465:
+            server = smtplib.SMTP_SSL(smtp_host, smtp_port, timeout=30)


+                    with concurrent.futures.ThreadPoolExecutor(max_workers=1) as pool:
+                        future = pool.submit(asyncio.run, _send_to_platform(platform, pconfig, chat_id, cleaned_delivery_content, thread_id=thread_id, media_files=media_files))
+                        result = future.result(timeout=30)
+                except Exception as e:


+                except Exception as e:
+                    msg = f"delivery to {platform_name}:{chat_id} failed: {e}"
+                    logger.error("Job '%s': %s", job["id"], msg)


- Normalize smtp_port to int before comparison (string env vars) - Pass explicit SSL context to SMTP_SSL for consistent cert verification - Use explicit ThreadPoolExecutor shutdown(wait=False) for effective timeout - Add exc_info=True to error logs for full traceback preservation

swissly · 2026-06-16T09:46:30Z

Thanks for the thorough review! All 4 findings addressed in 11d3527:

Port comparison — smtp_port normalized with int() before comparing to 465
SMTP_SSL context — shared ssl.create_default_context() passed to both SMTP_SSL and starttls()
ThreadPoolExecutor timeout — replaced context manager with explicit pool.shutdown(wait=False) in finally block so future.result(timeout=30) actually caps wait time
Missing traceback — added exc_info=True to the error log for full stack trace preservation

swissly · 2026-06-16T09:56:21Z

Review update: overlap with #27680

During self-review (Step 0 — duplicate search), I discovered that PR #27680 by @timothykersten already implements the SMTP_SSL fix for tools/send_message_tool.py (plus gateway adapter + 62 tests).

Action plan:

Once fix(email): use implicit TLS for SMTPS port 465 #27680 lands, I'll rebase and remove the send_message_tool.py changes from this PR
This PR will then focus purely on the scheduler delivery loop crash (the unique fix for bug(cron): multi-target email delivery only sends to first recipient #47163)

The two fixes are orthogonal concerns:

fix(email): use implicit TLS for SMTPS port 465 #27680: SMTP transport layer (port 465 compatibility)
fix(cron): prevent delivery loop crash on multi-target email delivery #47167: Delivery dispatch layer (loop crash on multi-target)

Address all remaining pr-preparation-checklist findings: - send_message_tool.py: wrap server.login/send in try/finally for SMTP cleanup - scheduler.py: add exc_info=True to outer exception handler (line 846) - tests: add TestMultiTargetDeliveryContinuesOnFailure (2 tests) All 138 scheduler tests pass.

swissly · 2026-06-21T20:26:56Z

Hi! This PR has been approved for a few days — just checking if there's anything else needed before merge. The Copilot comments were on the old commit (b018d4c) and have been addressed in the updated push (d96fcd6). Happy to address any additional feedback!

teknium1 · 2026-07-01T10:49:12Z

Merged via #56269 — your fix landed on main in commit 242c963 with your authorship preserved in git history. Thanks for the clean diagnosis and tests.

Notes on the salvage:

Kept your core fix: wrapping the delivery thread-pool fallback in its own try/except so a per-target failure can't escape the except RuntimeError block and crash the whole loop. This restores documented multi-target delivery (email:a,email:b).
Dropped the tools/send_message_tool.py SMTP_SSL hunk — that standalone _send_email was refactored into plugins/platforms/email/adapter.py::_standalone_send (tracking: gateway platform → bundled-plugin migration #41112), which already handles port 465 via SMTP_SSL (plus an IPv4 fallback). That part was already on main.
Adapted the error accumulation to the current per-target target_errors pattern and verified your regression tests fail without the fix.

Copilot AI review requested due to automatic review settings June 16, 2026 09:40

Copilot AI reviewed Jun 16, 2026

View reviewed changes

alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management P1 High — major feature broken, no workaround labels Jun 16, 2026

swissly mentioned this pull request Jun 16, 2026

fix(email): use implicit TLS for SMTPS port 465 #27680

Closed

srojk34 mentioned this pull request Jun 16, 2026

fix(tools): add IPv4 fallback and port-465 support to standalone email send #47269

Open

4 tasks

NullStateGGH approved these changes Jun 16, 2026

View reviewed changes

teknium1 closed this Jul 1, 2026

Navigation Menu

Uh oh!

fix(cron): prevent delivery loop crash on multi-target email delivery - #47167

fix(cron): prevent delivery loop crash on multi-target email delivery#47167
swissly wants to merge 3 commits into
NousResearch:mainfrom
swissly:fix/cron-multi-email-delivery

swissly commented Jun 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

swissly commented Jun 16, 2026

Uh oh!

swissly commented Jun 16, 2026

Uh oh!

swissly commented Jun 21, 2026

Uh oh!

teknium1 commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

swissly commented Jun 16, 2026

Summary

Changes

How to Verify

Testing

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

swissly commented Jun 16, 2026

Uh oh!

swissly commented Jun 16, 2026

Review update: overlap with #27680

Uh oh!

swissly commented Jun 21, 2026

Uh oh!

teknium1 commented Jul 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants