feat(sync): incremental IMAP sync with progress + per-item FETCH timeout #231

2026-07-30T02:19:18Z

KofTwentyTwo commented

2026-07-30 02:19:18 +00:00

(Migrated from github.com)

Closes #195 (also fixes #184, #183). Milestone M5. NON-proto — nuncio.proto is untouched; incrementality is entirely internal to the daemon/engine.

The bug this kills

ImapEngine::sync_messages ignored since_state, always issued UID FETCH 1:* (a full re-fetch every sync), and returned a fabricated state string derived from emails.len(). The daemon hardcoded sync_messages(&folder.id, None) and discarded the returned state — there was nowhere to persist a checkpoint. This PR makes the second sync of an account/folder issue a genuinely narrower FETCH derived from a real UIDNEXT checkpoint.

Layers

store — new folder_sync_state(account_id, folder_id, state) table + get_folder_sync_state / save_folder_sync_state, mirroring existing single-row CRUD. Round-trip test proves scoping + overwrite.
mail (engine) — after SELECT, capture the mailbox's real UIDNEXT; resolve_fetch_range turns the stored checkpoint into {uidnext}:* (incremental) vs 1:* (first sync / corrupt checkpoint, corrupt flagged distinctly via a typed FetchRange + a tracing::warn!). The returned checkpoint is derived from real server state (UIDNEXT, else max-UID-seen+1), never emails.len(). Each per-item FETCH read is wrapped in tokio::time::timeout(FETCH_ITEM_TIMEOUT) (named const, 60s, documented), surfacing a new MailError::FetchStalled instead of hanging.
mail (mock) — MockMailBackend now captures every since_state arg (since_state_calls()), lets a test set the returned checkpoint, and returns an empty (narrower) result when given a non-None checkpoint.
core — new CoreEvent::SyncProgress { account_id, folder_id, fetched, total }, mirroring BatchFilterProgress.
daemon (sync) — fetch_and_persist reads the folder's checkpoint, threads it into sync_messages, persists the returned checkpoint only after that folder's messages land, and publishes one SyncProgress per folder. map_core_event drops SyncProgress from the gRPC subscribe stream (no wire-contract case; proto unchanged).

How genuine incrementality is proven

crates/nunciod/src/sync.rs → sync_with_backend_persists_and_threads_the_folder_checkpoint_across_syncs: syncs the same account/folder twice via MockMailBackend and asserts since_state_calls() == [None, Some("uidnext-4242")] — the daemon persisted the first sync's returned state and fed it back into the second. Second sync fetches 0 (narrower).
crates/nuncio-mail/src/imap.rs → incremental_sync_narrows_the_fetch_range_on_the_second_pass: drives a real async_imap::Session over an in-memory tokio::io::duplex pair with a scripted IMAP responder; asserts the literal FETCH range is 1:* on the first sync and 105:* (never 1:*) on the second.

How the stall is bounded

