feat(jmap): enumerate changes so deleted mail stops haunting the store #414

2026-08-14T22:50:31Z

KofTwentyTwo commented

2026-08-14 22:50:31 +00:00

(Migrated from github.com)

What this fixes

CollectionSync::enumerate_changes expresses absence through removals +
present. IMAP implements it across a four-rung ladder; JMAP returned
removals: [] and present: None unconditionally, so nothing was ever
reconciled away and every JMAP account accumulated ghosts indefinitely.

JmapEngine::sync_changes now runs Email/changes (RFC 8621 §4.2 over the
RFC 8620 §5.2 core method) and maps it onto FolderChanges.

What now converges for JMAP

Deletes. destroyed ids become PlacementKey removals, addressed exactly
as the occupancies this engine stores are, so the sync path deletes those
placements and nothing else.
Moves out of a folder. Email/changes is account-wide (§4.2 has no
mailbox filter), so Email/get now also requests mailboxIds. A changed
message whose mailboxes no longer include the folder being synced loses its
occupancy here instead of lingering; one whose mailboxes name a different
folder is not placed here at all. The message itself survives as long as
another placement does — delete_placements decides that, not this.
Paging. hasMoreChanges: true is followed to convergence, accumulating
across pages. Bounded, and a server whose newState does not advance is an
honest error rather than an infinite loop.
Stale state. cannotCalculateChanges is an outcome, not a failure: the
pass re-enumerates the folder and only then reports present: Some(...) —
it is the one pass that genuinely looked at the whole mailbox.
Checkpoints. newState round-trips into next_state and is sent back as
the following pass's sinceState. It is scheme-tagged (v1:jmapstate:{state})
like the IMAP checkpoints, because a JMAP state is an opaque server token with
no self-describing shape. An untagged or unknown token re-enumerates rather
than being handed to a server that never issued it — over-fetching is
recoverable, misreading a token is not.

An ordinary incremental pass still reports present: None. It never enumerated
the folder, and claiming otherwise would delete every message that simply did
not change. No present-set is fabricated to make types line up.

What still does not

Flag/keyword-only changes elsewhere. A pass fetches every account-wide
changed id and discards the ones outside this folder. That is correct but
chatty on busy accounts; narrowing it would need Email/query + /queryChanges
per mailbox.
Push. Nothing here subscribes to the EventSource / StateChange push
channel; sync is still poll-driven.
Email/get still carries no octets, so content_hash stays None and
identity relies on the account-stable Email object id (tier 1, the same
guarantee RFC 8474 EMAILID gives). Unchanged by this PR.

