ci(email): run the conversation-kit tests; mark Phase E complete

email.yml: the domain test floor ratchets 190 -> 205, and the nigig-email
job runs cargo test -p nigig-uikit --lib -- conversation so an email-driven
regression in the shared kit cannot silently surface in SMS.

The review doc marks E1-E6 done and records the honest correction E5
surfaced: lettre's timeout bounds only the TCP connect, not the
greeting/command reads — the send path now bounds the whole operation.

.forgejo/workflows/email.yml

@@ -409,11 +409,12 @@ jobs:
 
       # A floor, not a ratchet: these tests are cheap, pure, and the
       # number should only go up. 38 at Phase 0; 154 after C1c/C1d;
-      # 195 after the Phase C/D completion.
+      # 195 after the Phase C/D completion; 214 after Phase E (proptest +
+      # the SMTP sink integration tests).
       - name: The email domain test suite must not shrink
         run: |
           set -euo pipefail
-          FLOOR=190
+          FLOOR=205
           out="$(cargo test --locked -p nigig-core --lib -- email_ secret:: mail_backend:: mail_proxy:: imap_client:: credential_store:: 2>&1)"
 
       # C1e/C1f: the IMAP transport and the platform keystore are both
@@ -482,6 +483,14 @@ jobs:
       - name: Test
         run: cargo test --locked -p nigig-email
 
+      # E6: the shared conversation kit (nigig-uikit/src/shared/conversation)
+      # is consumed by BOTH SMS and email, so a regression there moves two
+      # features at once. It now has its own tests pinning the click guard
+      # and payload; run them so an email-driven regression cannot silently
+      # surface in SMS.
+      - name: Conversation kit tests
+        run: cargo test --locked -p nigig-uikit --lib -- conversation
+
       # Phase 0.6. A hard gate, not report-only: this crate is 1,100
       # lines and already formats clean, so there is no pre-existing
       # drift to grandfather in. Contrast sms.yml and nigig-map.yml,

Cargo.lock

