.forgejo/workflows/email.yml

@@ -409,12 +409,11 @@ 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; 214 after Phase E (proptest +
+      # 195 after the Phase C/D completion.
-      # the SMTP sink integration tests).
       - name: The email domain test suite must not shrink
         run: |
           set -euo pipefail
-          FLOOR=205
+          FLOOR=190
           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
@@ -483,14 +482,6 @@ 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,7 +2984,6 @@ dependencies = [
  "matrix_client",
  "nigig-system-prefs",
  "postcard",
- "proptest",
  "rand 0.8.7",
  "reqwest",
  "robius-directories",

REVIEWS/NIGIG_EMAIL_ASSESSMENT_AND_PLAN.md

@@ -422,21 +422,6 @@ 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.
 
@@ -547,7 +532,6 @@ 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.
 
@@ -816,12 +800,12 @@ presenting them as equivalent choices.
 
 | ID | Task |
 |---|---|
-| ~~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). |
+| 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.**~~ **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`. |
+| E2 | **Property-test address parsing** (proptest is already a dev-dep in SMS) — never panic on arbitrary input. |
-| ~~E3~~ | ~~**Test-count floor gate.**~~ **DONE** — `email.yml` enforces `FLOOR=205`. |
+| E3 | **Test-count floor gate**, as SMS has (`FLOOR=100`). |
-| ~~E4~~ | ~~**Clippy ratchet.**~~ **DONE** — ratchet at the measured baseline (0 nigig-email-owned diagnostics). |
+| E4 | **Clippy ratchet** at the measured baseline. |
-| ~~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). |
+| 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~~ | ~~**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. |
+| **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). |
 
 ---
 
@@ -843,15 +827,10 @@ convincingly than any client-side fix can.
 
 Stated plainly, because the point of this document is to be trusted:
 
-- **The SMTP conversation has been executed, but not the TLS handshake.**
+- **No SMTP path has been executed.** No live server, no MITM test. The S1/S3
-  Phase E5 runs a real send against a local mock SMTP sink (envelope, every
+  analysis is a read of `lettre` 0.11.23's vendored source (`relay()`,
-  recipient, and the DATA payload are asserted), and a silent-server test
+  `TlsParameters::new`, `TlsVersion`), which is why I was able to catch my own
-  pins the operation-timeout bound. What is STILL not executed: a TLS
+  error — but it is still a read, not an observed handshake.
-  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,11 +59,6 @@ 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

@@ -1,95 +0,0 @@
-//! 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,30 +474,21 @@ use lettre::{
 #[cfg(not(target_arch = "wasm32"))]
 async fn smtp_test_impl(config: &SmtpConfig) -> Result<(), String> {
     let mailer = acquire_transport(config)?;
-    // A6: bound the WHOLE operation, not just the TCP connect. lettre's
+    mailer
-    // `.timeout()` only bounds the connect; a server that accepts and then
+        .transport()
-    // never greets (or never answers a command) would hang the read
+        .test_connection()
-    // forever. `platform::timeout` closes that gap (E5 integration test
+        .await
-    // `a_send_to_a_silent_server_errors_instead_of_hanging` pins it).
+        .map(|_| ())
-    let outcome = crate::platform::timeout(
+        .map_err(|e| format!("SMTP test failed: {e}"))
-        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"))]
-fn build_email_message(
+async fn send_email_impl(
     config: &SmtpConfig,
     to: &str,
     subject: &str,
     body: &str,
-) -> Result<Message, String> {
+) -> Result<(), String> {
     let from_mbox: Mailbox = config
         .from
         .parse()
@@ -525,48 +516,16 @@ fn build_email_message(
             .map_err(|e| format!("Invalid recipient {}: {e}", r.address))?;
         builder = builder.to(mbox);
     }
-    builder
+    let email = 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)?;
-    send_bounded(
+    mailer
-        mailer.transport(),
+        .transport()
-        email,
+        .send(email)
-        std::time::Duration::from_secs(SMTP_TIMEOUT_SECS),
+        .await
-    )
+        .map_err(|e| format!("Send failed: {e}"))?;
-    .await
+    Ok(())
 }
 
 /// Which TLS mode a port implies.
@@ -1280,213 +1239,4 @@ 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,9 +32,6 @@ 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,5 +1,6 @@
-use makepad_widgets::ActionDefaultRef;
+
 use makepad_widgets::*;
+use makepad_widgets::ActionDefaultRef;
 
 script_mod! {
     use mod.prelude.widgets.*
@@ -93,18 +94,6 @@ 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]
@@ -145,10 +134,7 @@ impl Widget for SharedConversationPreview {
 
         self.view.handle_event(cx, event, scope);
 
-        // E6: an empty address (a placeholder row) must never open a
+        if self.address.is_empty() {
-        // 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;
         }
 
@@ -156,7 +142,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 {
+                if fe.was_tap() && fe.is_over && !self.was_scrolling {
                     cx.widget_action(
                         self.widget_uid(),
                         SharedConversationPreviewAction::Clicked {
@@ -180,66 +166,4 @@ 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));
-    }
-}