Note on the adjacent hardcode (#339)

parse_email_get_response hardcoded folder_id = "inbox", mislabelling every
non-inbox mailbox. This was load-bearing for the fix, not adjacent to it:
removals are keyed by (account, folder, uid_validity, remote_id), and the
daemon syncs folders by their JMAP mailbox id (mb-inbox), so a removal keyed
mb-inbox could never match a placement stored as inbox. Every removal would
have been inert.

The production path therefore now labels placements with the mailbox actually
synced. The public parse_email_get_response helper keeps its fixed-folder
signature (it delegates with "inbox"); the folder-aware parse is what
sync_changes uses. Two existing assertions that encoded the old mislabelling
were updated. #339 can likely be closed or re-scoped — flagging rather than
claiming it.

Testing

crates/nuncio-mail/tests/wiremock_jmap_test.rs (all mocked, no live network):

destroyed ids become exactly those removals, and nothing the pass placed
hasMoreChanges: true loops and accumulates across two pages
cannotCalculateChanges falls back to full enumeration and only then reports
present: Some(...)
an incremental pass reports present: None
newState round-trips into next_state and is accepted verbatim on the next
pass (the second page's mock matches only that exact sinceState, so a token
that failed to round-trip would match nothing)
an untagged legacy checkpoint re-enumerates, with no Email/changes mock
mounted so any attempt would fail the test
a message moved to another mailbox is reconciled away, not placed

Plus unit tests for the checkpoint scheme round-trip, unknown-scheme rejection,
cannotCalculateChanges vs. a genuine method error, and mailbox-membership
filtering.

Mutation-verified: breaking the paging loop, dropping removals, letting the
incremental pass claim present, and turning cannotCalculateChanges into an
error each failed exactly the intended test and nothing else.

Gate: cargo fmt --all -- --check, cargo check-all, cargo test-all all
green; RUST_TEST_THREADS=2 cargo test-all run three times with zero FAILED
lines each.

Closes #408

## What this fixes `CollectionSync::enumerate_changes` expresses absence through `removals` + `present`. IMAP implements it across a four-rung ladder; JMAP returned `removals: []` and `present: None` unconditionally, so nothing was ever reconciled away and **every JMAP account accumulated ghosts indefinitely**. `JmapEngine::sync_changes` now runs `Email/changes` (RFC 8621 §4.2 over the RFC 8620 §5.2 core method) and maps it onto `FolderChanges`. ## What now converges for JMAP - **Deletes.** `destroyed` ids become `PlacementKey` removals, addressed exactly as the occupancies this engine stores are, so the sync path deletes those placements and nothing else. - **Moves out of a folder.** `Email/changes` is account-wide (§4.2 has no mailbox filter), so `Email/get` now also requests `mailboxIds`. A changed message whose mailboxes no longer include the folder being synced loses its occupancy here instead of lingering; one whose mailboxes name a *different* folder is not placed here at all. The message itself survives as long as another placement does — `delete_placements` decides that, not this. - **Paging.** `hasMoreChanges: true` is followed to convergence, accumulating across pages. Bounded, and a server whose `newState` does not advance is an honest error rather than an infinite loop. - **Stale state.** `cannotCalculateChanges` is an outcome, not a failure: the pass re-enumerates the folder and *only then* reports `present: Some(...)` — it is the one pass that genuinely looked at the whole mailbox. - **Checkpoints.** `newState` round-trips into `next_state` and is sent back as the following pass's `sinceState`. It is scheme-tagged (`v1:jmapstate:{state}`) like the IMAP checkpoints, because a JMAP state is an opaque server token with no self-describing shape. An untagged or unknown token re-enumerates rather than being handed to a server that never issued it — over-fetching is recoverable, misreading a token is not. An ordinary incremental pass still reports `present: None`. It never enumerated the folder, and claiming otherwise would delete every message that simply did not change. No present-set is fabricated to make types line up. ## What still does not - **Flag/keyword-only changes elsewhere.** A pass fetches every account-wide changed id and discards the ones outside this folder. That is correct but chatty on busy accounts; narrowing it would need `Email/query` + `/queryChanges` per mailbox. - **Push.** Nothing here subscribes to the EventSource / `StateChange` push channel; sync is still poll-driven. - **`Email/get` still carries no octets**, so `content_hash` stays `None` and identity relies on the account-stable Email object id (tier 1, the same guarantee RFC 8474 `EMAILID` gives). Unchanged by this PR. ## Note on the adjacent hardcode (#339) `parse_email_get_response` hardcoded `folder_id = "inbox"`, mislabelling every non-inbox mailbox. This was **load-bearing for the fix, not adjacent to it**: removals are keyed by `(account, folder, uid_validity, remote_id)`, and the daemon syncs folders by their JMAP mailbox id (`mb-inbox`), so a removal keyed `mb-inbox` could never match a placement stored as `inbox`. Every removal would have been inert. The production path therefore now labels placements with the mailbox actually synced. The public `parse_email_get_response` helper keeps its fixed-folder signature (it delegates with `"inbox"`); the folder-aware parse is what `sync_changes` uses. Two existing assertions that encoded the old mislabelling were updated. #339 can likely be closed or re-scoped — flagging rather than claiming it. ## Testing `crates/nuncio-mail/tests/wiremock_jmap_test.rs` (all mocked, no live network): - `destroyed` ids become exactly those removals, and nothing the pass placed - `hasMoreChanges: true` loops and accumulates across two pages - `cannotCalculateChanges` falls back to full enumeration and only then reports `present: Some(...)` - an incremental pass reports `present: None` - `newState` round-trips into `next_state` and is accepted verbatim on the next pass (the second page's mock matches only that exact `sinceState`, so a token that failed to round-trip would match nothing) - an untagged legacy checkpoint re-enumerates, with no `Email/changes` mock mounted so any attempt would fail the test - a message moved to another mailbox is reconciled away, not placed Plus unit tests for the checkpoint scheme round-trip, unknown-scheme rejection, `cannotCalculateChanges` vs. a genuine method error, and mailbox-membership filtering. Mutation-verified: breaking the paging loop, dropping removals, letting the incremental pass claim `present`, and turning `cannotCalculateChanges` into an error each failed exactly the intended test and nothing else. Gate: `cargo fmt --all -- --check`, `cargo check-all`, `cargo test-all` all green; `RUST_TEST_THREADS=2 cargo test-all` run three times with zero `FAILED` lines each. Closes #408

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#414