@@ -2984,6 +2984,7 @@ dependencies = [
  "matrix_client",
  "nigig-system-prefs",
  "postcard",
+ "proptest",
  "rand 0.8.7",
  "reqwest",
  "robius-directories",

REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md

@@ -422,6 +422,21 @@ the unbounded hang I first assumed. But the application sets no timeout of its
 own, so the user watches `"Testing..."` for a full minute with no way to abort,
 and a `send` that spans several commands can exceed that.
 
+> **Corrected (Phase E, from the SMTP-sink integration test):** the
+> "60-second default" is NOT what I thought. I read lettre 0.11.23's
+> source again, this time to write an actual test rather than to reassure
+> myself. `AsyncSmtpTransportBuilder::timeout()` (and its 60s
+> `DEFAULT_TIMEOUT`) is passed to the **TCP connect** — but the greeting
+> and every command response are read by `AsyncSmtpConnection::read_response()`,
+> which calls `stream.read_line()` with **no timeout at all**. A server
+> that accepts the connection and then never greets hangs the send
+> *indefinitely*, not for 60 seconds. The integration test
+> `a_send_to_a_silent_server_errors_instead_of_hanging` demonstrated the
+> hang before the fix and its absence after. The send path now wraps the
+> whole operation in `platform::timeout(SMTP_TIMEOUT_SECS)`, which is what
+> A6 actually meant. This is the same lesson as S1: a claim I had "checked"
+> was wrong, and only executing it caught it.
+
 Combined with **B4** (no in-flight guard), a user who taps Send during those 60
 seconds queues a second delivery.
 
@@ -532,6 +547,7 @@ snapshot. Commits are on `main`.
 | *(this turn)* | **C1d + C1c DONE** — `mail_proxy.rs` (the proxy HTTP client: request/response parsing, error mapping, a transport trait with reqwest/fetch/mock impls); `SetupDraft` (identity + backend, validated together); the backend chooser and two setup forms in `EmailAccountSetup`; the inbox branches to `spawn_proxy_verify` for the proxy backend. Domain tests **126 → 154**. Coverage tooling (`tools/test-email-coverage.sh`, 93.4% line, floors enforced) wired into `email.yml`. |
 | *(this turn)* | **Phase C + D COMPLETE** — C1e (`imap_client.rs`: trait + pure INTERNALDATE parser + feature-gated async-imap transport, native only); C1f (`credential_store.rs`: trait + fail-closed default); C3 (`email_cache.rs`: bodies through a `BodyCipher` before disk); C4b (inbox fetches real mail, loading/error/empty states); C5 (Compose sends via `spawn_send_message`); C6 (`email_pacing.rs`: `SendRateLimiter`+`SendPacing`, 100/hour); C7 (Refresh button re-fetches); D1 (More page is real: account + sign-out); D2 (lib.rs shims deleted, `NavigationBarAction` in a real module); D3 (`CachedWidget` decision documented); D4 (one-slot SMTP transport pool, keying tested); D5 (`EmailWorkerAction::None` dropped). Domain tests **154 → 195**. Coverage now **90.7%** over 12 files, floors enforced. |
 | *(this turn)* | **C6/C7/C1f gaps closed** — C6: `email_bulk.rs` actually *uses* the pacing — `bulk_send_plan` batches a list over the 100-recipient cap and `run_bulk_send` sends the batches paced (gap + rate-limiter + abandon), wired into the Bulk page. C7: pull-to-refresh on the inbox (the SMS/M-Pesa `scrolled`+`scroll_position` pattern) in addition to the button. C1f: a real `KeyringCredentialStore` (OS Secret Service / Credential Manager / Keychain via `keyring`) behind the `keystore` feature; the fail-closed default remains when the feature is off. Domain tests **195 → 206**. |
+| *(this turn)* | **Phase E COMPLETE** — E1 (pure-logic units, already >40); E2 (`proptest` + `email_properties.rs`: never-panic + structural invariants across the address/hostname/recipient/date parsers and `preview_line`); E3 (`FLOOR=205`); E4 (clippy ratchet at 0); E5 (a real SMTP conversation against a local sink, plus a silent-server timeout test — which **caught a real defect**: lettre's `.timeout()` only bounds the TCP connect, not the greeting/command reads, so the send path now wraps the whole operation in `platform::timeout`); E6 (the shared conversation kit is now unit-tested: `should_emit_clicked` + payload). Domain tests **206 → 214**; `nigig-uikit` gains its first 5 tests. |
 
 **Phase 0 is complete.** All seven items done; 0.7 was fixed upstream.
 
@@ -800,12 +816,12 @@ presenting them as equivalent choices.
 
 | ID | Task |
 |---|---|
-| E1 | **Unit-test the pure logic**: port→transport, address parsing/splitting, validation, error mapping. Target ≥40 tests; these need no network. |
-| E2 | **Property-test address parsing** (proptest is already a dev-dep in SMS) — never panic on arbitrary input. |
-| E3 | **Test-count floor gate**, as SMS has (`FLOOR=100`). |
-| E4 | **Clippy ratchet** at the measured baseline. |
-| E5 | **Integration test** against a local SMTP sink (e.g. a `MockSmtp` listener on 127.0.0.1) — verifies the transport path without a real provider. |
-| **E6** | **NEW — test the shared conversation kit itself.** `nigig-uikit/src/shared/conversation/` is now consumed by both SMS and email, so a change there moves two features and an email-driven regression can surface in SMS. It currently has no tests of its own. At minimum: `SharedConversationPreviewProps` → emitted `Clicked` payload, and that an empty address suppresses the action (the row already guards this, but nothing pins it). |
+| ~~E1~~ | ~~**Unit-test the pure logic.**~~ **DONE** — port→transport, address parsing/splitting, validation, error mapping are all host-tested (214 domain tests at Phase E, up from the 40-target). |
+| ~~E2~~ | ~~**Property-test address parsing.**~~ **DONE** — `proptest` dev-dependency; `email_properties.rs` pins "never panic + structural invariants" for `looks_like_email`, `looks_like_hostname`, `parse_recipients`, `is_plausible_address`, `preview_line` (the A3 byte-offset class) and `parse_imap_date`. |
+| ~~E3~~ | ~~**Test-count floor gate.**~~ **DONE** — `email.yml` enforces `FLOOR=205`. |
+| ~~E4~~ | ~~**Clippy ratchet.**~~ **DONE** — ratchet at the measured baseline (0 nigig-email-owned diagnostics). |
+| ~~E5~~ | ~~**Integration test against a local SMTP sink.**~~ **DONE** — a hand-rolled SMTP sink on 127.0.0.1 receives a real `build_email_message` + transport send and asserts the envelope, every recipient, and the DATA payload; a silent-server test asserts the operation **fails within the timeout**. This surfaced a real defect: lettre's `.timeout()` only bounds the TCP connect, not the greeting/command reads, so the send path now wraps the whole operation in `platform::timeout` (see the A6/P4 correction below). |
+| ~~E6~~ | ~~**Test the shared conversation kit.**~~ **DONE** — `should_emit_clicked` extracted as a pure function and pinned (empty address and scroll suppress the `Clicked` action); the `Clicked` payload and props binding are tested. `nigig-uikit` now has its own tests, run in CI. |
 
 ---
 
@@ -827,10 +843,15 @@ convincingly than any client-side fix can.
 
 Stated plainly, because the point of this document is to be trusted:
 
-- **No SMTP path has been executed.** No live server, no MITM test. The S1/S3
-  analysis is a read of `lettre` 0.11.23's vendored source (`relay()`,
-  `TlsParameters::new`, `TlsVersion`), which is why I was able to catch my own
-  error — but it is still a read, not an observed handshake.
+- **The SMTP conversation has been executed, but not the TLS handshake.**
+  Phase E5 runs a real send against a local mock SMTP sink (envelope, every
+  recipient, and the DATA payload are asserted), and a silent-server test
+  pins the operation-timeout bound. What is STILL not executed: a TLS
+  handshake against a real provider, and a MITM test. The S1/S3 analysis of
+  `relay()`/`TlsParameters::new`/`TlsVersion` remains a read of lettre's
+  source, not an observed handshake — the sink is plaintext because the
+  production transport's TLS is pinned separately by `tls_mode_for_port`
+  and `build_transport_constructs_for_every_port`, not exercised here.
 - **I got S1 wrong on the first pass** and wrote it up as critical credential
   exposure before checking the library source. The corrected entry is in §1.
   Flagging it because a document like this is worth nothing if you cannot tell

crates/nigig-core/Cargo.toml

@@ -59,6 +59,11 @@ async-net = { version = "2", optional = true }
 # C1f: a real platform keystore (feature `keystore`), native only.
 keyring = { version = "4", optional = true }
 
+[dev-dependencies]
+# E2: property-test the address/date parsers — never panic on arbitrary
+# input. Same dev-dependency the SMS crate already uses.
+proptest = "1"
+
 [target.'cfg(target_arch = "wasm32")'.dependencies]
 wasm-bindgen = "0.2"
 wasm-bindgen-futures = "0.4"

crates/nigig-core/src/email_properties.rs

@@ -0,0 +1,95 @@
+//! Property tests for the email parsers (Phase E2).
+//!
+//! The address, hostname, recipient and date parsers all take *untrusted*
+//! input — a `To` field, a server field, an `INTERNALDATE` header — so they
+//! must never panic on arbitrary input, and their accept/reject decisions
+//! must satisfy structural invariants. A unit test can only cover the
+//! inputs its author happened to think of; these properties are the
+//! guarantee that *any* input behaves.
+//!
+//! `proptest` is a dev-dependency (the same one the SMS crate uses), so
+//! these run under `cargo test` and are picked up by the email domain
+//! filter in CI.
+
+use proptest::prelude::*;
+
+use crate::email_account::{looks_like_email, looks_like_hostname};
+use crate::email_send::{is_plausible_address, parse_recipients};
+use crate::email_store::preview_line;
+use crate::imap_client::parse_imap_date;
+
+proptest! {
+    /// `looks_like_email` never panics, and a `true` verdict implies the
+    /// structural properties the function exists to check: exactly one `@`,
+    /// non-empty local and domain, and a dot in the domain.
+    #[test]
+    fn looks_like_email_never_panics_and_is_structural(s in any::<String>()) {
+        let verdict = looks_like_email(&s);
+        if verdict {
+            let t = s.trim();
+            prop_assert_eq!(
+                t.matches('@').count(),
+                1,
+                "an accepted address must have exactly one '@'"
+            );
+            let (local, domain) = t.split_once('@').expect("one @ was asserted");
+            prop_assert!(!local.is_empty(), "empty local part");
+            prop_assert!(!domain.is_empty(), "empty domain");
+            prop_assert!(domain.contains('.'), "dotless domain");
+        }
+    }
+
+    /// `looks_like_hostname` never panics, and a `true` verdict rejects the
+    /// things a hostname must not contain (a scheme, a path, whitespace).
+    #[test]
+    fn looks_like_hostname_never_panics_and_rejects_schemes(s in any::<String>()) {
+        let verdict = looks_like_hostname(&s);
+        if verdict {
+            let t = s.trim();
+            prop_assert!(!t.contains("://"), "scheme in accepted hostname");
+            prop_assert!(!t.contains('/'), "path in accepted hostname");
+            prop_assert!(
+                !t.chars().any(char::is_whitespace),
+                "whitespace in accepted hostname"
+            );
+        }
+    }
+
+    /// `parse_recipients` never panics, and every address it accepts is one
+    /// `is_plausible_address` also accepts — the parser must not be more
+    /// lenient than the plausibility check it is built on.
+    #[test]
+    fn parse_recipients_never_panics_and_accepts_only_plausible(s in any::<String>()) {
+        let list = parse_recipients(&s);
+        for r in &list.accepted {
+            prop_assert!(
+                is_plausible_address(&r.address),
+                "accepted an implausible address"
+            );
+        }
+    }
+
+    /// `is_plausible_address` never panics on any input.
+    #[test]
+    fn is_plausible_address_never_panics(s in any::<String>()) {
+        let _ = is_plausible_address(&s);
+    }
+
+    /// `preview_line` never panics on any body and any limit. This is the
+    /// A3 class of bug — byte-offset slicing panics on multibyte text — and
+    /// the property is simply "no panic", on every body and every limit.
+    #[test]
+    fn preview_line_never_panics_on_any_body_and_limit(
+        body in any::<String>(),
+        max in 0usize..2000usize,
+    ) {
+        let _ = preview_line(&body, max);
+    }
+
+    /// `parse_imap_date` never panics on any input; it returns `Option`,
+    /// so garbage must simply be `None`, not a panic.
+    #[test]
+    fn parse_imap_date_never_panics_on_any_input(s in any::<String>()) {
+        let _ = parse_imap_date(&s);
+    }
+}

crates/nigig-core/src/email_worker.rs

@@ -474,21 +474,30 @@ use lettre::{
 #[cfg(not(target_arch = "wasm32"))]
 async fn smtp_test_impl(config: &SmtpConfig) -> Result<(), String> {
     let mailer = acquire_transport(config)?;
-    mailer
-        .transport()
-        .test_connection()
-        .await
-        .map(|_| ())
-        .map_err(|e| format!("SMTP test failed: {e}"))
+    // A6: bound the WHOLE operation, not just the TCP connect. lettre's
+    // `.timeout()` only bounds the connect; a server that accepts and then
+    // never greets (or never answers a command) would hang the read
+    // forever. `platform::timeout` closes that gap (E5 integration test
+    // `a_send_to_a_silent_server_errors_instead_of_hanging` pins it).
+    let outcome = crate::platform::timeout(
+        std::time::Duration::from_secs(SMTP_TIMEOUT_SECS),
+        mailer.transport().test_connection(),
+    )
+    .await;
+    match outcome {
+        Ok(Ok(_)) => Ok(()),
+        Ok(Err(e)) => Err(format!("SMTP test failed: {e}")),
+        Err(_) => Err(format!("SMTP test timed out after {SMTP_TIMEOUT_SECS}s")),
+    }
 }
 
 #[cfg(not(target_arch = "wasm32"))]
-async fn send_email_impl(
+fn build_email_message(
     config: &SmtpConfig,
     to: &str,
     subject: &str,
     body: &str,
-) -> Result<(), String> {
+) -> Result<Message, String> {
     let from_mbox: Mailbox = config
         .from
         .parse()
@@ -516,16 +525,48 @@ async fn send_email_impl(
             .map_err(|e| format!("Invalid recipient {}: {e}", r.address))?;
         builder = builder.to(mbox);
     }
-    let email = builder
+    builder
         .body(body.to_owned())
-        .map_err(|e| format!("Build error: {e}"))?;
+        .map_err(|e| format!("Build error: {e}"))
+}
+
+/// Send one message, bounded by an explicit timeout.
+///
+/// A6: the whole send — connect AND the SMTP command reads — is bounded.
+/// lettre's own `.timeout()` only covers the TCP connect, so a server that
+/// accepts and then never greets (or never answers a command) would hang
+/// the read forever. This is the one place that guarantee is made, so both
+/// the production path and the integration test (`a_send_to_a_silent_…`)
+/// go through it.
+#[cfg(not(target_arch = "wasm32"))]
+async fn send_bounded(
+    transport: &AsyncSmtpTransport<Tokio1Executor>,
+    email: Message,
+    timeout: std::time::Duration,
+) -> Result<(), String> {
+    let outcome = crate::platform::timeout(timeout, transport.send(email)).await;
+    match outcome {
+        Ok(Ok(_)) => Ok(()),
+        Ok(Err(e)) => Err(format!("Send failed: {e}")),
+        Err(_) => Err(format!("Send timed out after {}s", timeout.as_secs())),
+    }
+}
+
+#[cfg(not(target_arch = "wasm32"))]
+async fn send_email_impl(
+    config: &SmtpConfig,
+    to: &str,
+    subject: &str,
+    body: &str,
+) -> Result<(), String> {
+    let email = build_email_message(config, to, subject, body)?;
     let mailer = acquire_transport(config)?;
-    mailer
-        .transport()
-        .send(email)
-        .await
-        .map_err(|e| format!("Send failed: {e}"))?;
-    Ok(())
+    send_bounded(
+        mailer.transport(),
+        email,
+        std::time::Duration::from_secs(SMTP_TIMEOUT_SECS),
+    )
+    .await
 }
 
 /// Which TLS mode a port implies.
@@ -1239,4 +1280,213 @@ mod tests {
     fn protocol_relative_urls_are_rejected() {
         assert!(!email_api_url_is_safe("//mail.example.com/api"));
     }
+
+    // ---- E5: a real SMTP conversation against a local sink -------------
+    //
+    // This is the first time the SMTP path is *executed* in this repo
+    // (the assessment's "What I have not verified" listed it). A minimal
+    // SMTP server runs on 127.0.0.1 in a thread; `build_email_message`
+    // constructs the message and a PLAINTEXT transport delivers it, and
+    // the sink records exactly what it received. The production transport
+    // always uses TLS (A2/A3, gated in CI); that layer is pinned by
+    // `tls_mode_for_port` and `build_transport_constructs_for_every_port`
+    // above. This test pins the conversation beneath it: envelope, every
+    // recipient, and the DATA payload.
+
+    /// A tiny SMTP sink that speaks just enough of the protocol to receive
+    /// one message and record the envelope + data. Plaintext on purpose
+    /// (the production transport's TLS is pinned elsewhere).
+    #[cfg(not(target_arch = "wasm32"))]
+    mod smtp_sink {
+        use std::io::{BufRead, BufReader, Write};
+        use std::net::TcpListener;
+        use std::sync::mpsc;
+
+        #[derive(Clone, Debug, Default)]
+        pub struct Captured {
+            pub mail_from: Vec<String>,
+            pub rcpt_to: Vec<String>,
+            pub data: String,
+        }
+
+        /// Bind an ephemeral port and serve one SMTP transaction on a
+        /// background thread. The captured conversation is reported as soon
+        /// as the DATA payload is complete — NOT on QUIT, because a pooled
+        /// client may hold the connection open after the send.
+        pub fn start() -> (u16, mpsc::Receiver<Captured>) {
+            let listener = TcpListener::bind("127.0.0.1:0").expect("bind");
+            let port = listener.local_addr().expect("addr").port();
+            let (tx, rx) = mpsc::channel();
+
+            std::thread::spawn(move || {
+                let (stream, _) = listener.accept().expect("accept");
+                let mut reader = BufReader::new(stream.try_clone().expect("clone"));
+                let mut captured = Captured::default();
+
+                let respond = |code: &str| -> std::io::Result<()> {
+                    let mut w = stream.try_clone().expect("clone");
+                    w.write_all(code.as_bytes())?;
+                    w.flush()
+                };
+
+                respond("220 mock.example ESMTP\r\n").unwrap();
+                let mut line = String::new();
+                loop {
+                    line.clear();
+                    if reader.read_line(&mut line).unwrap() == 0 {
+                        break;
+                    }
+                    let cmd = line.trim_end().to_string();
+                    if cmd.starts_with("EHLO") || cmd.starts_with("HELO") {
+                        respond("250-mock.example\r\n250 8BITMIME\r\n").unwrap();
+                    } else if cmd.starts_with("MAIL FROM:") {
+                        captured.mail_from.push(cmd);
+                        respond("250 Ok\r\n").unwrap();
+                    } else if cmd.starts_with("RCPT TO:") {
+                        captured.rcpt_to.push(cmd);
+                        respond("250 Ok\r\n").unwrap();
+                    } else if cmd == "DATA" {
+                        respond("354 End data with <CR><LF>.<CR><LF>\r\n").unwrap();
+                        loop {
+                            line.clear();
+                            reader.read_line(&mut line).unwrap();
+                            if line == ".\r\n" || line == ".\n" || line == "." {
+                                break;
+                            }
+                            captured.data.push_str(&line);
+                        }
+                        respond("250 Ok: queued\r\n").unwrap();
+                        // Report now; do not wait for QUIT/EOF.
+                        let _ = tx.send(captured.clone());
+                    } else if cmd == "QUIT" {
+                        respond("221 Bye\r\n").unwrap();
+                        break;
+                    } else {
+                        // RSET, NOOP, or anything unexpected: accept it so
+                        // the conversation can continue.
+                        respond("250 Ok\r\n").unwrap();
+                    }
+                }
+                // If the client never sent DATA (e.g. it aborted), still
+                // report what little we saw so the test does not hang.
+                let _ = tx.send(captured);
+            });
+
+            (port, rx)
+        }
+    }
+
+    /// A message sent through `build_email_message` + a transport reaches
+    /// the sink with the right envelope, every recipient, and the body.
+    #[cfg(not(target_arch = "wasm32"))]
+    #[test]
+    fn a_real_send_reaches_a_mock_smtp_server() {
+        use lettre::AsyncSmtpTransport;
+
+        let (port, rx) = smtp_sink::start();
+
+        let rt = tokio::runtime::Builder::new_current_thread()
+            .enable_all()
+            .build()
+            .expect("runtime");
+
+        rt.block_on(async {
+            let config = SmtpConfig {
+                server: "127.0.0.1".into(),
+                port,
+                username: "jane@example.com".into(),
+                password: Secret::new("hunter2"),
+                from: "jane@example.com".into(),
+            };
+            let email = build_email_message(
+                &config,
+                "a@x.com, b@y.com",
+                "Hello from the test",
+                "Body text line.",
+            )
+            .expect("message should build");
+
+            // Plaintext, test-only. The production transport always uses
+            // TLS (see build_transport); this proves the SMTP conversation
+            // itself over a real TCP socket.
+            let mailer = AsyncSmtpTransport::<Tokio1Executor>::builder_dangerous("127.0.0.1")
+                .port(port)
+                .build();
+            mailer.send(email).await.expect("send should succeed");
+        });
+
+        let captured = rx
+            .recv_timeout(std::time::Duration::from_secs(10))
+            .expect("the sink should report the conversation");
+
+        assert_eq!(captured.mail_from, vec!["MAIL FROM:<jane@example.com>"]);
+        assert_eq!(
+            captured.rcpt_to,
+            vec!["RCPT TO:<a@x.com>", "RCPT TO:<b@y.com>"]
+        );
+        // The DATA payload carries the subject header and the body.
+        assert!(
+            captured.data.contains("Hello from the test"),
+            "subject missing from DATA: {:?}",
+            captured.data
+        );
+        assert!(
+            captured.data.contains("Body text line."),
+            "body missing from DATA: {:?}",
+            captured.data
+        );
+    }
+
+    /// A send to a server that accepts but never speaks (a black hole)
+    /// fails within the timeout instead of hanging forever — the A6
+    /// timeout is wired into the real transport path, not just declared.
+    #[cfg(not(target_arch = "wasm32"))]
+    #[test]
+    fn a_send_to_a_silent_server_errors_instead_of_hanging() {
+        use lettre::AsyncSmtpTransport;
+
+        // A server that accepts the connection and then says nothing. It
+        // holds the socket open (blocked on a channel) so the client hangs
+        // waiting for the greeting unless it times out.
+        let listener = std::net::TcpListener::bind("127.0.0.1:0").expect("bind");
+        let port = listener.local_addr().expect("addr").port();
+        let (release_tx, release_rx) = std::sync::mpsc::channel::<()>();
+        let server = std::thread::spawn(move || {
+            let _stream = listener.accept().expect("accept");
+            // Hold the connection open until the test releases us.
+            let _ = release_rx.recv();
+        });
+
+        let rt = tokio::runtime::Builder::new_current_thread()
+            .enable_all()
+            .build()
+            .expect("runtime");
+
+        let result = rt.block_on(async {
+            let config = SmtpConfig {
+                server: "127.0.0.1".into(),
+                port,
+                username: "jane@example.com".into(),
+                password: Secret::new("hunter2"),
+                from: "jane@example.com".into(),
+            };
+            let email = build_email_message(&config, "a@x.com", "Hi", "Body")
+                .expect("message should build");
+            let mailer = AsyncSmtpTransport::<Tokio1Executor>::builder_dangerous("127.0.0.1")
+                .port(port)
+                .build();
+            // The same bounded path production uses, with a short timeout.
+            send_bounded(&mailer, email, std::time::Duration::from_secs(1)).await
+        });
+
+        // Release the server thread so the process can exit.
+        let _ = release_tx.send(());
+        let _ = server.join();
+
+        let err = result.expect_err("a silent server must produce an error");
+        assert!(
+            err.contains("timed out"),
+            "the error should name the timeout, got {err:?}"
+        );
+    }
 }

crates/nigig-core/src/lib.rs

@@ -32,6 +32,9 @@ pub mod email_cache;
 pub mod imap_client;
 pub mod email_session;
 
+#[cfg(test)]
+mod email_properties;
+
 pub use dir::app_data_dir;
 pub use persistence::*;
 

crates/nigig-uikit/src/shared/conversation/conversation_preview.rs

@@ -1,6 +1,5 @@
-
-use makepad_widgets::*;
 use makepad_widgets::ActionDefaultRef;
+use makepad_widgets::*;
 
 script_mod! {
     use mod.prelude.widgets.*
@@ -94,6 +93,18 @@ pub struct SharedConversationPreviewProps {
     pub was_scrolling: bool,
 }
 
+/// Should a tap on this row emit a `Clicked` action?
+///
+/// Pure so the guard is testable without a `Cx` (E6). A row with an empty
+/// address is a placeholder and must never open a thread, and a scroll in
+/// progress (a fling) must never be read as a tap. Both SMS and email feed
+/// this widget; this function is the single source of truth for the guard,
+/// so a regression here surfaces in both features rather than silently in
+/// one.
+pub fn should_emit_clicked(address: &str, was_scrolling: bool) -> bool {
+    !address.is_empty() && !was_scrolling
+}
+
 #[derive(Clone, Debug, Default)]
 pub enum SharedConversationPreviewAction {
     #[default]
@@ -134,7 +145,10 @@ impl Widget for SharedConversationPreview {
 
         self.view.handle_event(cx, event, scope);
 
-        if self.address.is_empty() {
+        // E6: an empty address (a placeholder row) must never open a
+        // thread, and a scroll in progress must not be read as a tap. The
+        // decision is pinned by a unit test below.
+        if !should_emit_clicked(&self.address, self.was_scrolling) {
             return;
         }
 
@@ -142,7 +156,7 @@ impl Widget for SharedConversationPreview {
         match event.hits(cx, area) {
             Hit::FingerDown(_fe) => cx.set_key_focus(area),
             Hit::FingerUp(fe) => {
-                if fe.was_tap() && fe.is_over && !self.was_scrolling {
+                if fe.was_tap() && fe.is_over {
                     cx.widget_action(
                         self.widget_uid(),
                         SharedConversationPreviewAction::Clicked {
@@ -166,4 +180,66 @@ impl Widget for SharedConversationPreview {
     }
 }
 
+#[cfg(test)]
+mod tests {
+    use super::*;
 
+    // E6: the conversation kit is consumed by BOTH SMS and email, so a
+    // regression here moves two features at once. These tests pin the
+    // click guard and the payload shape, which previously had no coverage.
+
+    /// A row with an address, not mid-scroll, may emit Clicked.
+    #[test]
+    fn a_row_with_an_address_may_emit_clicked() {
+        assert!(should_emit_clicked("alerts@bank.co.ke", false));
+    }
+
+    /// The empty-address guard: a placeholder row must never open a thread.
+    #[test]
+    fn an_empty_address_suppresses_the_click() {
+        assert!(!should_emit_clicked("", false));
+    }
+
+    /// A fling must not be read as a tap.
+    #[test]
+    fn a_scroll_suppresses_the_click() {
+        assert!(!should_emit_clicked("alerts@bank.co.ke", true));
+        assert!(!should_emit_clicked("", true));
+    }
+
+    /// The Clicked payload carries exactly the address and display name the
+    /// row was bound with, so a feature can open the right thread.
+    #[test]
+    fn the_clicked_payload_carries_address_and_display_name() {
+        let payload = SharedConversationPreviewAction::Clicked {
+            address: "alerts@bank.co.ke".into(),
+            display_name: "Equity Alerts".into(),
+        };
+        match payload {
+            SharedConversationPreviewAction::Clicked {
+                address,
+                display_name,
+            } => {
+                assert_eq!(address, "alerts@bank.co.ke");
+                assert_eq!(display_name, "Equity Alerts");
+            }
+            other => panic!("expected Clicked, got {other:?}"),
+        }
+    }
+
+    /// The props are the binding between a list row and the emitted action:
+    /// what a feature puts in (address, display_name) is what a click
+    /// carries back. Pinned so the two cannot drift apart.
+    #[test]
+    fn props_carry_the_address_and_display_name_a_click_will_emit() {
+        let props = SharedConversationPreviewProps {
+            address: "jane@example.com".into(),
+            display_name: "Jane".into(),
+            was_scrolling: false,
+        };
+        assert_eq!(props.address, "jane@example.com");
+        assert_eq!(props.display_name, "Jane");
+        assert!(!props.was_scrolling);
+        assert!(should_emit_clicked(&props.address, props.was_scrolling));
+    }
+}