From b6987746ab9ae55cc5cbf01651fe7bac331bed56 Mon Sep 17 00:00:00 2001 From: James Maes Date: Fri, 14 Aug 2026 18:08:52 -0500 Subject: [PATCH 1/2] docs: chart the shortest route to a hand-drivable daemon The engine is ahead of its contract: it executes verified mutations but the Mail service has no RPC to ask for one, so a human cannot move, delete or flag anything. By this repo's definition of done the placement work has an engine and nothing else. Records the three steps that buy manual usability, what works and what does not if you boot it today, the 81-issue backlog behind them, and the two test traps (tracing callsite Interest, tokio::time::pause over real I/O) that are invisible at default parallelism. --- docs/PATH-TO-MANUAL-USE.md | 167 +++++++++++++++++++++++++++++++++++++ 1 file changed, 167 insertions(+) create mode 100644 docs/PATH-TO-MANUAL-USE.md diff --git a/docs/PATH-TO-MANUAL-USE.md b/docs/PATH-TO-MANUAL-USE.md new file mode 100644 index 0000000..072f535 --- /dev/null +++ b/docs/PATH-TO-MANUAL-USE.md @@ -0,0 +1,167 @@ +# Path to manual use + +> **Purpose:** the shortest honest route from where the engine is today to a +> daemon a human can drive by hand and test end to end. Written 2026-08-14, +> after the ADR 0002 sync model landed. Complements [`ROADMAP.md`](ROADMAP.md) +> (the full plan) and [`HANDOFF.md`](HANDOFF.md) (machine-to-machine state). + +## Where we actually are + +`dev` is green and carries the whole of [ADR 0002](adr/0002-multi-engine-sync-model.md): +message/placement identity split, four-rung IMAP change enumeration, three-state +verified mutations, per-account filter ownership, JMAP `Email/changes`. + +**The engine is ahead of its contract.** That is the one fact that matters here. + +``` +$ grep -c 'rpc \(MoveMessage\|DeleteMessage\|FlagMessage\)' \ + crates/nuncio-proto/proto/nuncio/v1/nuncio.proto +0 +``` + +The daemon can move, delete and flag mail — the outbox executes verified +mutations with three honest outcomes — but **nothing can ask it to**. By this +repo's own definition of done (*engine + proto + CLI command + offline E2E*), +the placement work is not finished: it has an engine and nothing else. + +## What that means if you boot it today + +Works: add an account, sync, list folders, list and read messages, search, +calendar and contacts sync, export, audit. + +Does not: **any mutation**. You cannot move, delete or flag a message from the +CLI, because the RPC does not exist. + +Two consequences of the same gap: + +- **Filters cannot be switched on.** `filters_enabled` is per-account and + **default off** (deliberately — one engine owns side-effecting actions, see + ADR 0002 §2). There is no proto field and no CLI command, so the only way to + enable it is editing the `accounts` row in SQLite by hand. +- **Placements are invisible.** `Message.folder_id` names the one occupancy the + message was reached through. A client cannot see that a message sits in three + folders, which is the thing the split exists to model. + +### Before you run it + +```bash +git checkout dev && git pull +rm -f ~/.nuncio/nuncio_main.db* # see below +cargo run -p nunciod # gRPC on 127.0.0.1:9420 +cargo run -p nuncio-cli -- system status +``` + +Schema migrations were removed: databases are **created, never migrated**. A +store from before that change errors on read rather than upgrading. Deleting it +is correct — accounts and credentials live in the OS keyring and survive; cached +mail re-syncs. Expect this again whenever `IDENTITY_SCHEMA_VERSION` bumps. + +## The critical path + +Three steps, in order. Everything else in the roadmap is behind them. + +### 1. #407 — cross-engine identity *(in flight)* + +IMAP requests `EMAILID` (RFC 8474, where the server advertises `OBJECTID`) and +`X-GM-MSGID` (where it advertises `X-GM-EXT-1`), so it reaches identity tier 1 +instead of only tiers 3–4. Until this lands, an IMAP engine and a JMAP engine +mint **disjoint key spaces** for the same mailbox and ADR 0002's convergence +claim is not true across protocols. + +Bumps `IDENTITY_SCHEMA_VERSION` (2 → 3): the store rebuilds and re-syncs rather +than re-keying every row incrementally through the placement repoint arm. + +### 2. #395 — the keystone + +The single thing standing between "engine works" and "a human can drive it": + +- **Mutation RPCs on `Mail`** — Move / Delete / Flag / Unflag, addressed by + message key **plus the placement to act on**, since a message can occupy + several mailboxes and `MOVE`/`DELETE` against the wrong one is destructive. +- **`Conflict` as a typed error**, not a response field, so clients that predate + it take the error path instead of silently misreading success. Conflicts are + durable and queryable (ADR 0002 §5), so this needs list/resolve RPCs too. +- **`Message` carries its placement set** — the field the split has been waiting + for. Today's `folder_id` becomes one view of it. +- **`filters_enabled` on the wire** so filter ownership is settable without + hand-editing SQLite. +- DAV conditional write-back. + +This takes the repo's one proto-touching-story slot (currently free) and gates +the M7 API freeze. + +### 3. Manual-readiness pass + +Drive every RPC from `nuncio-cli` against a live daemon, fix what is missing or +broken, and write the exercise guide. The open WS-F CLI issues (#327, #328, +#329, #332) largely fall out of this. + +## Scope, honestly + +**81 open issues** as of 2026-08-14 — that is the roadmap through M7, not a task +list: + +| open | milestone | +| ---: | --- | +| 15 | WS-B Mail model & mutations | +| 12 | WS-E Calendar/Contacts write & Search | +| 10 | WS-F Ops, usability & maintainability | +| 9 | WS-C Sync, push & lifecycle | +| 8 | WS-D Security to 9/10 · M6.5 de-fabrication · (no milestone) | +| 5 | M7 API freeze | +| 3 | M6 Security hardening · WS-A Contract hardening | + +The three steps above are what buy manual usability. The rest is the product. + +## Known-open, deliberately + +Recorded so nobody re-derives them: + +- **`recovery::tests::test_stage_2_backup_creation_and_stage_3_table_salvage`** + failed once in CI and has never reproduced (0/40 locally under artificial CPU + load, quiet across every run since). Genuinely undiagnosed — not fixed, not + understood. If it recurs, get a reproduction before attempting a fix. +- **#339** — `parse_email_get_response` keeps a fixed-folder signature. The + *production* path was corrected while implementing JMAP `Email/changes` + (removals key on folder, so the hardcoded `"inbox"` made every JMAP removal + inert); the public helper still reads as the issue describes. +- **`export_mbox` no longer emits `X-Nuncio-Folder`**, permanently. A message can + hold several placements; one header would name one arbitrarily and MBOX cannot + express the set. +- **A filter fire claim outlives its message.** `filter_fired` is never reaped, so + a deleted-and-re-arriving message never re-fires. Deliberate: reaping would + silently re-enable duplicate `FORWARD` / `CALL WEBHOOK`. +- **`db.rs` has one pre-existing `set_default`** log-capture test. Everything else + uses the process-global subscriber. Harmless today, worth folding into a + cleanup — see below. + +## Two traps that cost real time here + +Both are written up because they are invisible locally and expensive to +rediscover. + +1. **`tracing` caches callsite `Interest` process-globally; `set_default` is + per-thread.** A callsite first executed by a thread with no subscriber caches + `never` and is skipped forever, before any thread-local subscriber is + consulted. Reproduces at `--test-threads=2` (CI's core count), invisible at + higher parallelism. Every log-capture helper must install **one global** + subscriber and route per thread. Never reintroduce `set_default` / + `with_default`. +2. **`tokio::time::pause()` cannot wrap real blocking I/O.** Auto-advance fires + whenever the runtime *looks* idle — including while a `sqlx` query sits on the + blocking pool — and it also trips sqlx's pool-acquire timer (default 30s, + frequently the same duration as the timeout under test), turning a healthy + query into `PoolTimedOut` and the result into misleading zeros. Make timeouts + injectable and wait them out in real time instead. + +## Picking this up cold + +1. Read [ADR 0002](adr/0002-multi-engine-sync-model.md) — it is the design, and + its **Pre-release posture** section is load-bearing: nothing is released, + stores are disposable, behaviour may change without a deprecation path. +2. `cargo fmt --all -- --check`, `cargo check-all`, `cargo test-all` — the gate. + Also run `RUST_TEST_THREADS=2 cargo test-all` before trusting a green run; + default parallelism hides the flake class above. +3. `gh pr list` — PRs stack (`A → B → C`), and merging into `dev` does **not** + auto-close issues, because GitHub only honours `Closes #N` on merge into the + default branch. Close them by hand. -- 2.54.0 From 93f00cdd0bf6e84e935805ea195dcae6532b828f Mon Sep 17 00:00:00 2001 From: James Maes Date: Fri, 14 Aug 2026 18:41:10 -0500 Subject: [PATCH 2/2] feat(imap): key messages by their server object id IMAP hardcoded no object id into the message key, so it could only ever reach the Message-ID+content tier or the folder-scoped surrogate, while JMAP entered at the top tier from the server's Email id. One account synced through both protocols therefore minted disjoint key spaces. RFC 8474 EMAILID is the same identifier JMAP exposes, so requesting it makes the two keys equal. Negotiate per capability: EMAILID only where the server advertises OBJECTID, X-GM-MSGID only where it advertises X-GM-EXT-1. A server offering neither behaves exactly as before. EMAILID cannot go through the typed client: imap-proto's msg_att parser is a closed alt with no catch-all, and async-imap turns the resulting nom error into a fatal io::Error that closes the session. The raw layer reads it off the transport beneath ImapStream instead, so an attribute the crate cannot parse can never reach its decoder. Lifting stored rows to a stronger tier changes every message_key, so IDENTITY_SCHEMA_VERSION moves to 3 and the existing reconciliation rebuilds the cached store rather than repointing rows one at a time. --- crates/nuncio-core/src/model.rs | 31 +- crates/nuncio-mail/src/imap.rs | 200 ++++++++--- crates/nuncio-mail/src/imap_raw.rs | 327 +++++++++++++++++- crates/nuncio-mail/src/test_server.rs | 246 +++++++++++-- .../nuncio-mail/tests/imap_identity_test.rs | 231 +++++++++++++ .../nuncio-mail/tests/imap_raw_wire_test.rs | 85 ++++- crates/nuncio-store/src/db.rs | 12 +- docs/adr/0002-multi-engine-sync-model.md | 24 +- 8 files changed, 1055 insertions(+), 101 deletions(-) create mode 100644 crates/nuncio-mail/tests/imap_identity_test.rs diff --git a/crates/nuncio-core/src/model.rs b/crates/nuncio-core/src/model.rs index 0df4a26..1b76353 100644 --- a/crates/nuncio-core/src/model.rs +++ b/crates/nuncio-core/src/model.rs @@ -350,17 +350,30 @@ impl Email { /// The surrogate tier is folder-scoped and therefore *not* stable across a /// move; it is the honest answer when the server offered nothing better. /// - /// # Keys do not yet converge across engine types + /// # What convergence across engine types does and does not follow from /// /// Precedence makes a key reproducible from the same inputs, not from the - /// same *message*. The IMAP engine does not currently request `EMAILID` or - /// `X-GM-MSGID`, so it can only ever reach the `Message-ID`+content tier or - /// the surrogate, while JMAP supplies a server id and enters at the top - /// tier. Two engines syncing one account therefore produce disjoint key - /// spaces: the same message is two identities, one per engine. This is a - /// known gap in the convergent multi-engine model, not a property to rely - /// on -- do not assume a key derived by one engine addresses the row - /// another engine wrote. + /// same *message*. Two engines converge exactly when they enter at the same + /// tier with the same value. + /// + /// They do when the server publishes a stable object id and both engines + /// ask for it. The IMAP engine requests RFC 8474 `EMAILID` from an + /// `OBJECTID` server and `X-GM-MSGID` from an `X-GM-EXT-1` one, and JMAP's + /// Email object id is the same identifier RFC 8474 exposes to IMAP -- so + /// one message synced through both protocols yields one key. That is the + /// whole point of the OBJECTID extension. + /// + /// They do not when the server offers neither. IMAP then falls to the + /// `Message-ID`+content tier or the surrogate, both derived from what the + /// message or the mailbox happened to contain, while a JMAP engine against + /// the same account still enters at the top tier from a server-assigned id. + /// Those key spaces are disjoint, and no comparison across them means + /// anything. The surrogate additionally is not stable across a *move*, even + /// within one engine. + /// + /// Object ids are also only comparable within one account on one server: + /// RFC 8474 section 4 scopes uniqueness that way, which is why every tier + /// here is account-scoped. pub fn derive_message_key( account_id: &str, remote: RemoteIdentity<'_>, diff --git a/crates/nuncio-mail/src/imap.rs b/crates/nuncio-mail/src/imap.rs index 57c5922..d7c9130 100644 --- a/crates/nuncio-mail/src/imap.rs +++ b/crates/nuncio-mail/src/imap.rs @@ -175,6 +175,13 @@ pub(crate) struct MailboxSyncCapabilities { /// `NOMODSEQ` has no mod-sequences to compare against no matter what the /// server advertises. pub mailbox_has_modseq: bool, + /// Server advertises `OBJECTID` (RFC 8474), so `EMAILID` may be asked for. + /// Asking a server that did not advertise it is not merely rude: an + /// unrecognised FETCH item is answered `BAD`, costing a round trip per + /// batch to learn nothing. + pub objectid: bool, + /// Server advertises `X-GM-EXT-1`, so `X-GM-MSGID` may be asked for. + pub gmail_extensions: bool, } /// One folder-sync pass, resolved before any body is fetched. @@ -182,6 +189,11 @@ pub(crate) struct MailboxSyncCapabilities { pub(crate) struct FolderSyncPlan { /// The mechanism this pass negotiated. pub rung: SyncRung, + /// What the negotiation found the server able to do. Carried past the + /// planning stage because the identity extensions decide what the *fetch* + /// asks for, and a fetch that asks for an unadvertised item is answered + /// `BAD`. + pub caps: MailboxSyncCapabilities, pub uid_validity: Option, pub uid_next: Option, pub highest_modseq: Option, @@ -326,6 +338,24 @@ pub fn build_fetch_command_query() -> &'static str { "(FLAGS INTERNALDATE RFC822.SIZE ENVELOPE BODY.PEEK[])" } +/// Add `X-GM-MSGID` to a FETCH item list when, and only when, the server +/// advertised `X-GM-EXT-1`. +/// +/// Gmail's message id is the second identity tier, and unlike RFC 8474's +/// `EMAILID` it can be asked for through the typed client: `imap-proto` parses +/// the attribute and `Fetch::gmail_msg_id` reads it back. So it rides along on +/// the fetch the sync already issues and costs no extra round trip. +/// +/// A server that advertises nothing gets a query byte-identical to the one it +/// got before this existed. That is the point: capability negotiation must not +/// change what an unextended server sees. +fn with_gmail_msgid(items: &str, advertised: bool) -> String { + match items.strip_suffix(')').filter(|_| advertised) { + Some(head) => format!("{head} X-GM-MSGID)"), + None => items.to_string(), + } +} + /// The `UID FETCH` query for the cheap enumeration pass: UID plus `RFC822.SIZE` /// only, deliberately WITHOUT a `BODY.PEEK[]`. This lets the sync discover every /// message's UID and size (to decide per-message whether the body is safe to @@ -906,15 +936,24 @@ impl ImapEngine { drop(enumeration); let mut emails = Vec::new(); + let body_query = with_gmail_msgid(build_fetch_command_query(), plan.caps.gmail_extensions); + let header_query = with_gmail_msgid( + build_headers_only_command_query(), + plan.caps.gmail_extensions, + ); // Phase 2a: normal messages -- full bodies, in bounded UID-set batches. for chunk in body_uids.chunks(batch_size) { let set = join_uid_batch(chunk); + // Read the batch's object ids first. It has to be its own command: + // `EMAILID` cannot ride on the body fetch, because that response + // goes through a parser which treats the item as a fatal error. + // Issuing it here is also what keeps the raw layer's precondition + // true -- the last thing on this connection was a tagged + // completion, so the client holds no buffered bytes. + let object_ids = self.fetch_object_ids(session, plan.caps, &set).await?; let items = { - let mut fetch_stream = session - .uid_fetch(&set, build_fetch_command_query()) - .await - .map_err(|e| { + let mut fetch_stream = session.uid_fetch(&set, &body_query).await.map_err(|e| { MailError::ImapError(format!( "UID FETCH failed for folder '{}': {}", folder_id, e @@ -927,11 +966,16 @@ impl ImapEngine { // `UID FETCH` must carry one (RFC 3501 section 6.4.8), so this // only fires for a non-conforming server. Recording it anyway // would write a placement at the meaningless UID 0. - if item.uid.filter(|u| *u >= 1).is_none() { + let Some(uid) = item.uid.filter(|u| *u >= 1) else { skipped_no_uid += 1; continue; - } - emails.push(self.build_email_from_fetch(folder_id, server_uid_validity, item)?); + }; + emails.push(self.build_email_from_fetch( + folder_id, + server_uid_validity, + item, + object_ids.get(&uid).map(String::as_str), + )?); } } @@ -940,11 +984,13 @@ impl ImapEngine { // buffer an unbounded number of header fetches at once. for chunk in oversized_uids.chunks(batch_size) { let set = join_uid_batch(chunk); + // An oversized message needs the object id most: its body is never + // fetched, so there is no content hash and the alternative is the + // folder-scoped surrogate. + let object_ids = self.fetch_object_ids(session, plan.caps, &set).await?; let items = { - let mut fetch_stream = session - .uid_fetch(&set, build_headers_only_command_query()) - .await - .map_err(|e| { + let mut fetch_stream = + session.uid_fetch(&set, &header_query).await.map_err(|e| { MailError::ImapError(format!( "UID FETCH (headers) failed for folder '{}': {}", folder_id, e @@ -955,11 +1001,16 @@ impl ImapEngine { for item in &items { // Same guard as the body pass: an item with no UID cannot be // placed, and UID 0 is not an address. - if item.uid.filter(|u| *u >= 1).is_none() { + let Some(uid) = item.uid.filter(|u| *u >= 1) else { skipped_no_uid += 1; continue; - } - emails.push(self.build_headers_only_email(folder_id, server_uid_validity, item)); + }; + emails.push(self.build_headers_only_email( + folder_id, + server_uid_validity, + item, + object_ids.get(&uid).map(String::as_str), + )); } } @@ -1067,12 +1118,19 @@ impl ImapEngine { where S: AsyncRead + AsyncWrite + Unpin + Send + Debug + 'static, { - let (has_qresync, has_condstore) = { + let (has_qresync, has_condstore, has_objectid, has_gmail) = { let caps = session .capabilities() .await .map_err(|e| MailError::ImapError(format!("CAPABILITY query failed: {e}")))?; - (caps.has_str("QRESYNC"), caps.has_str("CONDSTORE")) + ( + caps.has_str("QRESYNC"), + caps.has_str("CONDSTORE"), + // RFC 8474 names the capability OBJECTID; EMAILID is the FETCH + // item it brings, and asking for it otherwise earns a BAD. + caps.has_str("OBJECTID"), + caps.has_str("X-GM-EXT-1"), + ) }; let checkpoint = since_state.and_then(parse_checkpoint); @@ -1127,6 +1185,8 @@ impl ImapEngine { qresync_enabled, condstore: has_condstore, mailbox_has_modseq: selected.highest_modseq.is_some(), + objectid: has_objectid, + gmail_extensions: has_gmail, }; let mut rung = choose_rung(checkpoint, selected.uid_validity, caps); // The QRESYNC SELECT was issued optimistically; if the server answered @@ -1144,6 +1204,7 @@ impl ImapEngine { let mut plan = FolderSyncPlan { rung, + caps, uid_validity: selected.uid_validity, uid_next: selected.uid_next, highest_modseq: selected.highest_modseq, @@ -1202,6 +1263,29 @@ impl ImapEngine { Ok(plan) } + /// The batch's `uid -> EMAILID` map, or an empty one when the server never + /// advertised RFC 8474 `OBJECTID`. + /// + /// The capability gate is the whole reason this is a separate method: a + /// server without `OBJECTID` must see exactly the traffic it saw before + /// object ids existed -- no extra command, no extra round trip -- and a + /// server that has it must be asked through the raw layer rather than the + /// typed client, which cannot parse the reply without dying. + async fn fetch_object_ids( + &self, + session: &mut async_imap::Session, + caps: MailboxSyncCapabilities, + uid_set: &str, + ) -> Result, MailError> + where + S: AsyncRead + AsyncWrite + Unpin + Send + Debug + 'static, + { + if !caps.objectid { + return Ok(std::collections::BTreeMap::new()); + } + crate::imap_raw::uid_fetch_email_ids(session, uid_set).await + } + /// List the folder's UIDs, optionally narrowed to those changed since /// `changed_since`. Requests no bodies and no flags, so the cost is one /// short line per message rather than a download. @@ -1267,12 +1351,15 @@ impl ImapEngine { /// and spreading them out buys no clarity while pushing this function past /// the argument-count lint. /// - /// In practice every IMAP call site passes `email_id: None` and - /// `gm_msgid: None` today, because the FETCH never asks for them, so this - /// engine reaches only the `Message-ID`+content tier or the surrogate. The - /// JMAP engine enters at the top tier, which means an IMAP-derived key and - /// a JMAP-derived key for the *same* message will not match. Syncing one - /// account over both engines does not converge on a shared identity yet. + /// Both server-assigned tiers are reachable from here. `EMAILID` comes from + /// a raw `UID FETCH` issued only against an `OBJECTID` server + /// ([`crate::imap_raw::uid_fetch_email_ids`]) and `X-GM-MSGID` from the + /// typed fetch against an `X-GM-EXT-1` server, so an IMAP-derived key and a + /// JMAP-derived key for the same message on a server that publishes object + /// ids through both protocols are the same key -- which is what RFC 8474 + /// exists to make possible. Against a server that advertises neither, this + /// engine still lands on the `Message-ID`+content tier or the surrogate, + /// and those do not converge with a JMAP engine's server-assigned ids. fn place( &self, folder_id: &str, @@ -1298,11 +1385,16 @@ impl ImapEngine { /// when it does not (a server that returned only the ENVELOPE), a minimal /// record is synthesised from the envelope so the message is still surfaced /// honestly. + /// + /// `email_id` is this UID's RFC 8474 object id when the folder pass read + /// one; `X-GM-MSGID` is read from the item itself, since the typed parser + /// understands that attribute and the raw layer never needs to see it. fn build_email_from_fetch( &self, folder_id: &str, server_uid_validity: Option, fetch_data: &async_imap::types::Fetch, + email_id: Option<&str>, ) -> Result { let uid_num = fetch_data.uid.unwrap_or(0); // The message is addressed on the wire by its UID within the folder's @@ -1318,19 +1410,11 @@ impl ImapEngine { .flags() .any(|flag| matches!(flag, async_imap::types::Flag::Seen)); - // EMAILID (RFC 8474) and X-GM-MSGID are not requested by the current - // FETCH, so identity rests on the Message-ID/content pair where the - // message carried a Message-ID and on the folder-scoped surrogate - // otherwise. - // - // Adding those FETCH items is not a local change. Every already-stored - // message would derive a DIFFERENT key on its next resync, so each - // re-reported placement repoints to the new key -- and the old message - // row, now referenced by nothing, is never reaped. Its decrypted body - // stays in the plaintext FTS index forever, which is exactly the leak - // the reap on placement removal exists to prevent. The FETCH items can - // only be requested once the repoint path reaps the message it - // abandons; until then, requesting them leaks plaintext. + // Gmail's id is a 64-bit integer on the wire; it is rendered decimal + // here because identity inputs are hashed as strings and the rendering + // has to be the one every engine would produce from the same number. + let gm_msgid = fetch_data.gmail_msg_id().map(u64::to_string); + if let Some(raw_bytes) = fetch_data.body() { let mut email = MimeParserAdapter::parse_mime(&self.account_id, raw_bytes)?; let (identity, placement) = self.place( @@ -1338,8 +1422,8 @@ impl ImapEngine { &uid_validity, &remote_id, RemoteIdentity { - email_id: None, - gm_msgid: None, + email_id, + gm_msgid: gm_msgid.as_deref(), message_id: email.message_id.as_deref(), content_hash: email.content_hash.as_deref(), }, @@ -1383,16 +1467,16 @@ impl ImapEngine { .and_then(|raw| Email::normalize_message_id(&String::from_utf8_lossy(raw))); // With no octets there is no content hash, and the `Message-ID` alone - // is never enough to key on, so this path lands on the surrogate tier. - // That is the honest answer: nothing was fetched that could prove two - // folders hold the same message. + // is never enough to key on, so absent a server-assigned id this path + // lands on the surrogate tier. That is the honest answer: nothing was + // fetched that could prove two folders hold the same message. let (identity, placement) = self.place( folder_id, &uid_validity, &remote_id, RemoteIdentity { - email_id: None, - gm_msgid: None, + email_id, + gm_msgid: gm_msgid.as_deref(), message_id: message_id.as_deref(), content_hash: None, }, @@ -1429,6 +1513,7 @@ impl ImapEngine { folder_id: &str, server_uid_validity: Option, fetch_data: &async_imap::types::Fetch, + email_id: Option<&str>, ) -> PlacedMessage { let uid_num = fetch_data.uid.unwrap_or(0); let remote_id = uid_num.to_string(); @@ -1438,6 +1523,7 @@ impl ImapEngine { let is_read = fetch_data .flags() .any(|flag| matches!(flag, async_imap::types::Flag::Seen)); + let gm_msgid = fetch_data.gmail_msg_id().map(u64::to_string); if let Some(header_bytes) = fetch_data.header() { if let Ok(mut email) = MimeParserAdapter::parse_mime(&self.account_id, header_bytes) { @@ -1452,16 +1538,17 @@ impl ImapEngine { // something else, so drop it; the `Message-ID` it recovered // from those headers is still correct and is kept. Dropping it // also costs this message the content tier -- a `Message-ID` - // never keys on its own -- so an oversized message identifies - // by its folder-scoped surrogate until the octets are fetched. + // never keys on its own -- so absent a server-assigned id an + // oversized message identifies by its folder-scoped surrogate + // until the octets are fetched. email.content_hash = None; let (identity, placement) = self.place( folder_id, &uid_validity, &remote_id, RemoteIdentity { - email_id: None, - gm_msgid: None, + email_id, + gm_msgid: gm_msgid.as_deref(), message_id: email.message_id.as_deref(), content_hash: None, }, @@ -1479,13 +1566,17 @@ impl ImapEngine { // No usable header section: fall back to whatever the envelope offers. // `build_email_from_fetch` cannot error on this path (no body to parse), // but if it ever did, surface a minimal honest record rather than panic. - self.build_email_from_fetch(folder_id, server_uid_validity, fetch_data) + self.build_email_from_fetch(folder_id, server_uid_validity, fetch_data, email_id) .unwrap_or_else(|_| { let (identity, placement) = self.place( folder_id, &uid_validity, &remote_id, - RemoteIdentity::default(), + RemoteIdentity { + email_id, + gm_msgid: gm_msgid.as_deref(), + ..RemoteIdentity::default() + }, is_read, ); PlacedMessage { @@ -2141,6 +2232,10 @@ mod tests { qresync_enabled: true, condstore: true, mailbox_has_modseq: true, + // The identity extensions decide what a FETCH asks for, never + // which rung a pass runs on; they are set for completeness. + objectid: true, + gmail_extensions: true, } } @@ -2217,7 +2312,8 @@ mod tests { MailboxSyncCapabilities { qresync_enabled: false, condstore: true, - mailbox_has_modseq: true + mailbox_has_modseq: true, + ..MailboxSyncCapabilities::default() } ), SyncRung::Condstore @@ -2231,7 +2327,8 @@ mod tests { MailboxSyncCapabilities { qresync_enabled: false, condstore: false, - mailbox_has_modseq: false + mailbox_has_modseq: false, + ..MailboxSyncCapabilities::default() } ), SyncRung::UidSetDiff @@ -2247,7 +2344,8 @@ mod tests { MailboxSyncCapabilities { qresync_enabled: true, condstore: true, - mailbox_has_modseq: false + mailbox_has_modseq: false, + ..MailboxSyncCapabilities::default() } ), SyncRung::Full diff --git a/crates/nuncio-mail/src/imap_raw.rs b/crates/nuncio-mail/src/imap_raw.rs index cee5f64..b4eb29b 100644 --- a/crates/nuncio-mail/src/imap_raw.rs +++ b/crates/nuncio-mail/src/imap_raw.rs @@ -1,8 +1,8 @@ //! Raw IMAP command layer: the responses `async-imap`'s typed API discards. //! -//! The engine needs four things the typed client cannot express, and all four -//! decide whether a sync saw everything or whether a mutation actually -//! happened: +//! The engine needs five things the typed client cannot express, and they all +//! decide whether a sync saw everything, whether a mutation actually happened, +//! or whether two engines can agree on what a message *is*: //! //! | Needed | Why the typed API cannot give it | //! | --- | --- | @@ -10,6 +10,7 @@ //! | `SELECT (QRESYNC …)` | no such method, and `select()`'s parser drops `VANISHED` | //! | `COPYUID` | `uid_copy`/`uid_mv` return `Result<()>`; the tagged response *code* is discarded | //! | `[MODIFIED …]` | same discard, and `imap-proto` has no `ResponseCode` variant for it at all | +//! | `EMAILID` | `imap-proto` cannot *parse* it, and failing to parse kills the connection -- see [`uid_fetch_email_ids`] | //! //! ## Why this is hand-rolled rather than a fork //! @@ -32,13 +33,15 @@ //! against a server that emits the real syntax, and the alternative is having //! no conflict detection at all. +use std::collections::BTreeMap; use std::fmt::Debug; +use std::sync::atomic::{AtomicU64, Ordering}; use async_imap::Session; // Reached through `async-imap`'s re-export rather than a direct dependency, // so the parser types can never version-skew against the client using them. use async_imap::imap_proto::{Response, ResponseCode, Status, UidSetMember}; -use tokio::io::{AsyncRead, AsyncWrite}; +use tokio::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt}; use crate::parser::MailError; @@ -386,10 +389,326 @@ where Ok(exchange) } +/// Upper bound on the bytes one raw exchange will accumulate before giving up. +/// +/// The raw reader has no framing beyond "a line beginning with our tag", so a +/// server that answers with an endless stream and never completes the command +/// would otherwise grow this buffer without limit. One `EMAILID` line is a few +/// dozen bytes and a batch is capped at a few hundred messages, so this is +/// orders of magnitude above any legitimate reply. +const MAX_RAW_RESPONSE_BYTES: usize = 4 * 1024 * 1024; + +/// Tag prefix for commands this module writes straight to the transport. +/// +/// `async-imap` allocates its own tags from an `IdGenerator` that emits +/// `format!("A{:04}", n % 10_000)` -- always the letter `A` followed by four +/// decimal digits. A tag starting with `NX` therefore cannot collide with one +/// the typed client will ever issue, no matter how many commands a session +/// runs or how often the generator wraps. Colliding would be worse than +/// untidy: two commands sharing a tag makes each one's completion look like +/// the other's, and a reader would stop at the wrong response. +const RAW_TAG_PREFIX: &str = "NX"; + +/// Sequence behind [`RAW_TAG_PREFIX`]. Process-wide rather than per-session +/// because it costs nothing and makes a tag unique in a packet capture across +/// every connection, which is the only thing anyone reads these tags for. +static RAW_TAG_SEQUENCE: AtomicU64 = AtomicU64::new(0); + +fn next_raw_tag() -> String { + let n = RAW_TAG_SEQUENCE.fetch_add(1, Ordering::Relaxed); + format!("{RAW_TAG_PREFIX}{n}") +} + +/// `UID FETCH (UID EMAILID)` (RFC 8474 section 5.2), read straight off +/// the transport, returning `uid -> EMAILID`. +/// +/// # Why this bypasses the client entirely +/// +/// `imap-proto` 0.16.7 cannot parse `EMAILID`: its `msg_att` parser is a closed +/// `alt(...)` of known attributes with no catch-all, so an `EMAILID` item is a +/// hard nom error rather than an unknown-but-tolerated one. `async-imap`'s +/// decoder turns any non-`Incomplete` parse error into a fatal `io::Error` and +/// then latches `read_closed`, after which the stream yields `None` forever -- +/// the session is dead, not degraded. So `EMAILID` must never reach +/// `parse_response`, which rules out `Session::uid_fetch` **and** +/// [`run_collected`], since the latter reads through `Session::read_response` +/// and therefore through the same decoder. +/// +/// What is left is the transport underneath: `Session` exposes `&mut T` +/// (`AsMut`, and `Connection::get_mut` through `Deref`), so the command is +/// written and its reply scanned here, by hand, and no byte of it is offered +/// to nom. +/// +/// # The two conditions this is only correct under +/// +/// 1. **The client's read buffer must be empty.** `ImapStream` owns a buffer +/// that can hold bytes already pulled off the socket. Raw reads bypass it, +/// so anything sitting there is invisible to this scanner. Call this only +/// at a quiescent point -- immediately after a previous command's *tagged* +/// completion has been read -- and never interleaved with an in-flight +/// typed command or a live response stream. +/// 2. **The tag must not collide** with the client's own. See +/// [`RAW_TAG_PREFIX`]. +/// +/// # Degrading rather than failing +/// +/// A tagged `NO`/`BAD` yields an empty map, not an error. The object id is an +/// identity *upgrade*: without it a message still syncs under the +/// `Message-ID`+content tier or the surrogate, so a server that advertises +/// `OBJECTID` and then refuses the fetch must cost the sync precision, never +/// the mail. A stalled read is different and does surface, because a server +/// that stopped answering mid-command has left the session unusable. +pub async fn uid_fetch_email_ids( + session: &mut Session, + uid_set: &str, +) -> Result, MailError> +where + S: AsyncRead + AsyncWrite + Unpin + Send + Debug + 'static, +{ + if uid_set.is_empty() { + return Ok(BTreeMap::new()); + } + + let tag = next_raw_tag(); + let command = format!("{tag} UID FETCH {uid_set} (UID EMAILID)\r\n"); + + let stream: &mut S = session.as_mut(); + stream + .write_all(command.as_bytes()) + .await + .map_err(|e| MailError::ImapError(format!("failed to issue '{command:?}': {e}")))?; + stream + .flush() + .await + .map_err(|e| MailError::ImapError(format!("failed to flush '{command:?}': {e}")))?; + + let (lines, tagged) = read_until_tagged(stream, &tag).await?; + + if !tagged_is_ok(&tagged) { + tracing::warn!( + response = %tagged, + "server refused UID FETCH (UID EMAILID) despite advertising OBJECTID; \ + this pass identifies its messages by a weaker tier" + ); + return Ok(BTreeMap::new()); + } + + let mut ids = BTreeMap::new(); + for line in &lines { + if let Some((uid, email_id)) = parse_email_id_line(line) { + ids.insert(uid, email_id); + } + } + Ok(ids) +} + +/// Read complete CRLF-terminated lines until one begins with `tag`, returning +/// the lines before it and the tagged line itself. +/// +/// Line-oriented scanning is sound *for this command only*: an `EMAILID` +/// response carries no literal (`{n}`) section, so no line body can contain an +/// embedded CRLF. It must not be reused for a command whose reply can carry +/// literals -- a body fetch -- without adding literal handling. +async fn read_until_tagged(stream: &mut S, tag: &str) -> Result<(Vec, String), MailError> +where + S: AsyncRead + AsyncWrite + Unpin, +{ + let mut buffer: Vec = Vec::new(); + let mut chunk = [0u8; 4096]; + let mut scanned = 0usize; + let mut lines: Vec = Vec::new(); + let tag_prefix = format!("{tag} "); + + loop { + let read = tokio::time::timeout(RESPONSE_READ_TIMEOUT, stream.read(&mut chunk)) + .await + .map_err(|_| { + MailError::FetchStalled(format!( + "no response to raw '{tag}' within {RESPONSE_READ_TIMEOUT:?}" + )) + })? + .map_err(|e| MailError::ImapError(format!("reading response to raw '{tag}': {e}")))?; + if read == 0 { + return Err(MailError::ImapError(format!( + "connection closed before raw '{tag}' completed" + ))); + } + buffer.extend_from_slice(&chunk[..read]); + if buffer.len() > MAX_RAW_RESPONSE_BYTES { + return Err(MailError::ImapError(format!( + "raw '{tag}' response exceeded {MAX_RAW_RESPONSE_BYTES} bytes without completing" + ))); + } + + // Consume every line that is now complete. `scanned` marks how far the + // buffer has already been split, so a reply arriving in many chunks is + // not re-scanned from the start each time. + while let Some(offset) = buffer[scanned..].iter().position(|b| *b == b'\n') { + let end = scanned + offset; + let line = String::from_utf8_lossy(&buffer[scanned..end]) + .trim_end_matches('\r') + .to_string(); + scanned = end + 1; + if line.starts_with(&tag_prefix) { + return Ok((lines, line)); + } + lines.push(line); + } + } +} + +/// Whether a tagged completion line reports `OK`. +fn tagged_is_ok(tagged: &str) -> bool { + tagged + .split_whitespace() + .nth(1) + .is_some_and(|status| status.eq_ignore_ascii_case("OK")) +} + +/// Pull `(uid, EMAILID)` out of one untagged `FETCH` line. +/// +/// Both wire spellings are accepted. RFC 8474 section 5.2 defines the response +/// as `"EMAILID" SP "(" objectid ")"`, but the bare `EMAILID ` form +/// is what a hand-rolled scanner meets often enough that rejecting it would +/// mean silently dropping to a weaker identity tier against a server that +/// answered the question correctly enough. +/// +/// Returns `None` for anything that is not an untagged `FETCH` carrying both a +/// UID and a non-empty object id -- there is nothing to key on without both. +fn parse_email_id_line(line: &str) -> Option<(u32, String)> { + let upper = line.to_ascii_uppercase(); + if !upper.starts_with("* ") || !upper.contains(" FETCH ") { + return None; + } + // `to_ascii_uppercase` is byte-length preserving, so offsets found in the + // uppercased copy index the original exactly. Item *names* are + // case-insensitive (RFC 3501 section 9); an object id is not, so the value + // is always taken from the original line. + let uid = find_item_value(&upper, line, "UID ")?.parse::().ok()?; + let email_id = find_item_value(&upper, line, "EMAILID ")?; + let email_id = email_id + .trim_start_matches('(') + .trim_end_matches(')') + .trim_matches('"') + .to_string(); + if email_id.is_empty() { + return None; + } + Some((uid, email_id)) +} + +/// The token following FETCH item `name` in `line`, located via its uppercased +/// twin `upper`. +/// +/// The name must sit on a token boundary, so `UID ` never matches the tail of +/// another attribute name. A parenthesised value is returned whole, brackets +/// included, for the caller to strip. +fn find_item_value(upper: &str, line: &str, name: &str) -> Option { + let mut from = 0usize; + loop { + let at = from + upper[from..].find(name)?; + let boundary = at == 0 + || upper[..at] + .chars() + .next_back() + .is_some_and(|c| c == '(' || c == ' '); + if boundary { + let value_start = at + name.len(); + let rest = &line[value_start..]; + let end = if rest.starts_with('(') { + rest.find(')').map_or(rest.len(), |i| i + 1) + } else { + rest.find([' ', ')']).unwrap_or(rest.len()) + }; + let value = rest[..end].trim(); + if !value.is_empty() { + return Some(value.to_string()); + } + } + from = at + name.len(); + } +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn an_object_id_is_read_from_both_wire_spellings() { + // RFC 8474 5.2 parenthesises the id; the bare form appears in the wild. + // Both must key the same message, or which spelling a server chose + // would decide whether identity converges. + assert_eq!( + parse_email_id_line("* 1 FETCH (UID 7 EMAILID (M00000001))"), + Some((7, "M00000001".to_string())) + ); + assert_eq!( + parse_email_id_line("* 1 FETCH (UID 7 EMAILID M00000001)"), + Some((7, "M00000001".to_string())) + ); + // Item names are case-insensitive; the id itself is not. + assert_eq!( + parse_email_id_line("* 1 fetch (uid 7 emailid (Mixed-Case_01))"), + Some((7, "Mixed-Case_01".to_string())) + ); + // Order is not fixed, and other attributes may sit between them. + assert_eq!( + parse_email_id_line("* 3 FETCH (EMAILID (M9) FLAGS (\\Seen) UID 12)"), + Some((12, "M9".to_string())) + ); + } + + #[test] + fn a_line_missing_either_half_yields_nothing() { + // Half an answer is not a weaker identity, it is no identity: without + // the UID there is nothing to attach the id to, and without the id + // there is nothing to attach. + assert_eq!( + parse_email_id_line("* 1 FETCH (UID 7 FLAGS (\\Seen))"), + None + ); + assert_eq!(parse_email_id_line("* 1 FETCH (EMAILID (M1))"), None); + assert_eq!(parse_email_id_line("* 1 FETCH (UID 7 EMAILID ())"), None); + assert_eq!(parse_email_id_line("NX0 OK UID FETCH completed"), None); + assert_eq!(parse_email_id_line("* 4 EXPUNGE"), None); + } + + #[test] + fn an_item_name_is_only_matched_on_a_token_boundary() { + // RFC 8474 defines THREADID alongside EMAILID, so a line carrying both + // is ordinary. Every occurrence of `ID ` in it is the tail of a longer + // attribute name; without the boundary rule a search would read + // THREADID's value as the item's own and key the message on it. + let line = "* 1 FETCH (THREADID (T1) EMAILID (M1) UID 7)"; + let upper = line.to_ascii_uppercase(); + assert_eq!(find_item_value(&upper, line, "ID "), None); + assert_eq!( + find_item_value(&upper, line, "EMAILID "), + Some("(M1)".to_string()) + ); + assert_eq!(find_item_value(&upper, line, "UID "), Some("7".to_string())); + assert_eq!(parse_email_id_line(line), Some((7, "M1".to_string()))); + } + + #[test] + fn only_a_tagged_ok_counts_as_a_usable_answer() { + assert!(tagged_is_ok("NX7 OK UID FETCH completed")); + assert!(tagged_is_ok("NX7 ok uid fetch completed")); + assert!(!tagged_is_ok("NX7 NO server does not do that")); + assert!(!tagged_is_ok("NX7 BAD unknown item")); + assert!(!tagged_is_ok("NX7")); + } + + #[test] + fn raw_tags_can_never_collide_with_the_clients_own() { + // async-imap's IdGenerator emits `A` + four digits and wraps at 10_000, + // so a shared tag is only impossible if the prefix differs. + let tag = next_raw_tag(); + assert!(tag.starts_with(RAW_TAG_PREFIX)); + assert_ne!(&tag[..1], "A"); + assert_ne!(next_raw_tag(), tag, "each raw command gets its own tag"); + } + #[test] fn modified_response_code_is_recovered_from_the_tagged_text() { // imap-proto has no ResponseCode::Modified, and its opt(resp_text_code) diff --git a/crates/nuncio-mail/src/test_server.rs b/crates/nuncio-mail/src/test_server.rs index 9eaffea..357517d 100644 --- a/crates/nuncio-mail/src/test_server.rs +++ b/crates/nuncio-mail/src/test_server.rs @@ -48,21 +48,6 @@ pub enum ServerProfile { } impl ServerProfile { - /// The `CAPABILITY` atom list for this profile. `UIDPLUS` and `MOVE` are - /// advertised by all three because every profile we model supports them. - fn capability_line(self) -> String { - let mut caps = vec!["IMAP4rev1", "UIDPLUS", "MOVE", "ENABLE", "LIST-STATUS"]; - match self { - ServerProfile::Basic => {} - ServerProfile::Condstore => caps.push("CONDSTORE"), - ServerProfile::Qresync => { - caps.push("CONDSTORE"); - caps.push("QRESYNC"); - } - } - caps.join(" ") - } - fn supports_condstore(self) -> bool { matches!(self, ServerProfile::Condstore | ServerProfile::Qresync) } @@ -72,6 +57,57 @@ impl ServerProfile { } } +/// Which server-assigned message-id extensions this server publishes. +/// +/// Independent of [`ServerProfile`] because the two axes genuinely are +/// independent in the field: Gmail advertises `X-GM-EXT-1` with CONDSTORE and +/// no QRESYNC, Fastmail advertises `OBJECTID` with QRESYNC, and Exchange +/// advertises neither. Selectable per instance so a test can pin the +/// behaviour of a server that offers *nothing*, which is the case that must +/// stay byte-identical to how the engine behaved before object ids existed. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub struct IdentityExtensions { + /// RFC 8474 `OBJECTID`, so `EMAILID` is served when asked for. + pub objectid: bool, + /// Gmail's `X-GM-EXT-1`, so `X-GM-MSGID` is served when asked for. + pub gmail: bool, +} + +impl IdentityExtensions { + /// A server that publishes no stable message id at all. + pub fn none() -> Self { + Self::default() + } + + /// RFC 8474 only. + pub fn objectid() -> Self { + Self { + objectid: true, + gmail: false, + } + } + + /// Gmail's vendor extension only, which is what Gmail itself does. + pub fn gmail() -> Self { + Self { + objectid: false, + gmail: true, + } + } + + /// The `CAPABILITY` atoms these extensions contribute. + fn atoms(self) -> Vec<&'static str> { + let mut atoms = Vec::new(); + if self.objectid { + atoms.push("OBJECTID"); + } + if self.gmail { + atoms.push("X-GM-EXT-1"); + } + atoms + } +} + /// One message as the server holds it. #[derive(Debug, Clone)] pub struct MockMessage { @@ -80,6 +116,23 @@ pub struct MockMessage { pub flags: BTreeSet, /// Full RFC822 octets served for `BODY[]`. pub body: String, + /// The server-assigned ids this message keeps wherever it sits. + pub object_ids: ObjectIds, +} + +/// The message-level ids a server assigns once and reports identically from +/// every mailbox holding the message. +/// +/// That invariant is the entire point of RFC 8474: a UID is per-mailbox, an +/// `EMAILID` is not, so a copy in a second folder reports the same one. A mock +/// that minted a fresh id per placement would model a server that does not +/// exist and would let a broken client pass. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ObjectIds { + /// RFC 8474 `EMAILID`. + pub email_id: String, + /// Gmail `X-GM-MSGID`, a 64-bit integer on the wire. + pub gm_msgid: u64, } /// A mailbox: its UID space, its mod-sequence, and the tombstones needed to @@ -113,7 +166,7 @@ impl MockMailbox { self.highest_modseq } - fn append(&mut self, body: &str, flags: &[&str]) -> u32 { + fn append(&mut self, body: &str, flags: &[&str], object_ids: ObjectIds) -> u32 { let uid = self.uid_next; self.uid_next += 1; let modseq = self.next_modseq(); @@ -122,6 +175,7 @@ impl MockMailbox { modseq, flags: flags.iter().map(|f| (*f).to_string()).collect(), body: body.to_string(), + object_ids, }); uid } @@ -150,21 +204,57 @@ impl MockMailbox { #[derive(Debug)] pub struct ServerState { pub profile: ServerProfile, + /// Which server-assigned id extensions this instance publishes. + pub extensions: IdentityExtensions, pub mailboxes: BTreeMap, /// Every command line received, across all connections, in arrival order. /// Tests assert on this to prove *which* protocol path ran -- e.g. that a /// second sync narrowed its FETCH rather than re-fetching everything. pub commands: Vec, next_uid_validity: u32, + next_object_id: u64, } impl ServerState { - fn new(profile: ServerProfile) -> Self { + fn new(profile: ServerProfile, extensions: IdentityExtensions) -> Self { Self { profile, + extensions, mailboxes: BTreeMap::new(), commands: Vec::new(), next_uid_validity: 1000, + next_object_id: 1, + } + } + + /// The `CAPABILITY` atom list. `UIDPLUS` and `MOVE` are advertised by every + /// profile because every server we model supports them; the sync-ladder and + /// identity atoms are what varies. + fn capability_line(&self) -> String { + let mut caps = vec!["IMAP4rev1", "UIDPLUS", "MOVE", "ENABLE", "LIST-STATUS"]; + match self.profile { + ServerProfile::Basic => {} + ServerProfile::Condstore => caps.push("CONDSTORE"), + ServerProfile::Qresync => { + caps.push("CONDSTORE"); + caps.push("QRESYNC"); + } + } + caps.extend(self.extensions.atoms()); + caps.join(" ") + } + + /// Mint the ids for a message the test did not name explicitly. + fn mint_object_ids(&mut self) -> ObjectIds { + let n = self.next_object_id; + self.next_object_id += 1; + ObjectIds { + // RFC 8474 section 4 restricts an objectid to 1*255 of + // ALPHA/DIGIT/`_`/`-`, so this shape is representative rather than + // decorative -- a scanner that mishandles it mishandles the real + // thing. + email_id: format!("M{n:08}"), + gm_msgid: 1_278_455_344_230_334_000 + n, } } } @@ -195,11 +285,20 @@ fn lock_state(state: &Arc>) -> MutexGuard<'_, ServerState> { } impl MockImapServer { - /// Bind an ephemeral loopback port and start accepting connections. + /// Bind an ephemeral loopback port and start accepting connections, with + /// no server-assigned message-id extensions. pub async fn start(profile: ServerProfile) -> std::io::Result { + Self::start_with_identity(profile, IdentityExtensions::none()).await + } + + /// As [`Self::start`], additionally publishing `extensions`. + pub async fn start_with_identity( + profile: ServerProfile, + extensions: IdentityExtensions, + ) -> std::io::Result { let listener = TcpListener::bind("127.0.0.1:0").await?; let addr = listener.local_addr()?; - let state = Arc::new(Mutex::new(ServerState::new(profile))); + let state = Arc::new(Mutex::new(ServerState::new(profile, extensions))); let accept_state = Arc::clone(&state); let accept_task = tokio::spawn(async move { @@ -241,8 +340,26 @@ impl MockImapServer { .insert(name.to_string(), MockMailbox::new(uid_validity)); } - /// Append a message, creating the mailbox if absent. Returns its UID. + /// Append a message under freshly minted object ids, creating the mailbox + /// if absent. Returns its UID. pub fn append_message(&self, mailbox: &str, body: &str, flags: &[&str]) -> u32 { + let ids = lock_state(&self.state).mint_object_ids(); + self.append_message_with_object_ids(mailbox, body, flags, ids) + } + + /// Append a message under *given* object ids. + /// + /// This is how a test puts the same message in two mailboxes: a real + /// OBJECTID server reports one `EMAILID` for both copies, and only that + /// makes the cross-folder identity claim testable. Passing distinct ids to + /// two appends models two genuinely different messages. + pub fn append_message_with_object_ids( + &self, + mailbox: &str, + body: &str, + flags: &[&str], + object_ids: ObjectIds, + ) -> u32 { let mut state = lock_state(&self.state); let uid_validity = state.next_uid_validity; let entry = state.mailboxes.entry(mailbox.to_string()); @@ -251,13 +368,24 @@ impl MockImapServer { MockMailbox::new(uid_validity) }); let fresh = mailbox.uid_next == 1 && mailbox.messages.is_empty(); - let uid = mailbox.append(body, flags); + let uid = mailbox.append(body, flags, object_ids); if fresh { state.next_uid_validity += 1; } uid } + /// The object ids the server assigned to `uid` in `mailbox`. + pub fn object_ids(&self, mailbox: &str, uid: u32) -> Option { + let state = lock_state(&self.state); + state.mailboxes.get(mailbox).and_then(|mb| { + mb.messages + .iter() + .find(|m| m.uid == uid) + .map(|m| m.object_ids.clone()) + }) + } + /// Simulate another client removing a message -- the case a forward-only /// high-water-mark sync can never observe. pub fn expunge(&self, mailbox: &str, uid: u32) -> bool { @@ -331,7 +459,7 @@ async fn handle_connection( let st = lock_state(&state); format!( "* OK [CAPABILITY {}] Nuncio mock IMAP ready\r\n", - st.profile.capability_line() + st.capability_line() ) }; socket.write_all(greeting.as_bytes()).await?; @@ -381,11 +509,11 @@ fn dispatch(line: &str, session: &mut Session, st: &mut ServerState) -> Reply { match command.as_str() { "CAPABILITY" => Reply::open(format!( "* CAPABILITY {}\r\n{tag} OK CAPABILITY completed\r\n", - st.profile.capability_line() + st.capability_line() )), "LOGIN" => Reply::open(format!( "{tag} OK [CAPABILITY {}] Logged in\r\n", - st.profile.capability_line() + st.capability_line() )), "ENABLE" => handle_enable(&tag, &rest, session, st), "NOOP" => Reply::open(format!("{tag} OK NOOP completed\r\n")), @@ -499,7 +627,7 @@ fn handle_select( out.push_str(&format!("* VANISHED (EARLIER) {}\r\n", gone.join(","))); } for msg in mb.messages.iter().filter(|m| m.modseq > since) { - out.push_str(&fetch_line(mb, msg, false)); + out.push_str(&fetch_line(mb, msg, FetchItems::none())); } } } @@ -576,7 +704,39 @@ fn format_flags(flags: &BTreeSet) -> String { flags.iter().cloned().collect::>().join(" ") } -fn fetch_line(mb: &MockMailbox, msg: &MockMessage, include_body: bool) -> String { +/// Which optional items a `FETCH` reply should carry. +/// +/// A server returns what was asked for. That is not pedantry here: `EMAILID` +/// is unparseable by the client's own parser, so serving it unasked would kill +/// every connection that ran a typed fetch -- exactly the failure the raw +/// layer exists to route around. +#[derive(Debug, Clone, Copy, Default)] +struct FetchItems { + body: bool, + email_id: bool, + gm_msgid: bool, +} + +impl FetchItems { + /// Read the request off the command line, honouring what this server + /// actually advertised: an item the client asked for without the + /// capability is simply not served. + fn requested(upper: &str, extensions: IdentityExtensions) -> Self { + Self { + body: upper.contains("BODY[]") || upper.contains("RFC822"), + email_id: extensions.objectid && upper.contains("EMAILID"), + gm_msgid: extensions.gmail && upper.contains("X-GM-MSGID"), + } + } + + /// The shape a non-FETCH path (a QRESYNC `SELECT`, a non-silent `STORE`) + /// volunteers: flags and mod-sequence only. + fn none() -> Self { + Self::default() + } +} + +fn fetch_line(mb: &MockMailbox, msg: &MockMessage, items_wanted: FetchItems) -> String { let seq = mb.seq_of(msg.uid); let mut items = format!( "UID {} FLAGS ({}) MODSEQ ({})", @@ -584,7 +744,14 @@ fn fetch_line(mb: &MockMailbox, msg: &MockMessage, include_body: bool) -> String format_flags(&msg.flags), msg.modseq ); - if include_body { + if items_wanted.email_id { + // RFC 8474 section 5.2 parenthesises the value. + items.push_str(&format!(" EMAILID ({})", msg.object_ids.email_id)); + } + if items_wanted.gm_msgid { + items.push_str(&format!(" X-GM-MSGID {}", msg.object_ids.gm_msgid)); + } + if items_wanted.body { items.push_str(&format!(" BODY[] {{{}}}\r\n{}", msg.body.len(), msg.body)); } format!("* {seq} FETCH ({items})\r\n") @@ -598,6 +765,7 @@ fn handle_uid_fetch( st: &mut ServerState, ) -> Reply { let condstore = st.profile.supports_condstore(); + let extensions = st.extensions; let Some(mb) = st.mailboxes.get(selected) else { return Reply::open(format!("{tag} NO mailbox vanished\r\n")); }; @@ -606,7 +774,14 @@ fn handle_uid_fetch( }; let upper = line.to_ascii_uppercase(); - let include_body = upper.contains("BODY[]") || upper.contains("RFC822"); + // An unrecognised FETCH item is a syntax error (RFC 3501 section 6.4.5), so + // a server that never advertised OBJECTID rejects the whole command rather + // than quietly answering without the item. Modelling that is what makes the + // client's capability gate load-bearing instead of decorative. + if upper.contains("EMAILID") && !extensions.objectid { + return Reply::open(format!("{tag} BAD unknown FETCH item EMAILID\r\n")); + } + let items_wanted = FetchItems::requested(&upper, extensions); let changed_since = if condstore { parse_changed_since(&upper) } else { @@ -623,7 +798,7 @@ fn handle_uid_fetch( continue; } } - out.push_str(&fetch_line(mb, msg, include_body)); + out.push_str(&fetch_line(mb, msg, items_wanted)); } // RFC 7162 3.2.6: with CHANGEDSINCE ... VANISHED the server reports @@ -724,7 +899,7 @@ fn handle_uid_store( if !silent { for uid in &touched { if let Some(msg) = mb.messages.iter().find(|m| m.uid == *uid) { - out.push_str(&fetch_line(mb, msg, false)); + out.push_str(&fetch_line(mb, msg, FetchItems::none())); } } } @@ -784,11 +959,14 @@ fn handle_uid_copy( }; let present: Vec = mb.messages.iter().map(|m| m.uid).collect(); let uids = expand_uid_set(spec, &present, mb.uid_next); - let bodies: Vec<(u32, String, BTreeSet)> = mb + // The copies keep the source's object ids. RFC 8474 section 4 makes + // EMAILID a property of the message, so a server that minted a new one + // per mailbox would be reporting two messages where there is one. + let bodies: Vec<(String, BTreeSet, ObjectIds)> = mb .messages .iter() .filter(|m| uids.contains(&m.uid)) - .map(|m| (m.uid, m.body.clone(), m.flags.clone())) + .map(|m| (m.body.clone(), m.flags.clone(), m.object_ids.clone())) .collect(); (uids, mb.uid_validity, bodies) }; @@ -806,9 +984,9 @@ fn handle_uid_copy( let Some(dst) = st.mailboxes.get_mut(&dest) else { return Reply::open(format!("{tag} NO destination vanished\r\n")); }; - for (_, body, flags) in &bodies { + for (body, flags, object_ids) in &bodies { let refs: Vec<&str> = flags.iter().map(String::as_str).collect(); - dest_uids.push(dst.append(body, &refs)); + dest_uids.push(dst.append(body, &refs, object_ids.clone())); } dst.uid_validity }; diff --git a/crates/nuncio-mail/tests/imap_identity_test.rs b/crates/nuncio-mail/tests/imap_identity_test.rs new file mode 100644 index 0000000..06a2f5a --- /dev/null +++ b/crates/nuncio-mail/tests/imap_identity_test.rs @@ -0,0 +1,231 @@ +//! Offline tests for server-assigned message identity over IMAP. +//! +//! The claim under test is the one ADR 0002 rests on: a message has one +//! account-scoped key, and which protocol observed it does not change that key. +//! RFC 8474 exists precisely so a server implementing both IMAP and JMAP +//! reports the same object id through both, so the last test here compares an +//! IMAP-derived key against a JMAP-derived one and requires them to be equal. +//! +//! Everything runs against [`MockImapServer`] over loopback and against a JMAP +//! response parsed from a literal; no test here reaches a network. + +#![allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] + +use nuncio_core::model::IdentitySource; +use nuncio_mail::backend::FolderChanges; +use nuncio_mail::test_server::{IdentityExtensions, MockImapServer, ObjectIds, ServerProfile}; +use nuncio_mail::{JmapEngine, MailBackend}; + +/// Sync one folder through the real engine against the mock. +async fn sync(server: &MockImapServer, account_id: &str, folder: &str) -> FolderChanges { + let engine = nuncio_mail::ImapEngine::with_credentials( + account_id, + server.host(), + server.port(), + nuncio_core::TlsMode::Plain, + "user", + "pass", + ); + engine + .sync_changes(folder, None) + .await + .expect("folder sync succeeds") +} + +#[tokio::test] +async fn an_objectid_server_keys_messages_by_their_emailid() { + // Tier 1. Reaching it at all is the difference between an identity the + // server vouches for and one derived from whatever the message happened to + // contain. + let server = + MockImapServer::start_with_identity(ServerProfile::Qresync, IdentityExtensions::objectid()) + .await + .expect("server starts"); + server.create_mailbox("INBOX"); + let uid = server.append_message("INBOX", "Subject: one\r\n\r\nbody", &[]); + + let changes = sync(&server, "acct-objectid", "INBOX").await; + + assert_eq!(changes.upserts.len(), 1); + assert_eq!( + changes.upserts[0].source, + IdentitySource::EmailId, + "an OBJECTID server must be asked for EMAILID and the answer used" + ); + assert_eq!(changes.upserts[0].placement.remote_id, uid.to_string()); + assert!( + server + .commands_matching("EMAILID") + .iter() + .any(|c| c.to_ascii_uppercase().contains("UID FETCH")), + "the object id must come from a real UID FETCH, got {:?}", + server.commands() + ); +} + +#[tokio::test] +async fn the_same_message_in_a_second_folder_keys_identically() { + // The property a folder-scoped surrogate can never have, and the reason + // tier 1 is worth a separate round trip: two placements, one message. + let server = + MockImapServer::start_with_identity(ServerProfile::Qresync, IdentityExtensions::objectid()) + .await + .expect("server starts"); + server.create_mailbox("INBOX"); + server.create_mailbox("Archive"); + + let shared = ObjectIds { + email_id: "M00000042".to_string(), + gm_msgid: 1_278_455_344_230_334_865, + }; + let body = "Subject: filed\r\n\r\nbody"; + server.append_message_with_object_ids("INBOX", body, &[], shared.clone()); + server.append_message_with_object_ids("Archive", body, &[], shared); + + let inbox = sync(&server, "acct-shared", "INBOX").await; + let archive = sync(&server, "acct-shared", "Archive").await; + + assert_eq!(inbox.upserts.len(), 1); + assert_eq!(archive.upserts.len(), 1); + assert_eq!( + inbox.upserts[0].email.id, archive.upserts[0].email.id, + "one message in two folders must be one identity" + ); + assert_eq!(inbox.upserts[0].source, IdentitySource::EmailId); + assert_eq!(archive.upserts[0].source, IdentitySource::EmailId); + assert_ne!( + inbox.upserts[0].placement, archive.upserts[0].placement, + "the two occupancies are still distinct; only the message is shared" + ); +} + +#[tokio::test] +async fn a_gmail_shaped_server_keys_messages_by_x_gm_msgid() { + // Gmail advertises X-GM-EXT-1 and not OBJECTID, so tier 2 is the best it + // offers -- and it is reachable through the typed client, with no extra + // round trip, because `imap-proto` can parse that attribute. + let server = + MockImapServer::start_with_identity(ServerProfile::Condstore, IdentityExtensions::gmail()) + .await + .expect("server starts"); + server.create_mailbox("INBOX"); + server.append_message("INBOX", "Subject: gmail\r\n\r\nbody", &[]); + + let changes = sync(&server, "acct-gmail", "INBOX").await; + + assert_eq!(changes.upserts.len(), 1); + assert_eq!( + changes.upserts[0].source, + IdentitySource::GmailMsgId, + "X-GM-EXT-1 must lift identity to the Gmail message id" + ); + assert!( + server.commands_matching("EMAILID").is_empty(), + "a server without OBJECTID must never be asked for EMAILID, got {:?}", + server.commands() + ); +} + +#[tokio::test] +async fn a_server_advertising_neither_extension_is_left_exactly_as_it_was() { + // The regression that matters most: capability negotiation must be + // invisible to a server that has neither extension. No new command, and + // the same tier it reached before any of this existed. + for profile in [ + ServerProfile::Qresync, + ServerProfile::Condstore, + ServerProfile::Basic, + ] { + let server = MockImapServer::start(profile).await.expect("server starts"); + server.create_mailbox("INBOX"); + server.append_message("INBOX", "Subject: plain\r\n\r\nbody", &[]); + + let changes = sync(&server, "acct-plain", "INBOX").await; + + assert_eq!(changes.upserts.len(), 1, "{profile:?}"); + assert_eq!( + changes.upserts[0].source, + IdentitySource::Surrogate, + "{profile:?}: with no server id and no octets, the surrogate is the honest answer" + ); + assert!( + server.commands_matching("EMAILID").is_empty(), + "{profile:?}: no EMAILID command may be issued, got {:?}", + server.commands() + ); + assert!( + server.commands_matching("X-GM-MSGID").is_empty(), + "{profile:?}: no X-GM-MSGID item may be requested, got {:?}", + server.commands() + ); + } +} + +#[tokio::test] +async fn an_imap_emailid_and_a_jmap_object_id_produce_the_same_key() { + // The convergence assertion. One account, one message, one server-assigned + // id -- reported as RFC 8474 EMAILID over IMAP and as the Email object id + // over JMAP. If either engine derived its key from a different tier, or + // hashed under a different domain label, these two strings would differ. + const ACCOUNT: &str = "acct-converge"; + const OBJECT_ID: &str = "M00000001"; + + let server = + MockImapServer::start_with_identity(ServerProfile::Qresync, IdentityExtensions::objectid()) + .await + .expect("server starts"); + server.create_mailbox("INBOX"); + server.append_message_with_object_ids( + "INBOX", + "Subject: both\r\n\r\nbody", + &[], + ObjectIds { + email_id: OBJECT_ID.to_string(), + gm_msgid: 1, + }, + ); + let imap = sync(&server, ACCOUNT, "INBOX").await; + + let jmap_response = format!( + r#"{{"methodResponses":[["Email/get",{{"accountId":"{ACCOUNT}", + "state":"s-1","list":[{{"id":"{OBJECT_ID}","subject":"both", + "from":[{{"email":"a@nuncio.mx"}}],"to":[{{"email":"b@nuncio.mx"}}], + "receivedAt":1700001000,"isUnread":true}}]}},"c1"]]}}"# + ); + let (jmap, _state) = JmapEngine::new(ACCOUNT) + .parse_email_get_response(&jmap_response) + .expect("the JMAP response parses"); + + assert_eq!(imap.upserts.len(), 1); + assert_eq!(jmap.len(), 1); + assert_eq!( + imap.upserts[0].source, + IdentitySource::EmailId, + "the IMAP side must have entered at the object-id tier" + ); + assert_eq!( + jmap[0].source, + IdentitySource::EmailId, + "the JMAP side must have entered at the object-id tier" + ); + assert_eq!( + imap.upserts[0].email.id, jmap[0].email.id, + "two engines syncing one account must agree on what the message is" + ); + + // Equality alone would survive both engines moving to a different tier or + // a different domain label together, since they share one derivation. The + // golden pins the actual value: SHA-256 over the length-prefixed + // (tier, account, object id) triple. It is a tripwire, not a spec -- if it + // moves, every stored message_key has moved with it, and + // `IDENTITY_SCHEMA_VERSION` must move in the same commit. + const EXPECTED_KEY: &str = "ba2eb228c6b973c548837e32b84e95cf06b73664b3e350f5dfa30614b2e38bd0"; + assert_eq!( + imap.upserts[0].email.id, EXPECTED_KEY, + "the object-id tier's derivation changed; bump IDENTITY_SCHEMA_VERSION with it" + ); + + // And the two engines still describe *where* it sits in their own terms: + // convergence is a claim about identity, never about placement. + assert_ne!(imap.upserts[0].placement, jmap[0].placement); +} diff --git a/crates/nuncio-mail/tests/imap_raw_wire_test.rs b/crates/nuncio-mail/tests/imap_raw_wire_test.rs index 97ed5f2..8638380 100644 --- a/crates/nuncio-mail/tests/imap_raw_wire_test.rs +++ b/crates/nuncio-mail/tests/imap_raw_wire_test.rs @@ -13,7 +13,7 @@ use nuncio_core::model::PlacementKey; use nuncio_mail::imap_raw; -use nuncio_mail::test_server::{MockImapServer, ServerProfile}; +use nuncio_mail::test_server::{IdentityExtensions, MockImapServer, ObjectIds, ServerProfile}; /// Connect the real client to the mock server and authenticate. async fn login( @@ -301,6 +301,89 @@ async fn a_qresync_select_without_enable_is_refused() { ); } +#[tokio::test] +async fn object_ids_are_read_off_the_transport_and_the_session_survives_it() { + // The whole reason this bypasses the client: `imap-proto` has no parser arm + // for EMAILID, and `async-imap` turns a parse error into a fatal I/O error + // that latches the stream closed. So the test that matters is not only that + // the ids come back, but that the *typed* client still works afterwards -- + // proving the raw exchange left the connection and the client's read buffer + // in a state the next command can use. + let server = + MockImapServer::start_with_identity(ServerProfile::Qresync, IdentityExtensions::objectid()) + .await + .expect("server starts"); + server.create_mailbox("INBOX"); + let first = server.append_message_with_object_ids( + "INBOX", + "Subject: one\r\n\r\nx", + &[], + ObjectIds { + email_id: "M-first".to_string(), + gm_msgid: 11, + }, + ); + let second = server.append_message_with_object_ids( + "INBOX", + "Subject: two\r\n\r\nx", + &[], + ObjectIds { + email_id: "M-second".to_string(), + gm_msgid: 12, + }, + ); + + let mut session = login(&server).await; + imap_raw::run_collected(&mut session, "SELECT INBOX") + .await + .expect("select"); + + let ids = imap_raw::uid_fetch_email_ids(&mut session, &format!("{first},{second}")) + .await + .expect("the raw EMAILID fetch succeeds"); + + assert_eq!(ids.get(&first).map(String::as_str), Some("M-first")); + assert_eq!(ids.get(&second).map(String::as_str), Some("M-second")); + + // The session is still alive: a typed command after the raw exchange must + // still see the mailbox. + let after = imap_raw::run_collected(&mut session, "UID FETCH 1:* (UID)") + .await + .expect("the session survives the raw exchange"); + assert_eq!(after.fetched_uids, vec![first, second]); +} + +#[tokio::test] +async fn a_refused_object_id_fetch_costs_precision_not_the_sync() { + // A server that rejects the item leaves the message identifiable by a + // weaker tier. Erroring here would fail an entire folder sync over an + // identity *upgrade*, which is the wrong trade in both directions. + let server = MockImapServer::start(ServerProfile::Qresync) + .await + .expect("server starts"); + server.create_mailbox("INBOX"); + let uid = server.append_message("INBOX", "Subject: one\r\n\r\nx", &[]); + + let mut session = login(&server).await; + imap_raw::run_collected(&mut session, "SELECT INBOX") + .await + .expect("select"); + + let ids = imap_raw::uid_fetch_email_ids(&mut session, &uid.to_string()) + .await + .expect("a BAD answer is not a transport failure"); + assert!( + ids.is_empty(), + "nothing was learned, and that is the answer" + ); + + // And the connection is still usable, which a `BAD` must not change. + let after = imap_raw::run_collected(&mut session, "UID FETCH 1:* (UID)") + .await + .expect("the session survives a refusal"); + assert_eq!(after.fetched_uids, vec![uid]); +} + /// Drive a real folder sync against the mock and return what it observed. async fn sync_folder( server: &MockImapServer, diff --git a/crates/nuncio-store/src/db.rs b/crates/nuncio-store/src/db.rs index 24ced58..6d8c5b0 100644 --- a/crates/nuncio-store/src/db.rs +++ b/crates/nuncio-store/src/db.rs @@ -628,7 +628,17 @@ impl DatabaseEngine { /// /// Bump this only for a change of that kind, and only together with the /// reset it implies (see [`DatabaseEngine::reconcile_schema_version`]). - pub const IDENTITY_SCHEMA_VERSION: i64 = 2; + /// + /// Generation 3 is where the IMAP engine began asking for RFC 8474 + /// `EMAILID` and `X-GM-MSGID`. Every message a capable server holds now + /// derives its key from a stronger identity tier, so no key stored under + /// generation 2 still names the message it named. Letting that happen a row + /// at a time -- as each resync repointed a placement -- would work, but the + /// repoint reap is a safety net against stranded plaintext, not a migration + /// mechanism, and it would leave the store in two identity models for as + /// long as any folder went unsynced. Rebuilding once is the honest form of + /// the same change. + pub const IDENTITY_SCHEMA_VERSION: i64 = 3; /// Reconcile the on-disk identity-schema generation with this binary's. /// diff --git a/docs/adr/0002-multi-engine-sync-model.md b/docs/adr/0002-multi-engine-sync-model.md index 2e45fa1..a874edd 100644 --- a/docs/adr/0002-multi-engine-sync-model.md +++ b/docs/adr/0002-multi-engine-sync-model.md @@ -175,6 +175,24 @@ sound without a sequence number and resumption. Precedence: `EMAILID` (RFC 8474 OBJECTID) → `X-GM-MSGID` → `(account_id, normalized Message-ID, content_hash)` → `(account, folder, uidvalidity, uid)`. +**Both engines negotiate the top two tiers, so keys converge exactly where the +server publishes an object id.** The IMAP engine requests `EMAILID` from a +server advertising `OBJECTID` and `X-GM-MSGID` from one advertising +`X-GM-EXT-1`; JMAP's Email object id is the same identifier RFC 8474 exposes to +IMAP. One message synced over both protocols therefore lands on one key, which +is what makes the multi-engine model's identity claim true across protocols and +not only within one. + +Be precise about where it stops. Against a server advertising neither +extension, IMAP reaches only the Message-ID+content tier or the folder-scoped +surrogate, while a JMAP engine on the same account still enters at the top tier +from a server-assigned id — those key spaces are disjoint and cross-engine +comparison means nothing there. Object ids are unique within one account on one +server (RFC 8474 §4), never globally, which is why every tier is +account-scoped. And `EMAILID` cannot be requested through the typed IMAP client +at all: `imap-proto` has no parser arm for the attribute and a parse failure is +fatal to the connection, so it is read through the raw command layer instead. + Bodies are keyed by a content hash of the full RFC822 octets, first-write-wins. **A Message-ID hash alone must not be the key**: Message-IDs are public, echoed in every `References:` header, and forgeable, so with `INSERT OR REPLACE` a @@ -238,7 +256,11 @@ work is chunked per folder so it yields to user actions. - **`async-imap` 0.11.3 cannot express any of this**: no `ENABLE`, its `SELECT` parser drops `VANISHED`, and `check_status_ok` discards the tagged response code where `COPYUID` and `[MODIFIED …]` live. A fork-versus-hand-rolled - decision gates the protocol work. + decision gates the protocol work. `EMAILID` is worse than discarded: + `imap-proto`'s `msg_att` is a closed alternation with no catch-all, and the + decoder turns the resulting parse error into a fatal I/O error that latches + the connection closed — so the object id must be read off the transport + underneath the client, never through it. ## Alternatives considered -- 2.54.0