diff --git a/openspec/changes/email-reply-to-addresses/.openspec.yaml b/openspec/changes/email-reply-to-addresses/.openspec.yaml new file mode 100644 index 0000000..d1c6cc6 --- /dev/null +++ b/openspec/changes/email-reply-to-addresses/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-02-27 diff --git a/openspec/changes/email-reply-to-addresses/design.md b/openspec/changes/email-reply-to-addresses/design.md new file mode 100644 index 0000000..fb1d54e --- /dev/null +++ b/openspec/changes/email-reply-to-addresses/design.md @@ -0,0 +1,47 @@ +## Context + +The system sends emails in two directions with two different purposes: + +- **Outbound to parents**: The AI agent sends conversational replies and, upon registration completion, a confirmation email summarising the registration and next steps. +- **Outbound to admins/leaders**: The system sends a notification email to the relevant playgroup leader(s) and Markus Graf (CC) immediately after a registration is completed. + +Email clients use the `Reply-To` header (falling back to `From`) to determine where a reply is directed. Without explicit `Reply-To` headers, all replies from both parents and admins flow back to the registration system's inbox — which is correct for ongoing conversation but wrong for post-completion follow-up. + +## Goals / Non-Goals + +**Goals:** +- Ensure parent replies to completion confirmation emails reach the admin (Markus Graf) rather than the agent pipeline +- Ensure admin/leader replies to registration notification emails reach the registering parent directly +- Formally specify `Reply-To` behavior in the relevant capability specs + +**Non-Goals:** +- Changing the `From` address of any email +- Modifying conversational email behavior (mid-registration agent ↔ parent exchanges — these correctly use the registration address as both From and effective reply target) +- Introducing any new email addresses beyond what is already configured + +## Decisions + +### 1. Confirmation Email Reply-To: Admin Address + +**Decision**: Set `Reply-To: spielgruppen@familien-verein.ch` (Markus Graf) on all confirmation emails sent to parents after registration completion. + +**Rationale**: Once registration is complete, the conversation is over. Any parent reply is a human follow-up question — it should reach a human admin, not re-enter the agent pipeline. Markus Graf is the designated central admin contact and is already CC'd on all notifications. + +**Alternatives considered**: +- No Reply-To (default to From): Parent replies re-enter the agent inbox and may trigger unwanted agent responses post-completion. +- Reply-To the relevant playgroup leader (Andrea/Barbara): More targeted, but leaders vary by registration type and parents may not know who they're reaching. The central admin address is simpler and consistent. + +### 2. Notification Email Reply-To: Parent Address + +**Decision**: Set `Reply-To: ` on all registration notification emails sent to admins/leaders. + +**Rationale**: The primary reason admins reply to a notification is to contact the parent (e.g., to confirm a spot, ask a clarifying question, or provide further instructions). Pre-filling Reply-To with the parent's address eliminates a copy-paste step and reduces errors. This is already noted informally in `notification-template.md` — this change formalises it as a spec requirement. + +**Alternatives considered**: +- No Reply-To (default to From/registration inbox): Admins must manually copy the parent's email to reply, adding friction. + +## Risks / Trade-offs + +**Admin confirmation email replies go to Markus, not directly to the leader**: For outdoor registrations, the leader is Barbara Gross, but parent replies to confirmation go to Markus Graf. This is acceptable — Markus can forward as needed, and having a single consistent Reply-To is simpler than routing by playgroup type. + +**Mid-registration vs. post-registration distinction**: Conversational emails (mid-registration) should NOT set Reply-To to the admin — they must continue flowing back to the registration inbox so the agent can process them. The implementation must apply the admin Reply-To only to the final confirmation email, not to all outbound agent emails. diff --git a/openspec/changes/email-reply-to-addresses/proposal.md b/openspec/changes/email-reply-to-addresses/proposal.md new file mode 100644 index 0000000..94e253f --- /dev/null +++ b/openspec/changes/email-reply-to-addresses/proposal.md @@ -0,0 +1,21 @@ +## Why + +The system sends two distinct types of emails to different audiences with different needs for follow-up communication. Currently, the Reply-To behavior for these emails is either unspecified or inconsistent: + +1. **Confirmation emails to parents** — sent by the agent after registration is complete. If a parent replies to this email, that reply should reach the admin (Markus Graf), not bounce back into the agent's inbox for further automated processing. + +2. **Registration notification emails to playgroup leaders/admins** — sent to Andrea Sigrist, Barbara Gross, and Markus Graf when a new registration is submitted. If an admin wants to follow up with the parent directly, their reply must go to the parent's email, not back to the registration system. + +Without explicit Reply-To configuration, email clients will default to replying to the From address (the registration system's email). This creates confusion: parent replies to confirmation emails enter the agent pipeline instead of reaching a human admin, and admin replies to notification emails go to the registration inbox rather than the parent. + +## What Changes + +- **Confirmation emails**: Add a `Reply-To` header set to the admin email address (`spielgruppen@familien-verein.ch`) so parent replies reach a human directly +- **Admin notification emails**: Confirm and formally specify that `Reply-To` is set to the parent's email address so admins can respond to parents directly from their email client + +## Capabilities + +### Modified Capabilities + +- `email-channel`: Add Reply-To specification for confirmation emails sent to parents after registration completion +- `registration-notifications`: Formally specify Reply-To for admin notification emails (parent's email address) diff --git a/openspec/changes/email-reply-to-addresses/specs/email-channel/spec.md b/openspec/changes/email-reply-to-addresses/specs/email-channel/spec.md new file mode 100644 index 0000000..592c6d9 --- /dev/null +++ b/openspec/changes/email-reply-to-addresses/specs/email-channel/spec.md @@ -0,0 +1,15 @@ +## MODIFIED Requirements + +### Requirement: Confirmation email sets Reply-To to admin address + +The system SHALL set the `Reply-To` header to the admin email address (`spielgruppen@familien-verein.ch`) on the registration completion confirmation email sent to the parent. + +#### Scenario: Parent replies to confirmation email +- **WHEN** the agent sends a registration completion confirmation email to a parent +- **THEN** the email SHALL include a `Reply-To` header set to `spielgruppen@familien-verein.ch` +- **AND** a parent reply SHALL be delivered to the admin, not to the registration system's inbox + +#### Scenario: Mid-registration emails are unaffected +- **WHEN** the agent sends a conversational email during an ongoing registration (not the final confirmation) +- **THEN** the email SHALL NOT set `Reply-To` to the admin address +- **AND** parent replies SHALL continue to be routed back to the registration system for processing diff --git a/openspec/changes/email-reply-to-addresses/specs/registration-notifications/spec.md b/openspec/changes/email-reply-to-addresses/specs/registration-notifications/spec.md new file mode 100644 index 0000000..e42cea2 --- /dev/null +++ b/openspec/changes/email-reply-to-addresses/specs/registration-notifications/spec.md @@ -0,0 +1,18 @@ +## MODIFIED Requirements + +### Requirement: Notification email sets Reply-To to parent address + +The system SHALL set the `Reply-To` header to the registering parent's email address on all registration notification emails sent to playgroup leaders and the admin. + +#### Scenario: Leader replies to notification email +- **WHEN** a registration notification email is sent to a playgroup leader (Andrea Sigrist or Barbara Gross) +- **THEN** the email SHALL include a `Reply-To` header set to the parent's email address (`registration.parentGuardian.email`) +- **AND** a leader reply SHALL be delivered directly to the parent + +#### Scenario: Admin (CC) replies to notification email +- **WHEN** Markus Graf replies to a registration notification email (received as CC) +- **THEN** the reply SHALL be delivered directly to the parent's email address + +#### Scenario: Reply-To applies to all notification routing types +- **WHEN** a notification is sent for an indoor-only, outdoor-only, or both registration +- **THEN** all recipient copies (To and CC) SHALL have `Reply-To` set to the parent's email address diff --git a/openspec/changes/email-reply-to-addresses/tasks.md b/openspec/changes/email-reply-to-addresses/tasks.md new file mode 100644 index 0000000..ca0d231 --- /dev/null +++ b/openspec/changes/email-reply-to-addresses/tasks.md @@ -0,0 +1,19 @@ +## 1. Confirmation Email Reply-To (Email Channel) + +- [x] 1.1 Identify where the registration completion confirmation email is constructed in `src/` (email channel adapter / agent core) +- [x] 1.2 Add `Reply-To: spielgruppen@familien-verein.ch` header to the confirmation email only (not to mid-registration conversational emails) +- [x] 1.3 Add a unit test verifying the `Reply-To` header is present on the confirmation email +- [x] 1.4 Add a unit test verifying mid-registration emails do NOT carry the admin `Reply-To` header + +## 2. Notification Email Reply-To (Registration Notifications) + +- [x] 2.1 Identify where registration notification emails are constructed and sent +- [x] 2.2 Set `Reply-To: ` header on all outgoing notification emails (indoor, outdoor, and both routing types) +- [x] 2.3 Add a unit test verifying the `Reply-To` header equals the parent's email for indoor-only notification +- [x] 2.4 Add a unit test verifying the `Reply-To` header equals the parent's email for outdoor-only notification +- [x] 2.5 Add a unit test verifying the `Reply-To` header equals the parent's email when both leaders are notified + +## 3. Verification + +- [x] 3.1 Run the full test suite and confirm all tests pass +- [ ] 3.2 Manually send a test registration through the email channel and verify reply routing behaves correctly diff --git a/src/notifications/notifier.py b/src/notifications/notifier.py index 0e2eda4..a698220 100644 --- a/src/notifications/notifier.py +++ b/src/notifications/notifier.py @@ -215,6 +215,8 @@ class AdminNotifier: msg_outer["From"] = self._from_email msg_outer["To"] = parent_email msg_outer["Subject"] = subject + if self._cc_emails: + msg_outer["Reply-To"] = self._cc_emails[0] msg_alt = MIMEMultipart("alternative") msg_alt.attach(MIMEText(text_body, "plain", "utf-8")) diff --git a/tests/test_email_loop_prevention.py b/tests/test_email_loop_prevention.py index f25b27e..5d5d7ea 100644 --- a/tests/test_email_loop_prevention.py +++ b/tests/test_email_loop_prevention.py @@ -1,10 +1,11 @@ -"""Tests for email loop prevention. +"""Tests for email loop prevention and email channel reply headers. -Covers three layers: +Covers four areas: - detect_automated_message() — header-based bounce/automated sender detection - EmailAgent.handle_automated_message() — state tracking, one-shot admin alert - EmailAgent.process_message() — hard message-count cap (MAX_USER_MESSAGES) - AdminNotifier.notify_loop_escalation() — escalation email dispatch +- EmailChannel.send_reply() — mid-registration emails must not carry admin Reply-To """ import email @@ -13,7 +14,7 @@ import json import pytest from unittest.mock import MagicMock, patch -from src.channels.email_channel import detect_automated_message +from src.channels.email_channel import detect_automated_message, EmailChannel from src.agent.core import EmailAgent, MAX_USER_MESSAGES from src.models.conversation import ConversationState, ChatMessage from src.notifications.notifier import AdminNotifier @@ -650,3 +651,42 @@ class TestNotifyLoopEscalation: ) mock_smtp_cls.assert_not_called() + + +# --------------------------------------------------------------------------- +# EmailChannel.send_reply — mid-registration emails must not have admin Reply-To +# (task 1.4) +# --------------------------------------------------------------------------- + + +class TestSendReplyNoAdminReplyTo: + def test_send_reply_has_no_reply_to_header(self, mocker): + """Mid-registration conversational emails must not carry a Reply-To header.""" + channel = EmailChannel( + imap_host="imap.example.com", + imap_port=993, + smtp_host="smtp.example.com", + smtp_port=587, + username="agent@example.com", + password="secret", + use_ssl=True, + use_tls=True, + registration_email="agent@example.com", + ) + + mock_smtp_cls = mocker.patch("smtplib.SMTP") + captured = {} + + def fake_sendmail(from_, to_, msg_str): + captured["msg"] = msg_str + + mock_smtp_cls.return_value.sendmail.side_effect = fake_sendmail + + channel.send_reply( + to="parent@example.com", + subject="Re: Anmeldung", + body="Wie heisst dein Kind?", + ) + + parsed = email.message_from_string(captured["msg"]) + assert parsed.get("Reply-To") is None diff --git a/tests/test_notifier.py b/tests/test_notifier.py index 53960a8..ea407a5 100644 --- a/tests/test_notifier.py +++ b/tests/test_notifier.py @@ -345,3 +345,143 @@ class TestGenerateQrBillPng: """Output starts with the PNG magic bytes.""" png = notifier._generate_qr_bill_png() assert png[:4] == b"\x89PNG" + + +# --------------------------------------------------------------------------- +# Reply-To header — confirmation email to parent (task 1.3) +# --------------------------------------------------------------------------- + + +class TestNotifyParentReplyTo: + def test_confirmation_email_has_reply_to_admin( + self, notifier, complete_registration, mocker + ): + """Confirmation email sets Reply-To to the first CC (admin) address.""" + mock_smtp_cls = mocker.patch("smtplib.SMTP") + captured = {} + + def fake_sendmail(from_, to_, msg_str): + captured["msg"] = msg_str + + mock_smtp_cls.return_value.sendmail.side_effect = fake_sendmail + + notifier.notify_parent(complete_registration, language="de") + + parsed = email.message_from_string(captured["msg"]) + assert parsed.get("Reply-To") == "markus@example.com" + + def test_confirmation_email_no_reply_to_when_no_cc( + self, complete_registration, mocker + ): + """When no CC emails are configured, no Reply-To header is set.""" + notifier_no_cc = AdminNotifier( + smtp_host="smtp.example.com", + smtp_port=587, + username="agent@example.com", + password="secret", + from_email="agent@example.com", + cc_emails=[], + ) + mock_smtp_cls = mocker.patch("smtplib.SMTP") + captured = {} + + def fake_sendmail(from_, to_, msg_str): + captured["msg"] = msg_str + + mock_smtp_cls.return_value.sendmail.side_effect = fake_sendmail + + notifier_no_cc.notify_parent(complete_registration, language="de") + + parsed = email.message_from_string(captured["msg"]) + assert parsed.get("Reply-To") is None + + +# --------------------------------------------------------------------------- +# Reply-To header — admin notification emails (tasks 2.3, 2.4, 2.5) +# --------------------------------------------------------------------------- + + +class TestNotifyAdminReplyTo: + def _capture_msg(self, mocker): + """Return a side-effect function and a dict that captures the raw MIME string.""" + captured = {} + + mock_smtp_cls = mocker.patch("smtplib.SMTP") + + def fake_sendmail(from_, to_, msg_str): + captured["msg"] = msg_str + + mock_smtp_cls.return_value.sendmail.side_effect = fake_sendmail + return captured + + def test_indoor_notification_reply_to_is_parent_email( + self, notifier, complete_registration, mocker + ): + """Indoor-only notification sets Reply-To to the parent's email.""" + from src.models.registration import Booking, BookingDay + + complete_registration.booking = Booking( + playgroup_types=["indoor"], + selected_days=[BookingDay(day="monday", type="indoor")], + ) + captured = self._capture_msg(mocker) + + notifier.notify_admin( + complete_registration, + registration_id="reg-001", + version=1, + conversation_id="conv-001", + channel="email", + ) + + parsed = email.message_from_string(captured["msg"]) + assert parsed.get("Reply-To") == "anna.muster@example.com" + + def test_outdoor_notification_reply_to_is_parent_email( + self, notifier, complete_registration, mocker + ): + """Outdoor-only notification sets Reply-To to the parent's email.""" + from src.models.registration import Booking, BookingDay + + complete_registration.booking = Booking( + playgroup_types=["outdoor"], + selected_days=[BookingDay(day="monday", type="outdoor")], + ) + captured = self._capture_msg(mocker) + + notifier.notify_admin( + complete_registration, + registration_id="reg-002", + version=1, + conversation_id="conv-002", + channel="email", + ) + + parsed = email.message_from_string(captured["msg"]) + assert parsed.get("Reply-To") == "anna.muster@example.com" + + def test_both_types_notification_reply_to_is_parent_email( + self, notifier, complete_registration, mocker + ): + """Both-types notification sets Reply-To to the parent's email.""" + from src.models.registration import Booking, BookingDay + + complete_registration.booking = Booking( + playgroup_types=["indoor", "outdoor"], + selected_days=[ + BookingDay(day="monday", type="indoor"), + BookingDay(day="monday", type="outdoor"), + ], + ) + captured = self._capture_msg(mocker) + + notifier.notify_admin( + complete_registration, + registration_id="reg-003", + version=1, + conversation_id="conv-003", + channel="email", + ) + + parsed = email.message_from_string(captured["msg"]) + assert parsed.get("Reply-To") == "anna.muster@example.com"