a_never_responding_fetch_surfaces_fetchstalled_without_hanging (#[tokio::test(start_paused = true)]): the scripted server answers LOGIN/SELECT then goes silent on FETCH; the paused clock fast-forwards the 60s timeout so MailError::FetchStalled is returned in negligible wall-clock time (asserted < 5s via std::time::Instant). test-util added as a nuncio-mail dev-dependency for paused-clock tests.

Offline-only

No live IMAP anywhere: MockMailBackend + in-memory tokio::io::duplex streams fed literal scripted IMAP responses. Prior art 16d0a11 was treated as reference only (it did timeout+progress but NOT real incrementality) and re-implemented fresh.

Gate output (all three commands green, 0 ignored)

cargo fmt --all -- --check      # clean
cargo check-all                 # clippy --all-targets --workspace -D warnings: Finished, 0 warnings
cargo test-all                  # all suites ok; 0 failed; 0 ignored

Selected suite counts:

nuncio-store: 57 passed
nuncio-mail (lib): 51 passed; wiremock_jmap: 3
nuncio-core: 52 passed
nunciod (lib): 115 passed; e2e (spine/grpc_auth/accounts/etc.): all passed
workspace total: 0 failed, 0 ignored

Out of scope / notes

ImapEngine::sync_folder_messages's credential-less fixture path (a pre-existing test scaffold returning a canned email) was left in place; its returned checkpoint is now internally consistent ("101", one past the fixture UID) rather than emails.len()-derived. Not fabricated production data — only the unit-test fixture branch. No new issue filed.

Closes #195 (also fixes #184, #183). Milestone M5. **NON-proto** — `nuncio.proto` is untouched; incrementality is entirely internal to the daemon/engine. ## The bug this kills `ImapEngine::sync_messages` ignored `since_state`, always issued `UID FETCH 1:*` (a full re-fetch every sync), and returned a fabricated state string derived from `emails.len()`. The daemon hardcoded `sync_messages(&folder.id, None)` and discarded the returned state — there was nowhere to persist a checkpoint. This PR makes the **second** sync of an account/folder issue a genuinely narrower FETCH derived from a real UIDNEXT checkpoint. ## Layers - **store** — new `folder_sync_state(account_id, folder_id, state)` table + `get_folder_sync_state` / `save_folder_sync_state`, mirroring existing single-row CRUD. Round-trip test proves scoping + overwrite. - **mail (engine)** — after `SELECT`, capture the mailbox's real `UIDNEXT`; `resolve_fetch_range` turns the stored checkpoint into `{uidnext}:*` (incremental) vs `1:*` (first sync / corrupt checkpoint, corrupt flagged distinctly via a typed `FetchRange` + a `tracing::warn!`). The returned checkpoint is derived from real server state (UIDNEXT, else max-UID-seen+1), never `emails.len()`. Each per-item FETCH read is wrapped in `tokio::time::timeout(FETCH_ITEM_TIMEOUT)` (named const, 60s, documented), surfacing a new `MailError::FetchStalled` instead of hanging. - **mail (mock)** — `MockMailBackend` now captures every `since_state` arg (`since_state_calls()`), lets a test set the returned checkpoint, and returns an empty (narrower) result when given a non-None checkpoint. - **core** — new `CoreEvent::SyncProgress { account_id, folder_id, fetched, total }`, mirroring `BatchFilterProgress`. - **daemon (sync)** — `fetch_and_persist` reads the folder's checkpoint, threads it into `sync_messages`, persists the returned checkpoint **only after** that folder's messages land, and publishes one `SyncProgress` per folder. `map_core_event` drops `SyncProgress` from the gRPC subscribe stream (no wire-contract case; proto unchanged). ## How genuine incrementality is proven - `crates/nunciod/src/sync.rs` → `sync_with_backend_persists_and_threads_the_folder_checkpoint_across_syncs`: syncs the same account/folder twice via `MockMailBackend` and asserts `since_state_calls() == [None, Some("uidnext-4242")]` — the daemon persisted the first sync's returned state and fed it back into the second. Second sync fetches 0 (narrower). - `crates/nuncio-mail/src/imap.rs` → `incremental_sync_narrows_the_fetch_range_on_the_second_pass`: drives a real `async_imap::Session` over an in-memory `tokio::io::duplex` pair with a scripted IMAP responder; asserts the literal FETCH range is `1:*` on the first sync and `105:*` (never `1:*`) on the second. ## How the stall is bounded `a_never_responding_fetch_surfaces_fetchstalled_without_hanging` (`#[tokio::test(start_paused = true)]`): the scripted server answers LOGIN/SELECT then goes silent on FETCH; the paused clock fast-forwards the 60s timeout so `MailError::FetchStalled` is returned in negligible **wall-clock** time (asserted `< 5s` via `std::time::Instant`). `test-util` added as a nuncio-mail dev-dependency for paused-clock tests. ## Offline-only No live IMAP anywhere: `MockMailBackend` + in-memory `tokio::io::duplex` streams fed literal scripted IMAP responses. Prior art `16d0a11` was treated as reference only (it did timeout+progress but NOT real incrementality) and re-implemented fresh. ## Gate output (all three commands green, 0 ignored) ``` cargo fmt --all -- --check # clean cargo check-all # clippy --all-targets --workspace -D warnings: Finished, 0 warnings cargo test-all # all suites ok; 0 failed; 0 ignored ``` Selected suite counts: - nuncio-store: 57 passed - nuncio-mail (lib): 51 passed; wiremock_jmap: 3 - nuncio-core: 52 passed - nunciod (lib): 115 passed; e2e (spine/grpc_auth/accounts/etc.): all passed - workspace total: 0 failed, 0 ignored ## Out of scope / notes - `ImapEngine::sync_folder_messages`'s credential-less fixture path (a pre-existing test scaffold returning a canned email) was left in place; its returned checkpoint is now internally consistent (`"101"`, one past the fixture UID) rather than `emails.len()`-derived. Not fabricated production data — only the unit-test fixture branch. No new issue filed.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#231