resolve review feedback.

- eliminates panic opportunity on receiving email
- strict enum
- added unit test for ensuring that atmos headers don't leak onto
  directmx

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

crates/tranquil-comms/src/email/message.rs

@@ -52,23 +52,35 @@ pub(super) fn recipient_domain(message: &Message) -> Result<EmailDomain, SendErr
 }
 
 // for use with comail.at
-#[derive(Debug, Clone, PartialEq, Eq)]
-struct XAtmosCategory(&'static str);
-
-impl Header for XAtmosCategory {
+#[derive(Debug, Clone, PartialEq, Eq, Copy)]
+enum AtmosCategory {
+    PasswordReset,
+    MfaOtp,
+    Verification,
+}
+impl AtmosCategory {
+    fn as_str(self) -> &'static str {
+        match self {
+            Self::PasswordReset => "password-reset",
+            Self::MfaOtp => "mfa-otp",
+            Self::Verification => "verification",
+        }
+    }
+}
+impl Header for AtmosCategory {
     fn name() -> HeaderName {
         HeaderName::new_from_ascii_str("X-Atmos-Category")
     }
     fn parse(_s: &str) -> Result<Self, Box<dyn std::error::Error + Send + Sync>> {
         //since we're never receiving email, we don't care about parsing
-        unimplemented!()
+        Err("X-Atmos-Category is write-only".into())
     }
     fn display(&self) -> HeaderValue {
-        HeaderValue::new(Self::name(), self.0.to_string())
+        HeaderValue::new(Self::name(), self.as_str().to_string())
     }
 }
 
-fn atmos_category(comms_type: CommsType) -> Option<XAtmosCategory> {
+fn atmos_category(comms_type: CommsType) -> Option<AtmosCategory> {
     use CommsType::*;
     match comms_type {
         EmailVerification
@@ -78,10 +90,10 @@ fn atmos_category(comms_type: CommsType) -> Option<XAtmosCategory> {
         | LegacyLoginAlert
         | EmailUpdate
         | PlcOperation
-        | AccountDeletion => Some(XAtmosCategory("verification")),
-        PasswordReset | PasskeyRecovery => Some(XAtmosCategory("password-reset")),
-        TwoFactorCode => Some(XAtmosCategory("mfa-otp")),
-        Welcome => Some(XAtmosCategory("bulk")),
+        | AccountDeletion
+        | Welcome => Some(AtmosCategory::Verification),
+        PasswordReset | PasskeyRecovery => Some(AtmosCategory::PasswordReset),
+        TwoFactorCode => Some(AtmosCategory::MfaOtp),
         AdminEmail => None,
     }
 }

crates/tranquil-comms/src/email/mod.rs

@@ -31,22 +31,11 @@ pub struct EmailSender {
     from: Mailbox,
     mode: SendMode,
     dkim: Option<DkimSigner>,
-    atmos_categories: bool,
 }
 
 impl EmailSender {
-    pub fn new(
-        from: Mailbox,
-        mode: SendMode,
-        dkim: Option<DkimSigner>,
-        atmos_categories: bool,
-    ) -> Self {
-        Self {
-            from,
-            mode,
-            dkim,
-            atmos_categories,
-        }
+    pub fn new(from: Mailbox, mode: SendMode, dkim: Option<DkimSigner>) -> Self {
+        Self { from, mode, dkim }
     }
 
     pub fn from_config(cfg: &tranquil_config::TranquilConfig) -> Result<Option<Self>, SendError> {
@@ -66,19 +55,8 @@ impl EmailSender {
             Some(host) => build_smarthost(cfg, host)?,
             None => build_direct_mx(cfg)?,
         };
-        let atmos_categories = cfg.email.smarthost.apply_atmos_categories;
-        info!(
-            ?mode,
-            dkim = dkim.is_some(),
-            atmos_categories,
-            "Email sender initialized"
-        );
-        Ok(Some(Self {
-            from,
-            mode,
-            dkim,
-            atmos_categories,
-        }))
+        info!(?mode, dkim = dkim.is_some(), "Email sender initialized");
+        Ok(Some(Self { from, mode, dkim }))
     }
 }
 
@@ -146,6 +124,7 @@ fn build_smarthost(
     Ok(SendMode::Smarthost {
         transport: Box::new(builder.build()),
         total_timeout,
+        apply_atmos_categories: cfg.email.smarthost.apply_atmos_categories,
     })
 }
 
@@ -198,6 +177,16 @@ fn build_dkim(cfg: &tranquil_config::DkimConfig) -> Result<Option<DkimSigner>, S
     DkimSigner::load(selector, domain, path).map(Some)
 }
 
+fn wants_atmos_categories(mode: &SendMode) -> bool {
+    match mode {
+        SendMode::Smarthost {
+            apply_atmos_categories,
+            ..
+        } => *apply_atmos_categories,
+        SendMode::DirectMx { .. } => false,
+    }
+}
+
 #[async_trait]
 impl CommsSender for EmailSender {
     fn channel(&self) -> CommsChannel {
@@ -205,7 +194,8 @@ impl CommsSender for EmailSender {
     }
 
     async fn send(&self, notification: &QueuedComms) -> Result<(), SendError> {
-        let mut message = message::build(&self.from, notification, self.atmos_categories)?;
+        let mut message =
+            message::build(&self.from, notification, wants_atmos_categories(&self.mode))?;
         if let Some(signer) = &self.dkim {
             signer.sign(&mut message);
         }
@@ -218,3 +208,45 @@ impl CommsSender for EmailSender {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use lettre::Tokio1Executor;
+    use std::time::Duration;
+
+    fn dummy_smarthost(apply_atmos_categories: bool) -> SendMode {
+        let transport =
+            AsyncSmtpTransport::<Tokio1Executor>::builder_dangerous("localhost").build();
+        SendMode::Smarthost {
+            transport: Box::new(transport),
+            total_timeout: Duration::from_secs(10),
+            apply_atmos_categories,
+        }
+    }
+
+    fn dummy_direct_mx() -> SendMode {
+        SendMode::DirectMx {
+            resolver: Arc::new(TokioAsyncResolver::tokio(
+                ResolverConfig::default(),
+                ResolverOpts::default(),
+            )),
+            helo: HeloName::parse("mta.nel.pet").unwrap(),
+            command_timeout: Duration::from_secs(5),
+            total_timeout: Duration::from_secs(10),
+            require_tls: false,
+            inflight: Arc::new(Semaphore::new(1)),
+        }
+    }
+
+    #[tokio::test]
+    async fn smarthost_reflects_its_own_flag() {
+        assert!(wants_atmos_categories(&dummy_smarthost(true)));
+        assert!(!wants_atmos_categories(&dummy_smarthost(false)));
+    }
+
+    #[test]
+    fn direct_mx_never_wants_atmos_categories() {
+        assert!(!wants_atmos_categories(&dummy_direct_mx()));
+    }
+}

crates/tranquil-comms/src/email/transport.rs

@@ -19,6 +19,7 @@ pub enum SendMode {
     Smarthost {
         transport: Box<AsyncSmtpTransport<Tokio1Executor>>,
         total_timeout: Duration,
+        apply_atmos_categories: bool,
     },
     DirectMx {
         resolver: Arc<TokioAsyncResolver>,
@@ -33,8 +34,15 @@ pub enum SendMode {
 impl std::fmt::Debug for SendMode {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            Self::Smarthost { total_timeout, .. } => {
-                write!(f, "SendMode::Smarthost(total_timeout={total_timeout:?})")
+            Self::Smarthost {
+                total_timeout,
+                apply_atmos_categories,
+                ..
+            } => {
+                write!(
+                    f,
+                    "SendMode::Smarthost(total_timeout={total_timeout:?}, apply_atmos_categories={apply_atmos_categories:?})"
+                )
             }
             Self::DirectMx {
                 helo, require_tls, ..
@@ -52,6 +60,7 @@ pub async fn dispatch(mode: &SendMode, message: Message) -> Result<(), SendError
         SendMode::Smarthost {
             transport,
             total_timeout,
+            ..
         } => with_total_timeout(*total_timeout, run_send(transport, message)).await,
         SendMode::DirectMx {
             resolver,

crates/tranquil-comms/tests/email_smtp.rs

@@ -53,9 +53,9 @@ fn build_smarthost_sender_with_total_timeout(
         SendMode::Smarthost {
             transport: Box::new(transport),
             total_timeout,
+            apply_atmos_categories: false,
         },
         None,
-        false,
     )
 }
 

crates/tranquil-config/src/lib.rs

@@ -1125,7 +1125,7 @@ pub struct SmarthostConfig {
     #[config(env = "MAIL_SMARTHOST_TOTAL_TIMEOUT_SECS", default = 60)]
     pub total_timeout_secs: u64,
 
-    // Apply Atmos/Comail.at categories for headers to meet AUP.
+    /// Apply Atmos/Comail.at categories for headers to meet AUP.
     #[config(env = "MAIL_APPLY_ATMOS_CATEGORIES", default = false)]
     pub apply_atmos_categories: bool,
 }