feat(mail): add a raw IMAP layer for ENABLE, tagged codes, and VANISHED #401

2026-08-10T21:09:03Z

KofTwentyTwo commented

2026-08-10 21:09:03 +00:00

(Migrated from github.com)

Summary of Changes

Resolves the dependency gate blocking #390 (change enumeration) and #391 (verified mutations).

Based on story/imap-mock-harness (PR #384), not dev — these tests drive the real client against that harness. GitHub will retarget this to dev automatically when #384 merges. Review the two-commit diff, or read only the second commit.

The decision: hand-roll, not fork

#388 asked for an explicit fork-vs-hand-roll call. Hand-roll, and the investigation is worth recording:

Needed	Typed API	Reachable?
`ENABLE`	not implemented	✅ `run_command`
`SELECT (QRESYNC …)`	no method; `select()` drops `VANISHED`	✅ `Response::Vanished` is parsed
`COPYUID`	`uid_mv` returns `Result<()>`, code discarded	✅ `ResponseCode::CopyUid` is parsed
`[MODIFIED …]`	code discarded	⚠️ see below

Session::run_command and read_response are public and imap-proto already parses what matters, so the gap is just a collector that reads to the tagged response and keeps what arrived. ~100 lines, against maintaining a fork for the same result.

The one real gap, and why the workaround is defensible

imap-proto 0.16.7 has no ResponseCode::Modified variant at all — a correction to the earlier review, which assumed the wire layer was complete. ResponseData's raw bytes are also unreachable (private module, no re-export), so it cannot be read from the parse tree.

But resp_text is opt(resp_text_code) followed by free text, so an unrecognised code is not a parse error — the whole [MODIFIED 7,9] survives in the information string. That is where it is recovered from.

Parsing a response code out of a human-readable field is not how this should work forever, and upstreaming the variant is the real fix. It is deterministic, it is pinned by a wire test against a server emitting the real syntax, and the alternative is no conflict detection at all.

Two subtleties the collector exists to get right

COPYUID arrives in an untagged OK for MOVE — RFC 6851 §4.3 advises exactly that. A collector reading only the tagged response misses it on every conforming server, then has no proof the move happened.
UID EXPUNGE answers with VANISHED, not EXPUNGE, once QRESYNC is enabled (§4.4). Counting EXPUNGE alone reports failure on every successful delete. Both are gathered.

Absent COPYUID is deliberately not failure. A UID set matching nothing succeeds having done nothing (RFC 3501 §6.4.8), and a conforming server may omit it for a UIDNOTSTICKY destination — so absence means unknown, which is what #391's third outcome is for.

Type of Change

Bug fix (non-breaking change fixing an issue)
New feature (non-breaking change adding functionality)
Breaking change
Documentation update

Additive. No existing call path changes yet — #390 and #391 are the consumers.

Related Issues

Refs #388. Unblocks #390, #391. Design: ADR 0002 (PR #385).

Quality Verification Checklist

I have verified code formatting (cargo fmt --all -- --check)
I have verified clippy lints (cargo clippy --all-targets --workspace -- -D warnings), plus --features test-support
I have executed the full test suite (cargo test --workspace) and all tests pass (614 passed, 0 failed)
Business logic is implemented in headless engine crates, keeping UI presentation shells skinny
No secrets, keys, or credentials are added to tracked files

Notes for review

7 wire tests drive the real async-imap client over real TCP against the mock, because the claim is that these responses survive the round trip — a unit test over a hand-built struct would only assert on data this crate produced. They cover: VANISHED recovered from a QRESYNC SELECT; a losing conditional store reporting MODIFIED and leaving the flag unapplied; a winning one reporting no conflict; COPYUID from an untagged OK; an already-moved UID reporting OK with no COPYUID; expunge provable on both QRESYNC and plain servers; QRESYNC refused without ENABLE.
imap-proto is reached through async-imap's re-export rather than a direct dependency, so the parser types cannot version-skew against the client using them.
Mock server bug fixed: parse_flag_list took the first parenthesised group, so UID STORE 1 (UNCHANGEDSINCE 2) +FLAGS.SILENT (\\Flagged) applied UNCHANGEDSINCE and 2 as flag names. The flag list is always the final argument. Caught by the winning-store test.
A self dev-dependency enables test-support for this crate's own tests/ targets; production builds still never compile the harness.
enable() deliberately does not confirm which capabilities were enabled — the untagged * ENABLED line has no imap-proto representation, and confirming it is unnecessary since RFC 7162 §3.2.3 makes a successful QRESYNC SELECT its own proof.
No .proto change.

## Summary of Changes Resolves the dependency gate blocking #390 (change enumeration) and #391 (verified mutations). > **Based on `story/imap-mock-harness` (PR #384), not `dev`** — these tests drive the real client against that harness. GitHub will retarget this to `dev` automatically when #384 merges. Review the two-commit diff, or read only the second commit. ### The decision: hand-roll, not fork #388 asked for an explicit fork-vs-hand-roll call. **Hand-roll**, and the investigation is worth recording: | Needed | Typed API | Reachable? | | --- | --- | --- | | `ENABLE` | not implemented | ✅ `run_command` | | `SELECT (QRESYNC …)` | no method; `select()` drops `VANISHED` | ✅ `Response::Vanished` is parsed | | `COPYUID` | `uid_mv` returns `Result<()>`, code discarded | ✅ `ResponseCode::CopyUid` is parsed | | `[MODIFIED …]` | code discarded | ⚠️ see below | `Session::run_command` and `read_response` are public and `imap-proto` already parses what matters, so the gap is just a collector that reads to the tagged response and keeps what arrived. ~100 lines, against maintaining a fork for the same result. ### The one real gap, and why the workaround is defensible **`imap-proto` 0.16.7 has no `ResponseCode::Modified` variant at all** — a correction to the earlier review, which assumed the wire layer was complete. `ResponseData`'s raw bytes are also unreachable (private module, no re-export), so it cannot be read from the parse tree. But `resp_text` is `opt(resp_text_code)` followed by free text, so an unrecognised code **is not a parse error** — the whole `[MODIFIED 7,9]` survives in the `information` string. That is where it is recovered from. Parsing a response code out of a human-readable field is not how this should work forever, and upstreaming the variant is the real fix. It is deterministic, it is pinned by a wire test against a server emitting the real syntax, and the alternative is **no conflict detection at all**. ### Two subtleties the collector exists to get right - **`COPYUID` arrives in an *untagged* `OK` for `MOVE`** — RFC 6851 §4.3 advises exactly that. A collector reading only the tagged response misses it on every conforming server, then has no proof the move happened. - **`UID EXPUNGE` answers with `VANISHED`, not `EXPUNGE`, once QRESYNC is enabled** (§4.4). Counting `EXPUNGE` alone reports failure on every successful delete. Both are gathered. **Absent `COPYUID` is deliberately not failure.** A UID set matching nothing succeeds having done nothing (RFC 3501 §6.4.8), and a conforming server may omit it for a `UIDNOTSTICKY` destination — so absence means *unknown*, which is what #391's third outcome is for. ## Type of Change - [ ] Bug fix (non-breaking change fixing an issue) - [x] New feature (non-breaking change adding functionality) - [ ] Breaking change - [ ] Documentation update Additive. No existing call path changes yet — #390 and #391 are the consumers. ## Related Issues Refs #388. Unblocks #390, #391. Design: [ADR 0002](docs/adr/0002-multi-engine-sync-model.md) (PR #385). ## Quality Verification Checklist - [x] I have verified code formatting (`cargo fmt --all -- --check`) - [x] I have verified clippy lints (`cargo clippy --all-targets --workspace -- -D warnings`), plus `--features test-support` - [x] I have executed the full test suite (`cargo test --workspace`) and all tests pass (**614 passed, 0 failed**) - [x] Business logic is implemented in headless engine crates, keeping UI presentation shells skinny - [x] No secrets, keys, or credentials are added to tracked files ## Notes for review - **7 wire tests** drive the real `async-imap` client over real TCP against the mock, because the claim is that these responses survive the round trip — a unit test over a hand-built struct would only assert on data this crate produced. They cover: VANISHED recovered from a QRESYNC SELECT; a losing conditional store reporting MODIFIED *and* leaving the flag unapplied; a winning one reporting no conflict; COPYUID from an untagged OK; an already-moved UID reporting OK with **no** COPYUID; expunge provable on both QRESYNC and plain servers; QRESYNC refused without ENABLE. - `imap-proto` is reached through `async-imap`'s re-export rather than a direct dependency, so the parser types cannot version-skew against the client using them. - **Mock server bug fixed:** `parse_flag_list` took the *first* parenthesised group, so `UID STORE 1 (UNCHANGEDSINCE 2) +FLAGS.SILENT (\\Flagged)` applied `UNCHANGEDSINCE` and `2` as flag names. The flag list is always the final argument. Caught by the winning-store test. - A self dev-dependency enables `test-support` for this crate's own `tests/` targets; production builds still never compile the harness. - `enable()` deliberately does not confirm *which* capabilities were enabled — the untagged `* ENABLED` line has no `imap-proto` representation, and confirming it is unnecessary since RFC 7162 §3.2.3 makes a successful QRESYNC SELECT its own proof. - No `.proto` change.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#401