refactor(mail): remove fake/dead protocol code #262

2026-07-31T15:04:02Z

KofTwentyTwo commented

2026-07-31 15:04:02 +00:00

(Migrated from github.com)

Closes #253.

Summary

De-fabrication pass over nuncio-mail and nuncio-cal: no pub production code path in these crates now returns canned data or fabricates success.

Part 1 — orphaned duplicate CardDAV client (deleted)

Deleted crates/nuncio-cal/src/carddav.rs in full (CardDavClient/Contact, ~148 lines). It made no network call, implemented no backend trait, and was a non-functional duplicate of the real, working nuncio_contacts::carddav::CardDavClient.
Removed its mod carddav; / pub use carddav::{CardDavClient, Contact}; lines from crates/nuncio-cal/src/lib.rs.
Verified no dependents: git grep -n "nuncio_cal::CardDavClient\|nuncio_cal::Contact\|nuncio_cal::carddav\|use nuncio_cal" shows only CalDavAccountConfig/CalDavClient/CalendarBackend/MockCalendarBackend usages outside the crate — nothing referenced the deleted module.

Part 2 — credential-less fake-data mail paths (honest error)

ImapEngine::sync_folders / ImapEngine::sync_folder_messages (called from MailBackend::sync_messages): the credential-less branches returned hardcoded Folder/Email fixtures. Changed to return MailError::AuthError ("... requires credentials") instead.
- Bonus fix while touching this: even with credentials, sync_folders previously fell back to the same canned folders whenever the real mailbox list came back empty — a genuinely empty mailbox is a valid result, not a signal to fabricate data, so that fallback is removed too.
JmapEngine::sync_folders / JmapEngine::sync_messages: same treatment — the !has_credentials() branches now return MailError::AuthError instead of a hardcoded folder list / a synthetic sample_jmap payload run through the real parser.
MailBackend::send_email: removed from the trait entirely. ImapEngine::send_email discarded the message and returned Ok(()) unconditionally (pure fabricated success — no transport). JmapEngine::send_email checked recipient.is_empty() but otherwise also just returned Ok(()) with no transport. Verified nothing in production calls MailBackend::send_email: git grep -n "\.send_email(" across crates/* shows only test call sites (removed here) and SmtpTransportEngine::send_email (an unrelated inherent method — the real SMTP path used by nunciod::send, driven through the separate MessageSender trait, not MailBackend). mail_backend (Arc<dyn MailBackend>) is only ever probed with .is_some() in grpc.rs, never called for sending.
MockMailBackend::send_email (its MailBackend impl) removed along with the trait method; its unused sent_messages tracking field/getter removed too (dead once the mock stopped implementing send_email). MockMessageSender (used by the real send-path tests) is untouched.

Mechanism chosen and why

CardDAV dup: delete, since it did nothing and had no dependents.
Credential-less sync_folders/sync_messages: honest typed error (MailError::AuthError), not #[cfg(test)]-gating, because ImapEngine::new/JmapEngine::new are legitimate two-step-construction public constructors (paired with set_credentials), not test-only scaffolding — gating them behind #[cfg(test)] would remove a valid construction pattern for external callers of the crate. An honest error is the correct behavior for "you asked me to sync without ever giving me credentials."
send_email: removed from the trait rather than "error honestly," since grep confirmed zero production callers of MailBackend::send_email — the real send path (MessageSender/SmtpTransportEngine) already exists and is exercised end-to-end in nunciod::send. Keeping a dead trait method around only to have it return Unsupported would be a code smell with no behavioral value; removing it is the more honest fix.

Tests migrated (real fixtures, still offline)

nuncio-mail/src/imap.rs: replaced imap_engine_sync_folders_and_messages (asserted on the fabricated fixture + called the now-removed send_email) with:
- sync_folders_and_sync_messages_require_credentials — asserts the honest AuthError.
- sync_folders_lists_real_mailboxes_over_a_scripted_server — a new test driving MailBackend::sync_folders against a real plain-TCP loopback IMAP server scripted to answer LOGIN/LIST/LOGOUT, proving the folder list is genuinely read off the wire (mirrors the existing tokio::io::duplex scripted-server pattern already used elsewhere in the file for sync_folder_messages_with_session).
nuncio-mail/src/jmap.rs: replaced uncredentialed_engine_keeps_static_fallback_behaviour with uncredentialed_engine_requires_credentials_to_sync, asserting the honest AuthError for both sync_folders and sync_messages. The existing wiremock suite (tests/wiremock_jmap_test.rs) already fully covers the credentialed real-HTTP path through the MailBackend trait (jmap_engine_sync_folders_and_messages_through_mail_backend_trait) and was untouched.
nuncio-mail/src/mock.rs: dropped the send_email/sent_messages assertions from mock_backend_folder_and_message_operations (method no longer exists); MockMessageSender's own send test is untouched.
No live network in any test — the IMAP test uses a real loopback TCP socket with a scripted responder (no external I/O); JMAP credentialed behavior is wiremock-backed (already existed, untouched).

Gate output

$ cargo fmt --all -- --check
(clean, no output)

$ cargo check-all
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 4.62s
(zero warnings)

$ cargo test-all
nuncio-cal:  25 passed (23 unit + 2 wiremock), 0 failed, 0 ignored
nuncio-mail: 56 passed (53 unit + 3 wiremock), 0 failed, 0 ignored
nunciod:     120 unit + 13 integration (e2e) passed, 0 failed, 0 ignored
(full workspace: 0 failed, 0 ignored)

Local pre-commit hook (fmt + clippy --all-targets -D warnings + full test suite) ran and passed on commit c990348.

Test plan

cargo fmt --all -- --check
cargo check-all (clippy --all-targets --workspace -D warnings, 0 warnings)
cargo test-all (0 failed, 0 ignored)
Grep-verified no production dependents before deleting nuncio-cal/src/carddav.rs
Grep-verified no production callers of MailBackend::send_email before removing it from the trait

Closes #253. ## Summary De-fabrication pass over `nuncio-mail` and `nuncio-cal`: no `pub` production code path in these crates now returns canned data or fabricates success. ### Part 1 — orphaned duplicate CardDAV client (deleted) - Deleted `crates/nuncio-cal/src/carddav.rs` in full (`CardDavClient`/`Contact`, ~148 lines). It made no network call, implemented no backend trait, and was a non-functional duplicate of the real, working `nuncio_contacts::carddav::CardDavClient`. - Removed its `mod carddav;` / `pub use carddav::{CardDavClient, Contact};` lines from `crates/nuncio-cal/src/lib.rs`. - **Verified no dependents**: `git grep -n "nuncio_cal::CardDavClient\|nuncio_cal::Contact\|nuncio_cal::carddav\|use nuncio_cal"` shows only `CalDavAccountConfig`/`CalDavClient`/`CalendarBackend`/`MockCalendarBackend` usages outside the crate — nothing referenced the deleted module. ### Part 2 — credential-less fake-data mail paths (honest error) - `ImapEngine::sync_folders` / `ImapEngine::sync_folder_messages` (called from `MailBackend::sync_messages`): the credential-less branches returned hardcoded `Folder`/`Email` fixtures. Changed to return `MailError::AuthError` ("... requires credentials") instead. - Bonus fix while touching this: even *with* credentials, `sync_folders` previously fell back to the same canned folders whenever the real mailbox list came back empty — a genuinely empty mailbox is a valid result, not a signal to fabricate data, so that fallback is removed too. - `JmapEngine::sync_folders` / `JmapEngine::sync_messages`: same treatment — the `!has_credentials()` branches now return `MailError::AuthError` instead of a hardcoded folder list / a synthetic `sample_jmap` payload run through the real parser. - `MailBackend::send_email`: removed from the trait entirely. `ImapEngine::send_email` discarded the message and returned `Ok(())` unconditionally (pure fabricated success — no transport). `JmapEngine::send_email` checked `recipient.is_empty()` but otherwise also just returned `Ok(())` with no transport. **Verified nothing in production calls `MailBackend::send_email`**: `git grep -n "\.send_email("` across `crates/*` shows only test call sites (removed here) and `SmtpTransportEngine::send_email` (an unrelated *inherent* method — the real SMTP path used by `nunciod::send`, driven through the separate `MessageSender` trait, not `MailBackend`). `mail_backend` (`Arc<dyn MailBackend>`) is only ever probed with `.is_some()` in `grpc.rs`, never called for sending. - `MockMailBackend::send_email` (its `MailBackend` impl) removed along with the trait method; its unused `sent_messages` tracking field/getter removed too (dead once the mock stopped implementing `send_email`). `MockMessageSender` (used by the real send-path tests) is untouched. ### Mechanism chosen and why - CardDAV dup: **delete**, since it did nothing and had no dependents. - Credential-less `sync_folders`/`sync_messages`: **honest typed error** (`MailError::AuthError`), not `#[cfg(test)]`-gating, because `ImapEngine::new`/`JmapEngine::new` are legitimate two-step-construction public constructors (paired with `set_credentials`), not test-only scaffolding — gating them behind `#[cfg(test)]` would remove a valid construction pattern for external callers of the crate. An honest error is the correct behavior for "you asked me to sync without ever giving me credentials." - `send_email`: **removed from the trait** rather than "error honestly," since grep confirmed zero production callers of `MailBackend::send_email` — the real send path (`MessageSender`/`SmtpTransportEngine`) already exists and is exercised end-to-end in `nunciod::send`. Keeping a dead trait method around only to have it return `Unsupported` would be a code smell with no behavioral value; removing it is the more honest fix. ### Tests migrated (real fixtures, still offline) - `nuncio-mail/src/imap.rs`: replaced `imap_engine_sync_folders_and_messages` (asserted on the fabricated fixture + called the now-removed `send_email`) with: - `sync_folders_and_sync_messages_require_credentials` — asserts the honest `AuthError`. - `sync_folders_lists_real_mailboxes_over_a_scripted_server` — a new test driving `MailBackend::sync_folders` against a real plain-TCP loopback IMAP server scripted to answer `LOGIN`/`LIST`/`LOGOUT`, proving the folder list is genuinely read off the wire (mirrors the existing `tokio::io::duplex` scripted-server pattern already used elsewhere in the file for `sync_folder_messages_with_session`). - `nuncio-mail/src/jmap.rs`: replaced `uncredentialed_engine_keeps_static_fallback_behaviour` with `uncredentialed_engine_requires_credentials_to_sync`, asserting the honest `AuthError` for both `sync_folders` and `sync_messages`. The existing wiremock suite (`tests/wiremock_jmap_test.rs`) already fully covers the credentialed real-HTTP path through the `MailBackend` trait (`jmap_engine_sync_folders_and_messages_through_mail_backend_trait`) and was untouched. - `nuncio-mail/src/mock.rs`: dropped the `send_email`/`sent_messages` assertions from `mock_backend_folder_and_message_operations` (method no longer exists); `MockMessageSender`'s own send test is untouched. - No live network in any test — the IMAP test uses a real loopback TCP socket with a scripted responder (no external I/O); JMAP credentialed behavior is wiremock-backed (already existed, untouched). ## Gate output ``` $ cargo fmt --all -- --check (clean, no output) $ cargo check-all Finished `dev` profile [unoptimized + debuginfo] target(s) in 4.62s (zero warnings) $ cargo test-all nuncio-cal: 25 passed (23 unit + 2 wiremock), 0 failed, 0 ignored nuncio-mail: 56 passed (53 unit + 3 wiremock), 0 failed, 0 ignored nunciod: 120 unit + 13 integration (e2e) passed, 0 failed, 0 ignored (full workspace: 0 failed, 0 ignored) ``` Local pre-commit hook (fmt + clippy `--all-targets -D warnings` + full test suite) ran and passed on commit `c990348`. ## Test plan - [x] `cargo fmt --all -- --check` - [x] `cargo check-all` (clippy `--all-targets --workspace -D warnings`, 0 warnings) - [x] `cargo test-all` (0 failed, 0 ignored) - [x] Grep-verified no production dependents before deleting `nuncio-cal/src/carddav.rs` - [x] Grep-verified no production callers of `MailBackend::send_email` before removing it from the trait

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#262