feat(mail): report what a mutation proved, not that it was accepted #403

2026-08-10T22:06:18Z

KofTwentyTwo commented

2026-08-10 22:06:18 +00:00

(Migrated from github.com)

Summary of Changes

Closes the "reported completed, did nothing" hole.

Stacked on #402 → #401 → #384. Review the final commit; GitHub retargets as the stack lands.

The defect

A mutation reported success whenever the server answered OK. IMAP answers OK for a UID command that matched nothing (RFC 3501 §6.4.8) — so a mutation that lost a race (message already moved or deleted by another client) was recorded completed having done nothing. Intent silently dropped, outbox row marked done.

Why two states could not express it

"Succeeded" and "failed" leave out the case that actually occurs: the server accepted the command and its response does not establish what happened. Collapsing that into either neighbour is the bug.

MutationOutcome::Applied { token } | Conflict { observed } | Unknown { reason }

Outcome	Requires
`Applied`	positive proof — `COPYUID` naming destination UIDs, a removal report, an echoed `FETCH`
`Conflict`	positive proof — `[MODIFIED …]`. Means changed by someone else, never gone
`Unknown`	everything else

Unknown is the honest reading of an absent COPYUID: only SHOULD for MOVE (RFC 6851 §4.3), legitimately omitted for a UIDNOTSTICKY destination, and indistinguishable from a UID set that matched nothing.

How the outbox treats each

Unknown retries. The asymmetry decides it: a mutation that did apply and is re-attempted is a no-op; one that did not and is recorded complete is unrecoverable.

Conflict is terminal and visible (conflicted status), not retried. The intent was formed against state that no longer exists, so re-issuing it would either do nothing or overwrite someone else's change.

Delete needed the collecting layer

A QRESYNC-enabled server reports VANISHED, not EXPUNGE (RFC 6851 §4.4), and the typed expunge stream only yields the latter — reading it alone reports nothing removed on every successful delete.

Type of Change

Bug fix (non-breaking change fixing an issue)
Breaking change (fix or feature causing existing functionality to change)
New feature
Documentation update

MailBackend::apply_mutation returns MutationOutcome instead of (). Internal; no .proto change.

Related Issues

Refs #391. Design: ADR 0002 §3 (PR #385). Depends on #401.

Quality Verification Checklist

cargo fmt --all -- --check
cargo clippy --all-targets --workspace -- -D warnings
cargo test --workspace — 627 passed, 0 failed
Business logic in headless engine crates
No secrets, keys, or credentials added

Notes for review

3 new wire tests against a real server: a move that lost a race is Unknown (and the destination is genuinely empty); a confirmed delete is Applied; a delete of an absent UID is Unknown with the real message untouched.
uid_store_flags was deleted — superseded by the collecting layer, which can see MODIFIED.
JMAP reports Applied directly. Email/set names the ids it updated or destroyed, so there is no COPYUID-style ambiguity: the id is stable and the server either lists it or does not.
Not in this PR, stated plainly: #391 also scoped wiring the dead calculate_backoff_ms. Doing it properly needs a next_retry_at column and a query filter — a schema change that should sequence against the monitor plan's outbox migration. Half-wiring it (retrying on the fixed 5s tick and calling it backoff) would be worse than leaving it visibly dead. A 25-second blip still permanently fails every queued mutation; that regression is unchanged by this PR and needs its own change.
Also deferred from #391: the durable conflicts table and ListConflicts/ResolveConflict RPCs. Conflicts are currently recorded as a terminal conflicted status and logged at WARN. Those RPCs are #395's proto work.

## Summary of Changes Closes the **"reported completed, did nothing"** hole. > Stacked on #402 → #401 → #384. Review the final commit; GitHub retargets as the stack lands. ### The defect A mutation reported success whenever the server answered `OK`. IMAP answers `OK` for a UID command that matched nothing (RFC 3501 §6.4.8) — so a mutation that lost a race (message already moved or deleted by another client) was recorded `completed` having done nothing. Intent silently dropped, outbox row marked done. ### Why two states could not express it "Succeeded" and "failed" leave out the case that actually occurs: **the server accepted the command and its response does not establish what happened.** Collapsing that into either neighbour *is* the bug. ```rust MutationOutcome::Applied { token } | Conflict { observed } | Unknown { reason } ``` | Outcome | Requires | | --- | --- | | `Applied` | positive proof — `COPYUID` naming destination UIDs, a removal report, an echoed `FETCH` | | `Conflict` | positive proof — `[MODIFIED …]`. Means *changed by someone else*, **never** *gone* | | `Unknown` | everything else | `Unknown` is the honest reading of an absent `COPYUID`: only `SHOULD` for `MOVE` (RFC 6851 §4.3), legitimately omitted for a `UIDNOTSTICKY` destination, and indistinguishable from a UID set that matched nothing. ### How the outbox treats each **`Unknown` retries.** The asymmetry decides it: a mutation that *did* apply and is re-attempted is a no-op; one that did *not* and is recorded complete is unrecoverable. **`Conflict` is terminal and visible** (`conflicted` status), not retried. The intent was formed against state that no longer exists, so re-issuing it would either do nothing or overwrite someone else's change. ### Delete needed the collecting layer A QRESYNC-enabled server reports `VANISHED`, not `EXPUNGE` (RFC 6851 §4.4), and the typed expunge stream only yields the latter — reading it alone reports *nothing removed* on every successful delete. ## Type of Change - [x] Bug fix (non-breaking change fixing an issue) - [x] Breaking change (fix or feature causing existing functionality to change) - [ ] New feature - [ ] Documentation update `MailBackend::apply_mutation` returns `MutationOutcome` instead of `()`. Internal; no `.proto` change. ## Related Issues Refs #391. Design: [ADR 0002 §3](docs/adr/0002-multi-engine-sync-model.md) (PR #385). Depends on #401. ## Quality Verification Checklist - [x] `cargo fmt --all -- --check` - [x] `cargo clippy --all-targets --workspace -- -D warnings` - [x] `cargo test --workspace` — **627 passed, 0 failed** - [x] Business logic in headless engine crates - [x] No secrets, keys, or credentials added ## Notes for review - **3 new wire tests** against a real server: a move that lost a race is `Unknown` (and the destination is genuinely empty); a confirmed delete is `Applied`; a delete of an absent UID is `Unknown` with the real message untouched. - `uid_store_flags` was deleted — superseded by the collecting layer, which can see `MODIFIED`. - **JMAP reports `Applied` directly.** `Email/set` names the ids it updated or destroyed, so there is no COPYUID-style ambiguity: the id is stable and the server either lists it or does not. - **Not in this PR, stated plainly:** #391 also scoped *wiring the dead `calculate_backoff_ms`*. Doing it properly needs a `next_retry_at` column and a query filter — a schema change that should sequence against the monitor plan's outbox migration. Half-wiring it (retrying on the fixed 5s tick and calling it backoff) would be worse than leaving it visibly dead. **A 25-second blip still permanently fails every queued mutation**; that regression is unchanged by this PR and needs its own change. - Also deferred from #391: the durable `conflicts` table and `ListConflicts`/`ResolveConflict` RPCs. Conflicts are currently recorded as a terminal `conflicted` status and logged at WARN. Those RPCs are #395's proto work.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#403