Merge PR #1077: fix(email): auto_reply_enabled should not block proactive sends
This commit is contained in:
@@ -108,11 +108,6 @@ class EmailChannel(BaseChannel):
|
|||||||
logger.warning("Skip email send: consent_granted is false")
|
logger.warning("Skip email send: consent_granted is false")
|
||||||
return
|
return
|
||||||
|
|
||||||
force_send = bool((msg.metadata or {}).get("force_send"))
|
|
||||||
if not self.config.auto_reply_enabled and not force_send:
|
|
||||||
logger.info("Skip automatic email reply: auto_reply_enabled is false")
|
|
||||||
return
|
|
||||||
|
|
||||||
if not self.config.smtp_host:
|
if not self.config.smtp_host:
|
||||||
logger.warning("Email channel SMTP host not configured")
|
logger.warning("Email channel SMTP host not configured")
|
||||||
return
|
return
|
||||||
@@ -122,6 +117,15 @@ class EmailChannel(BaseChannel):
|
|||||||
logger.warning("Email channel missing recipient address")
|
logger.warning("Email channel missing recipient address")
|
||||||
return
|
return
|
||||||
|
|
||||||
|
# Determine if this is a reply (recipient has sent us an email before)
|
||||||
|
is_reply = to_addr in self._last_subject_by_chat
|
||||||
|
force_send = bool((msg.metadata or {}).get("force_send"))
|
||||||
|
|
||||||
|
# autoReplyEnabled only controls automatic replies, not proactive sends
|
||||||
|
if is_reply and not self.config.auto_reply_enabled and not force_send:
|
||||||
|
logger.info("Skip automatic email reply to {}: auto_reply_enabled is false", to_addr)
|
||||||
|
return
|
||||||
|
|
||||||
base_subject = self._last_subject_by_chat.get(to_addr, "nanobot reply")
|
base_subject = self._last_subject_by_chat.get(to_addr, "nanobot reply")
|
||||||
subject = self._reply_subject(base_subject)
|
subject = self._reply_subject(base_subject)
|
||||||
if msg.metadata and isinstance(msg.metadata.get("subject"), str):
|
if msg.metadata and isinstance(msg.metadata.get("subject"), str):
|
||||||
|
|||||||
@@ -169,7 +169,8 @@ async def test_send_uses_smtp_and_reply_subject(monkeypatch) -> None:
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_send_skips_when_auto_reply_disabled(monkeypatch) -> None:
|
async def test_send_skips_reply_when_auto_reply_disabled(monkeypatch) -> None:
|
||||||
|
"""When auto_reply_enabled=False, replies should be skipped but proactive sends allowed."""
|
||||||
class FakeSMTP:
|
class FakeSMTP:
|
||||||
def __init__(self, _host: str, _port: int, timeout: int = 30) -> None:
|
def __init__(self, _host: str, _port: int, timeout: int = 30) -> None:
|
||||||
self.sent_messages: list[EmailMessage] = []
|
self.sent_messages: list[EmailMessage] = []
|
||||||
@@ -201,6 +202,11 @@ async def test_send_skips_when_auto_reply_disabled(monkeypatch) -> None:
|
|||||||
cfg = _make_config()
|
cfg = _make_config()
|
||||||
cfg.auto_reply_enabled = False
|
cfg.auto_reply_enabled = False
|
||||||
channel = EmailChannel(cfg, MessageBus())
|
channel = EmailChannel(cfg, MessageBus())
|
||||||
|
|
||||||
|
# Mark alice as someone who sent us an email (making this a "reply")
|
||||||
|
channel._last_subject_by_chat["alice@example.com"] = "Previous email"
|
||||||
|
|
||||||
|
# Reply should be skipped (auto_reply_enabled=False)
|
||||||
await channel.send(
|
await channel.send(
|
||||||
OutboundMessage(
|
OutboundMessage(
|
||||||
channel="email",
|
channel="email",
|
||||||
@@ -210,6 +216,7 @@ async def test_send_skips_when_auto_reply_disabled(monkeypatch) -> None:
|
|||||||
)
|
)
|
||||||
assert fake_instances == []
|
assert fake_instances == []
|
||||||
|
|
||||||
|
# Reply with force_send=True should be sent
|
||||||
await channel.send(
|
await channel.send(
|
||||||
OutboundMessage(
|
OutboundMessage(
|
||||||
channel="email",
|
channel="email",
|
||||||
@@ -222,6 +229,56 @@ async def test_send_skips_when_auto_reply_disabled(monkeypatch) -> None:
|
|||||||
assert len(fake_instances[0].sent_messages) == 1
|
assert len(fake_instances[0].sent_messages) == 1
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_send_proactive_email_when_auto_reply_disabled(monkeypatch) -> None:
|
||||||
|
"""Proactive emails (not replies) should be sent even when auto_reply_enabled=False."""
|
||||||
|
class FakeSMTP:
|
||||||
|
def __init__(self, _host: str, _port: int, timeout: int = 30) -> None:
|
||||||
|
self.sent_messages: list[EmailMessage] = []
|
||||||
|
|
||||||
|
def __enter__(self):
|
||||||
|
return self
|
||||||
|
|
||||||
|
def __exit__(self, exc_type, exc, tb):
|
||||||
|
return False
|
||||||
|
|
||||||
|
def starttls(self, context=None):
|
||||||
|
return None
|
||||||
|
|
||||||
|
def login(self, _user: str, _pw: str):
|
||||||
|
return None
|
||||||
|
|
||||||
|
def send_message(self, msg: EmailMessage):
|
||||||
|
self.sent_messages.append(msg)
|
||||||
|
|
||||||
|
fake_instances: list[FakeSMTP] = []
|
||||||
|
|
||||||
|
def _smtp_factory(host: str, port: int, timeout: int = 30):
|
||||||
|
instance = FakeSMTP(host, port, timeout=timeout)
|
||||||
|
fake_instances.append(instance)
|
||||||
|
return instance
|
||||||
|
|
||||||
|
monkeypatch.setattr("nanobot.channels.email.smtplib.SMTP", _smtp_factory)
|
||||||
|
|
||||||
|
cfg = _make_config()
|
||||||
|
cfg.auto_reply_enabled = False
|
||||||
|
channel = EmailChannel(cfg, MessageBus())
|
||||||
|
|
||||||
|
# bob@example.com has never sent us an email (proactive send)
|
||||||
|
# This should be sent even with auto_reply_enabled=False
|
||||||
|
await channel.send(
|
||||||
|
OutboundMessage(
|
||||||
|
channel="email",
|
||||||
|
chat_id="bob@example.com",
|
||||||
|
content="Hello, this is a proactive email.",
|
||||||
|
)
|
||||||
|
)
|
||||||
|
assert len(fake_instances) == 1
|
||||||
|
assert len(fake_instances[0].sent_messages) == 1
|
||||||
|
sent = fake_instances[0].sent_messages[0]
|
||||||
|
assert sent["To"] == "bob@example.com"
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_send_skips_when_consent_not_granted(monkeypatch) -> None:
|
async def test_send_skips_when_consent_not_granted(monkeypatch) -> None:
|
||||||
class FakeSMTP:
|
class FakeSMTP:
|
||||||
|
|||||||
Reference in New Issue
Block a user