M3: Real JMAP client wired into the existing Mail sync path #189

New Issue

2026-07-28T14:04:27Z

KofTwentyTwo commented

2026-07-28 14:04:27 +00:00

(Migrated from github.com)

Context

Story for milestone M3. Read docs/STORY-WORKFLOW.md first — it defines the process, gates, and patterns; this issue defines only the specific work. nuncio-mail's JmapEngine (crates/nuncio-mail/src/jmap.rs) implements MailBackend but is entirely fake in production: sync_folders returns 3 hardcoded Folders and sync_messages always re-parses one canned static JSON blob regardless of folder_id/since_state, and it never performs any HTTP I/O. crates/nunciod/src/sync.rs's build_mail_backend (the ONLY place a concrete protocol engine is constructed) calls JmapEngine::new(&config.id) — it passes neither the account's server_host/server_port nor its keyring password, so a JMAP account today can never sync anything real, even though nunciod::sync's generic fetch→persist plumbing (fetch_and_persist) already works for any MailBackend. There is also a live wire-format bug: JmapEmail (the raw JMAP Email object struct) has no #[serde(rename_all = "camelCase")], so real JMAP servers' receivedAt/isUnread/bodySnippet keys never match the struct's snake_case field names and silently deserialize to None/defaults (the existing wiremock test never asserts on those fields, so this passes unnoticed today). The goal: make JmapEngine a real RFC 8620/8621 client — session discovery, Email/query+Email/get, and a real Mailbox/get folder listing — that flows through the exact same MailBackend → fetch_and_persist → DatabaseEngine::save_email path IMAP already uses, with NO new proto surface (the existing nuncio.v1.Mail service already covers both protocols).

Goal

JmapEngine, when constructed with real credentials, performs genuine HTTP session discovery against /.well-known/jmap, then real Mailbox/get and Email/query/Email/get JSON-RPC calls against the discovered apiUrl, and returns folders/messages actually parsed from the server's response — never canned data. It must NOT fake success: a session-discovery or JMAP-call HTTP/parse failure must surface as a genuine MailError, never silently fall back to the old static sample. A JmapEngine constructed with NO credentials (mirroring ImapEngine::new's uncredentialed path) may keep today's deterministic no-network fallback behaviour so existing credential-less unit tests keep working — but the production path (nunciod::sync::build_mail_backend) MUST always construct it with the account's real host and keyring password, exactly like it already does for ImapEngine::with_credentials.

Files to touch / patterns to mirror

crates/nuncio-mail/Cargo.toml — reqwest (line 33) is currently a [dev-dependencies]-only crate; JMAP is going to make real HTTP calls from production code, so move reqwest.workspace = true into [dependencies] (it can be removed from [dev-dependencies] once it's a regular dependency).
crates/nuncio-mail/src/jmap.rs — the crate to rewrite:
- JmapEmail (line 62): add #[serde(rename_all = "camelCase")] on the struct. This is a real, currently-live bug: the struct's received_at/is_unread/body_snippet fields never match the wire's receivedAt/isUnread/bodySnippet keys, so every parsed message silently gets a zero timestamp, read: true (!is_unread.unwrap_or(false)), and no snippet. Add a regression test asserting these three fields round-trip correctly from camelCase JSON (the existing wiremock_jmap_session_discovery_and_email_get_sync test conspicuously never asserts on them).
- JmapEngine (line 79): extend it to carry an HTTP client, the account's host, and optional username/password, mirroring ImapEngine's shape in crates/nuncio-mail/src/imap.rs (ImapEngine::new vs ImapEngine::with_credentials at lines 173/183, and the uncredentialed static-fallback branch in sync_folder_messages at lines 324-342). Add JmapEngine::with_credentials(account_id: &str, host: &str, username: &str, password: &str) -> Self; keep JmapEngine::new(account_id: &str) for the uncredentialed/preview case.
- Add a Mailbox/get request builder + response parser, mirroring the existing build_email_get_request/parse_email_get_response shape (lines 101-117, 143-202): a JmapMailbox { id: String, name: String, #[serde(default)] total_emails: u32, #[serde(default)] unread_emails: u32 } with #[serde(rename_all = "camelCase")] (RFC 8621 §2 Mailbox object uses totalEmails/unreadEmails), and a JmapMailboxGetResponse wrapper like JmapEmailGetResponse.
- Add an Email/query request builder ({"filter": {"inMailboxes": [mailbox_id]}}) + a minimal response parser extracting the ids array, since Email/get needs explicit ids (or None for "all" — real servers require a bounded query first).
- Rewrite the MailBackend impl (lines 233-291):
  - sync_folders: with credentials, GET the session URL (Self::build_session_url(&self.host)) with HTTP Basic auth (username/password), parse it, POST Mailbox/get to session.api_url, and map each JmapMailbox to a domain Folder. Without credentials, keep returning the existing static 3-folder list (do not remove the no-credential fallback; downstream tests and any preview/dry-run caller rely on JmapEngine::new staying deterministic and network-free).
  - sync_messages(folder_id, since_state): with credentials, discover the session (or reuse a cached one), Email/query for folder_id's mailbox id, then Email/get those ids, and return the parsed Vec<Email> + new state via parse_email_get_response. Mirror ImapEngine::sync_messages's current scope exactly: it is honest to ignore since_state for now (a full re-fetch) — ImapEngine does the same today; incremental sync for either protocol is out of scope here (see the M5-S2 roadmap story). Without credentials, keep today's static sample fallback.
crates/nunciod/src/sync.rs — build_mail_backend (line 126): change the AccountProtocol::Jmap arm from Box::new(JmapEngine::new(&config.id)) to Box::new(JmapEngine::with_credentials(&config.id, &config.server_host, &config.email_address, password)), mirroring the ImapSmtp arm immediately above it.
- This makes the existing run_account_sync_persists_real_backend_messages_and_reports_success test (in this file's #[cfg(test)] mod tests, using sample_jmap_account) attempt REAL network I/O against "jmap.nuncio.mx" once credentials flow through — which is forbidden (STORY-WORKFLOW.md §2.4: no live network calls in tests). You must fix this test as part of this story: point sample_jmap_account's server_host at a wiremock::MockServer's own .uri() (which already includes an http:// scheme, so JmapEngine::build_session_url will use it as-is rather than prepending https://) instead of "jmap.nuncio.mx", and mount real .well-known/jmap + Mailbox/get/Email/query/Email/get stub responses on it. This turns that test into the "offline E2E fetch→store→list" proof for JMAP.
- crates/nunciod/Cargo.toml — add wiremock.workspace = true to [dev-dependencies] (not currently present); it is already a workspace dependency (used by nuncio-mail/nuncio-cal).
crates/nuncio-mail/tests/wiremock_jmap_test.rs — the existing test only exercises the static helper functions (build_session_url/parse_session/build_email_get_request/parse_email_get_response) directly, never JmapEngine's actual MailBackend trait methods. Add a new test that constructs JmapEngine::with_credentials(...) pointed at the MockServer, and calls .sync_folders() and .sync_messages(folder_id, None) through the trait, asserting the real HTTP round trip produces the mocked folder/message data (mirrors crates/nuncio-cal/tests/wiremock_caldav_test.rs's pattern of exercising the backend trait itself, not just its parsing helpers).

Implementation steps

Add #[serde(rename_all = "camelCase")] to JmapEmail; add a test proving receivedAt/isUnread/bodySnippet now populate correctly from realistic camelCase JSON.
Move reqwest to [dependencies] in crates/nuncio-mail/Cargo.toml.
Add JmapMailbox/JmapMailboxGetResponse structs + JmapEngine::build_mailbox_get_request/parse_mailbox_get_response, and an Email/query request builder + id-list parser, following the exact shape of the existing Email/get builder/parser pair.
Extend JmapEngine with an internal reqwest::Client, host, username: Option<String>, password: Option<String>; add with_credentials; keep new uncredentialed.
Rewrite sync_folders/sync_messages to branch on credentials: real HTTP session discovery + Mailbox/Email JSON-RPC calls when present, else the existing static fallback (unchanged) when absent. A genuine HTTP failure or unparseable response must return Err(MailError::NetworkError(..))/Err(MailError::ParseFailed(..)) — never fall back to fake data.
Update nunciod::sync::build_mail_backend's AccountProtocol::Jmap arm to pass real server_host/email_address/password via with_credentials.
Add wiremock to nunciod's dev-dependencies; rewrite the JMAP branch of run_account_sync_persists_real_backend_messages_and_reports_success (or add a sibling test) to mount a MockServer and point sample_jmap_account.server_host at its .uri(), proving run_account_sync → real HTTP JMAP fetch → DatabaseEngine::save_email → db.get_message(...) end to end.
Add the new JmapEngine-through-MailBackend wiremock test in crates/nuncio-mail/tests/wiremock_jmap_test.rs.
Run cargo verify; fix any other existing test that assumed JmapEngine::new(...)'s old always-static behaviour if build_mail_backend construction changed its shape.

Acceptance criteria

JmapEmail has #[serde(rename_all = "camelCase")]; a test proves receivedAt/isUnread/bodySnippet parse correctly (not silently defaulted).
JmapEngine::with_credentials(...) performs real HTTP session discovery (GET /.well-known/jmap) and real Mailbox/get + Email/query/Email/get JSON-RPC calls against the discovered apiUrl; a network/parse failure surfaces as a genuine MailError, never fabricated data.
JmapEngine::new(...) (uncredentialed) keeps its existing deterministic, network-free fallback behaviour so existing unit tests are unaffected.
nunciod::sync::build_mail_backend constructs the JMAP engine with the account's real server_host/email_address/keyring password, mirroring the IMAP arm.
Wiremock test in crates/nuncio-mail/tests/wiremock_jmap_test.rs exercises JmapEngine through the MailBackend trait (sync_folders/sync_messages), not just the static parse helpers.
Offline E2E in crates/nunciod/src/sync.rs (wiremock-backed, server_host pointed at the mock server) proves fetch → store → db.get_message/db.list_messages for a JMAP account through run_account_sync, with no live network calls.
No proto change (this story reuses the existing nuncio.v1.Mail service unchanged).
cargo verify green (fmt + clippy -D warnings all-targets + test-all), 0 ignored.

Definition of done & process

Follow docs/STORY-WORKFLOW.md: no fabricated data; comment convention (intent-only, no story/GH/phase refs in .rs/.proto); mock-only tests. Branch story/m3-jmap-client off dev; PR to dev with the gate output pasted as evidence; the PO reviews before the issue is closed.

Dependencies / notes

No proto coordination needed (no nuncio.proto change). Not dependent on any other in-flight story. JMAP-native outbound send (RFC 8621 EmailSubmission) is explicitly OUT of scope: crates/nunciod/src/send.rs's SendMessage RPC always builds a real SmtpTransportEngine directly regardless of the account's inbound protocol (MailBackend::send_email is never called in production for either IMAP or JMAP today — grep confirms it), so JmapEngine::send_email can remain its current stub. Incremental (since_state-aware) sync for JMAP is deliberately out of scope, matching IMAP's current same-gap (tracked separately as M5-S2). Size: L.

## Context Story for milestone M3. **Read [`docs/STORY-WORKFLOW.md`](https://github.com/KofTwentyTwo/nuncio/blob/dev/docs/STORY-WORKFLOW.md) first** — it defines the process, gates, and patterns; this issue defines only the specific work. `nuncio-mail`'s `JmapEngine` (`crates/nuncio-mail/src/jmap.rs`) implements `MailBackend` but is entirely fake in production: `sync_folders` returns 3 hardcoded `Folder`s and `sync_messages` always re-parses one canned static JSON blob regardless of `folder_id`/`since_state`, and it never performs any HTTP I/O. `crates/nunciod/src/sync.rs`'s `build_mail_backend` (the ONLY place a concrete protocol engine is constructed) calls `JmapEngine::new(&config.id)` — it passes neither the account's `server_host`/`server_port` nor its keyring password, so a JMAP account today can never sync anything real, even though `nunciod::sync`'s generic fetch→persist plumbing (`fetch_and_persist`) already works for any `MailBackend`. There is also a live wire-format bug: `JmapEmail` (the raw JMAP `Email` object struct) has no `#[serde(rename_all = "camelCase")]`, so real JMAP servers' `receivedAt`/`isUnread`/`bodySnippet` keys never match the struct's snake_case field names and silently deserialize to `None`/defaults (the existing wiremock test never asserts on those fields, so this passes unnoticed today). The goal: make `JmapEngine` a real RFC 8620/8621 client — session discovery, `Email/query`+`Email/get`, and a real `Mailbox/get` folder listing — that flows through the exact same `MailBackend` → `fetch_and_persist` → `DatabaseEngine::save_email` path IMAP already uses, with NO new proto surface (the existing `nuncio.v1.Mail` service already covers both protocols). ## Goal `JmapEngine`, when constructed with real credentials, performs genuine HTTP session discovery against `/.well-known/jmap`, then real `Mailbox/get` and `Email/query`/`Email/get` JSON-RPC calls against the discovered `apiUrl`, and returns folders/messages actually parsed from the server's response — never canned data. It must NOT fake success: a session-discovery or JMAP-call HTTP/parse failure must surface as a genuine `MailError`, never silently fall back to the old static sample. A `JmapEngine` constructed with NO credentials (mirroring `ImapEngine::new`'s uncredentialed path) may keep today's deterministic no-network fallback behaviour so existing credential-less unit tests keep working — but the production path (`nunciod::sync::build_mail_backend`) MUST always construct it with the account's real host and keyring password, exactly like it already does for `ImapEngine::with_credentials`. ## Files to touch / patterns to mirror - `crates/nuncio-mail/Cargo.toml` — `reqwest` (line 33) is currently a `[dev-dependencies]`-only crate; JMAP is going to make real HTTP calls from production code, so move `reqwest.workspace = true` into `[dependencies]` (it can be removed from `[dev-dependencies]` once it's a regular dependency). - `crates/nuncio-mail/src/jmap.rs` — the crate to rewrite: - `JmapEmail` (line 62): add `#[serde(rename_all = "camelCase")]` on the struct. This is a real, currently-live bug: the struct's `received_at`/`is_unread`/`body_snippet` fields never match the wire's `receivedAt`/`isUnread`/`bodySnippet` keys, so every parsed message silently gets a zero timestamp, `read: true` (`!is_unread.unwrap_or(false)`), and no snippet. Add a regression test asserting these three fields round-trip correctly from camelCase JSON (the existing `wiremock_jmap_session_discovery_and_email_get_sync` test conspicuously never asserts on them). - `JmapEngine` (line 79): extend it to carry an HTTP client, the account's host, and optional `username`/`password`, mirroring `ImapEngine`'s shape in `crates/nuncio-mail/src/imap.rs` (`ImapEngine::new` vs `ImapEngine::with_credentials` at lines 173/183, and the uncredentialed static-fallback branch in `sync_folder_messages` at lines 324-342). Add `JmapEngine::with_credentials(account_id: &str, host: &str, username: &str, password: &str) -> Self`; keep `JmapEngine::new(account_id: &str)` for the uncredentialed/preview case. - Add a `Mailbox/get` request builder + response parser, mirroring the existing `build_email_get_request`/`parse_email_get_response` shape (lines 101-117, 143-202): a `JmapMailbox { id: String, name: String, #[serde(default)] total_emails: u32, #[serde(default)] unread_emails: u32 }` with `#[serde(rename_all = "camelCase")]` (RFC 8621 §2 `Mailbox` object uses `totalEmails`/`unreadEmails`), and a `JmapMailboxGetResponse` wrapper like `JmapEmailGetResponse`. - Add an `Email/query` request builder (`{"filter": {"inMailboxes": [mailbox_id]}}`) + a minimal response parser extracting the `ids` array, since `Email/get` needs explicit ids (or `None` for "all" — real servers require a bounded query first). - Rewrite the `MailBackend` impl (lines 233-291): - `sync_folders`: with credentials, GET the session URL (`Self::build_session_url(&self.host)`) with HTTP Basic auth (`username`/`password`), parse it, POST `Mailbox/get` to `session.api_url`, and map each `JmapMailbox` to a domain `Folder`. Without credentials, keep returning the existing static 3-folder list (do not remove the no-credential fallback; downstream tests and any preview/dry-run caller rely on `JmapEngine::new` staying deterministic and network-free). - `sync_messages(folder_id, since_state)`: with credentials, discover the session (or reuse a cached one), `Email/query` for `folder_id`'s mailbox id, then `Email/get` those ids, and return the parsed `Vec<Email>` + new state via `parse_email_get_response`. Mirror `ImapEngine::sync_messages`'s current scope exactly: it is honest to ignore `since_state` for now (a full re-fetch) — `ImapEngine` does the same today; incremental sync for either protocol is out of scope here (see the `M5-S2` roadmap story). Without credentials, keep today's static sample fallback. - `crates/nunciod/src/sync.rs` — `build_mail_backend` (line 126): change the `AccountProtocol::Jmap` arm from `Box::new(JmapEngine::new(&config.id))` to `Box::new(JmapEngine::with_credentials(&config.id, &config.server_host, &config.email_address, password))`, mirroring the `ImapSmtp` arm immediately above it. - This makes the existing `run_account_sync_persists_real_backend_messages_and_reports_success` test (in this file's `#[cfg(test)] mod tests`, using `sample_jmap_account`) attempt REAL network I/O against `"jmap.nuncio.mx"` once credentials flow through — which is forbidden (`STORY-WORKFLOW.md` §2.4: no live network calls in tests). You must fix this test as part of this story: point `sample_jmap_account`'s `server_host` at a `wiremock::MockServer`'s own `.uri()` (which already includes an `http://` scheme, so `JmapEngine::build_session_url` will use it as-is rather than prepending `https://`) instead of `"jmap.nuncio.mx"`, and mount real `.well-known/jmap` + `Mailbox/get`/`Email/query`/`Email/get` stub responses on it. This turns that test into the "offline E2E fetch→store→list" proof for JMAP. - `crates/nunciod/Cargo.toml` — add `wiremock.workspace = true` to `[dev-dependencies]` (not currently present); it is already a workspace dependency (used by `nuncio-mail`/`nuncio-cal`). - `crates/nuncio-mail/tests/wiremock_jmap_test.rs` — the existing test only exercises the static helper functions (`build_session_url`/`parse_session`/`build_email_get_request`/`parse_email_get_response`) directly, never `JmapEngine`'s actual `MailBackend` trait methods. Add a new test that constructs `JmapEngine::with_credentials(...)` pointed at the `MockServer`, and calls `.sync_folders()` and `.sync_messages(folder_id, None)` through the trait, asserting the real HTTP round trip produces the mocked folder/message data (mirrors `crates/nuncio-cal/tests/wiremock_caldav_test.rs`'s pattern of exercising the backend trait itself, not just its parsing helpers). ## Implementation steps 1. Add `#[serde(rename_all = "camelCase")]` to `JmapEmail`; add a test proving `receivedAt`/`isUnread`/`bodySnippet` now populate correctly from realistic camelCase JSON. 2. Move `reqwest` to `[dependencies]` in `crates/nuncio-mail/Cargo.toml`. 3. Add `JmapMailbox`/`JmapMailboxGetResponse` structs + `JmapEngine::build_mailbox_get_request`/`parse_mailbox_get_response`, and an `Email/query` request builder + id-list parser, following the exact shape of the existing `Email/get` builder/parser pair. 4. Extend `JmapEngine` with an internal `reqwest::Client`, `host`, `username: Option<String>`, `password: Option<String>`; add `with_credentials`; keep `new` uncredentialed. 5. Rewrite `sync_folders`/`sync_messages` to branch on credentials: real HTTP session discovery + Mailbox/Email JSON-RPC calls when present, else the existing static fallback (unchanged) when absent. A genuine HTTP failure or unparseable response must return `Err(MailError::NetworkError(..))`/`Err(MailError::ParseFailed(..))` — never fall back to fake data. 6. Update `nunciod::sync::build_mail_backend`'s `AccountProtocol::Jmap` arm to pass real `server_host`/`email_address`/`password` via `with_credentials`. 7. Add `wiremock` to `nunciod`'s dev-dependencies; rewrite the JMAP branch of `run_account_sync_persists_real_backend_messages_and_reports_success` (or add a sibling test) to mount a `MockServer` and point `sample_jmap_account.server_host` at its `.uri()`, proving `run_account_sync` → real HTTP JMAP fetch → `DatabaseEngine::save_email` → `db.get_message(...)` end to end. 8. Add the new `JmapEngine`-through-`MailBackend` wiremock test in `crates/nuncio-mail/tests/wiremock_jmap_test.rs`. 9. Run `cargo verify`; fix any other existing test that assumed `JmapEngine::new(...)`'s old always-static behaviour if `build_mail_backend` construction changed its shape. ## Acceptance criteria - [ ] `JmapEmail` has `#[serde(rename_all = "camelCase")]`; a test proves `receivedAt`/`isUnread`/`bodySnippet` parse correctly (not silently defaulted). - [ ] `JmapEngine::with_credentials(...)` performs real HTTP session discovery (`GET /.well-known/jmap`) and real `Mailbox/get` + `Email/query`/`Email/get` JSON-RPC calls against the discovered `apiUrl`; a network/parse failure surfaces as a genuine `MailError`, never fabricated data. - [ ] `JmapEngine::new(...)` (uncredentialed) keeps its existing deterministic, network-free fallback behaviour so existing unit tests are unaffected. - [ ] `nunciod::sync::build_mail_backend` constructs the JMAP engine with the account's real `server_host`/`email_address`/keyring password, mirroring the IMAP arm. - [ ] Wiremock test in `crates/nuncio-mail/tests/wiremock_jmap_test.rs` exercises `JmapEngine` through the `MailBackend` trait (`sync_folders`/`sync_messages`), not just the static parse helpers. - [ ] Offline E2E in `crates/nunciod/src/sync.rs` (wiremock-backed, `server_host` pointed at the mock server) proves fetch → store → `db.get_message`/`db.list_messages` for a JMAP account through `run_account_sync`, with no live network calls. - [ ] No proto change (this story reuses the existing `nuncio.v1.Mail` service unchanged). - [ ] `cargo verify` green (fmt + clippy `-D warnings` all-targets + test-all), 0 ignored. ## Definition of done & process Follow [`docs/STORY-WORKFLOW.md`](https://github.com/KofTwentyTwo/nuncio/blob/dev/docs/STORY-WORKFLOW.md): no fabricated data; comment convention (intent-only, no story/GH/phase refs in `.rs`/`.proto`); mock-only tests. Branch `story/m3-jmap-client` off `dev`; PR to `dev` with the gate output pasted as evidence; the PO reviews before the issue is closed. ## Dependencies / notes No proto coordination needed (no `nuncio.proto` change). Not dependent on any other in-flight story. JMAP-native outbound send (RFC 8621 `EmailSubmission`) is explicitly OUT of scope: `crates/nunciod/src/send.rs`'s `SendMessage` RPC always builds a real `SmtpTransportEngine` directly regardless of the account's inbound protocol (`MailBackend::send_email` is never called in production for either IMAP or JMAP today — grep confirms it), so `JmapEngine::send_email` can remain its current stub. Incremental (`since_state`-aware) sync for JMAP is deliberately out of scope, matching IMAP's current same-gap (tracked separately as `M5-S2`). Size: L.

KofTwentyTwo commented

2026-07-29 23:40:21 +00:00

(Migrated from github.com)

Delivered via PR #221 (merged to dev). Review PASS, zero findings — real JMAP session→query→get wired into MailBackend, camelCase bug fixed (all JMAP structs audited), canned fallbacks removed on the credentialed path (uncredentialed path confirmed test-only), sync wiring mirrors IMAP, mock-only tests. M3 (JMAP) complete.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#189