docs/adr/0002-multi-engine-sync-model.md

@@ -0,0 +1,282 @@
+# ADR 0002 — Convergent sync model for multiple concurrent engines
+
+- **Status:** Accepted
+- **Date:** 2026-08-10
+- **Deciders:** James Maes
+- **Extends:** [ADR 0001](0001-engine-first-grpc-architecture.md). Nothing here
+  changes the engine-first, contract-published architecture; this decides how a
+  daemon behaves as one of several clients of the same mail account.
+
+## Context
+
+ADR 0001 establishes one daemon owning all state. It does not say what happens
+when **several** daemons own state derived from the **same** upstream account —
+which is the normal case for a local-first product the moment a user runs it on
+a laptop and a desktop, and is already the case today whenever the user also has
+a phone mail app or Gmail open in a browser.
+
+The engine currently implements a **single-writer, append-only** model:
+
+- Mail sync is a forward-only per-folder high-water mark (`{uidnext}:*`), and
+  `fetch_and_persist` only ever calls `save_email`. Nothing observes deletion.
+- CalDAV/CardDAV sync re-queries a full window every time; there is no
+  sync-token, no CTag, no ETag use, and no write-back at all.
+- Local identity is `surrogate_id(account, folder, uidvalidity, uid)`, so the
+  same message in a different folder is a different message.
+- Mutations are fire-and-forget: `apply_mutation` returns `Result<()>` and
+  `Ok(())` is recorded as `completed` with no check that anything happened.
+
+The consequences are not hypothetical. A message moved or deleted by any other
+client becomes a permanent local ghost. A mutation that lost a race against
+another client is reported as success, because a UID command against a UID that
+no longer exists returns `OK` having done nothing (RFC 3501 §6.4.8). Two engines
+diverge silently; ten diverge identically and confidently.
+
+A design was drafted, then reviewed independently for protocol correctness,
+codebase fit, and adversarially. The reviews falsified four load-bearing claims
+in the draft. This ADR records the corrected decision; the evidence is in
+[`docs/reviews/2026-08-10-multi-engine-design-review.md`](../reviews/2026-08-10-multi-engine-design-review.md).
+
+## Pre-release posture
+
+Nothing is released. There is no installed base, no data anyone is entitled to
+keep, and no compatibility obligation. **Getting this model right outranks
+continuity of any store that exists today**, and the decisions below are made on
+that basis.
+
+Concretely, and in contrast to how a shipped product would have to reason:
+
+- **A local store is disposable.** Where a correct model needs data that cannot
+  be reconstructed from what is persisted, the answer is to reset the store and
+  re-sync from the server -- never to carry a degraded identity forward forever
+  so that old rows survive. A permanently un-deduped subset of history is a
+  worse outcome than a one-time re-sync on a developer's machine.
+- **Behaviour may change without a deprecation path.** A default that is wrong
+  gets corrected, not preserved behind a flag.
+- **Mixed-version fleets are not a supported configuration.** Upgrade all
+  daemons for an account together. `PRAGMA user_version` still earns its place
+  as cheap insurance against a stale binary silently writing an old model, but
+  the design owes nothing to interoperating with an older one.
+
+This section exists to be **deleted at the first real release**, at which point
+every migration question below becomes a genuine obligation.
+
+## Decision
+
+### 1. The servers are the coordinator. We do not build a distributed system.
+
+No engine-to-engine messaging, no leader election, no shared database, no vector
+clocks or CRDTs, no broker, no custom sync protocol. IMAP and WebDAV are designed
+for many concurrent clients against one collection and ship the primitives for
+it. If each engine is a **correct** client, N engines is **O(N) server load and
+O(1) design complexity**.
+
+The work is therefore not "add multi-engine support." It is "finish the sync
+model"; multi-engine falls out of it.
+
+### 2. There is one bounded exception: side-effecting actions.
+
+The servers coordinate *state*, not *actions*. `MOVE`/`COPY`/`FLAG` converge
+under at-least-once execution — ten moves produce one net effect. **`FORWARD`
+and `CALL WEBHOOK` do not.** They land on third parties with no shared resource
+to compare-and-set against, and every engine independently classifies the same
+message as new. Ten engines page an on-call engineer ten times per alert.
+
+- A per-account `filters_enabled` setting, **default off**, gives exactly one
+  engine ownership of filter execution. This converts a silent duplication bug
+  into an explicit, documented limitation, and requires no protocol work.
+- Where CONDSTORE and custom keywords are available, an engine may instead claim
+  a message with `UID STORE <uid> +FLAGS.SILENT (UNCHANGEDSINCE <modseq>)
+  ($NuncioFiltered)`; a `MODIFIED` response means another engine won the claim.
+  This is the same compare-and-set primitive as §4, so it adds no new machinery.
+- Rule identity becomes a content hash of normalized NSQL rather than random
+  bytes, and side-effecting actions carry `Idempotency-Key:
+  hash(rule_key, message_key)`, so receivers can dedup what we cannot make
+  once-only.
+
+### 3. Change enumeration replaces the high-water mark — four rungs, per mailbox.
+
+`CollectionSync::enumerate_changes(collection, since) -> {upserts, removals,
+next_token}`. The `removals` set is the entire fix for ghost messages, and it
+fixes the single-engine case too.
+
+| Rung | Mechanism | Providers |
+| --- | --- | --- |
+| 1 | QRESYNC: `ENABLE QRESYNC` + `SELECT (QRESYNC …)` → `VANISHED` | Fastmail/Cyrus, Dovecot |
+| 2 | CONDSTORE: `UID FETCH 1:* (FLAGS) (CHANGEDSINCE n)` + periodic UID-set diff | **Gmail** |
+| 3 | `STATUS`/`LIST-STATUS` change gate, then ESEARCH UID-set diff | **Exchange**, Yahoo |
+| 4 | Full resync | last resort |
+
+**Gmail advertises CONDSTORE but not QRESYNC; Exchange advertises neither.**
+Detection is per *mailbox*, not per account, because `NOMODSEQ` is a mailbox
+property. A two-rung ladder would put the largest provider on the most expensive
+path.
+
+CalDAV and CardDAV both use RFC 6578 `sync-collection` (identical shape), with
+`sync-level: infinite`, 507 truncation handling, loop-to-convergence, and
+`calendar-multiget`/`addressbook-multiget` to fetch bodies for changed hrefs.
+CTag is a vendor extension, not a peer mechanism, and is a fallback only.
+
+Sync state is structured, not a string: `(uidvalidity, highestmodseq,
+known-uid-set, seq-match-sample)`. QRESYNC **never signals modseq expiry** — a
+too-old modseq produces a large `VANISHED`, not an error — so a full resync is
+always a client-side derivation from a UIDVALIDITY mismatch, `NOMODSEQ`, or a
+HIGHESTMODSEQ regression. A periodic full UID-set reconciliation is retained
+even on rung 1 as a safety net.
+
+**Sync tokens carry a scheme tag** (`v2:qresync:…`). The existing checkpoint is
+`{uidvalidity}{DELIM}{uid}` — the same shape as `uidvalidity:highestmodseq` with
+a different meaning. Untagged reuse would hand a UIDNEXT to a server as a
+MODSEQ; because UIDNEXT is typically the larger number, the server would answer
+"nothing changed" and everything below it would never be enumerated again.
+Unknown schemes force a full resync.
+
+### 4. Mutations carry a precondition and must prove their effect. Three outcomes.
+
+```
+MutationOutcome = Applied { token: Option<Token> } | Conflict { observed } | Unknown
+```
+
+`Conflict` requires **positive evidence**: `[MODIFIED …]` in the tagged
+response, or HTTP 412. Anything ambiguous is `Unknown`, which triggers
+re-enumeration of the affected collections rather than a verdict.
+
+This matters because the obvious proofs are unreliable:
+
+- `COPYUID` is only *SHOULD* for MOVE (RFC 6851 §4.3), is advised in an
+  **untagged** OK, has no empty form, and is legitimately omitted for
+  UIDNOTSTICKY destinations. Present ⇒ applied; absent ⇒ **unknown**.
+- Under QRESYNC, `UID EXPUNGE` returns `VANISHED`, not `EXPUNGE` (RFC 6851 §4.4),
+  so counting `EXPUNGE` responses reports failure on every successful delete.
+- `+FLAGS.SILENT` suppresses the untagged FETCH (RFC 3501 §6.4.6) and is the
+  normal form for conditional stores, so "no FETCH ⇒ conflict" fires on the most
+  common flag operation.
+- A DAV server that rewrites a submitted object MUST NOT return an ETag
+  (RFC 4791 §5.3.4), and normalisation is the norm; `Applied` must tolerate a
+  missing token and schedule a re-read.
+
+Flags use per-flag last-writer-wins with benign merge. RFC 7162 §3.1.3 only
+*SHOULD*s avoiding spurious `MODIFIED` on single-modseq servers, so a blanket
+hard-conflict policy would make routine concurrent flag activity a constant
+conflict stream.
+
+### 5. Conflicts are durable and queryable, not a notification.
+
+Mutations model **desired end-state on an identity**, not an imperative
+operation on a placement, so re-enumerate-and-retarget preserves user intent;
+re-targets are capped, then a hard conflict is raised. Conflicts persist in a
+table with RPCs to list and resolve them. Events become an optimization over a
+queryable source of truth — necessary because `Subscribe` is a broadcast stream
+that silently drops events on lag, so no event-driven guarantee is currently
+sound without a sequence number and resumption.
+
+### 6. Identity is content-addressed and account-scoped.
+
+Precedence: `EMAILID` (RFC 8474 OBJECTID) → `X-GM-MSGID` → `(account_id,
+normalized Message-ID, content_hash)` → `(account, folder, uidvalidity, uid)`.
+
+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
+message reusing a known Message-ID would silently overwrite the body of a
+message the user already trusts and would evade filters by never being seen as
+new. That hole does not exist today and must not be introduced. The same
+Message-ID with different content is two messages, which also covers
+list-copy-vs-direct and drafts.
+
+A message is separated from its **placements** — `PRIMARY KEY(account_id,
+folder_id, uidvalidity, uid)`, with `uidvalidity` in the key so UIDs restarting
+after a bump cannot collide with old rows. DAV gets the same split: href =
+placement, iCalendar/vCard UID = identity, ETag = version.
+
+### 7. Load is bounded by bandwidth, not connection count.
+
+Gmail publishes both a 15-simultaneous-connection and a 2,500 MB/day download
+cap **per account**. On rungs 2 and 3, ten engines over five accounts reach the
+byte cap long before the connection cap. Per-engine budget derives from a
+`fleet_size` setting, since cross-engine rate limiting is impossible without the
+coordination layer we decline to build. `Retry-After` is honoured *with* jitter
+proportional to `fleet_size`, or ten engines retry in the same instant.
+
+One mutex-guarded connection per (engine, account) with mutation priority; sync
+work is chunked per folder so it yields to user actions.
+
+## Consequences
+
+- **This gates the M7 API freeze.** Mutation RPCs do not exist (`Mail` has no
+  Move/Delete/Flag); `Conflict` must be a typed error, not a response field, so
+  old clients take the error path; `Message` needs identity and placements.
+  Most individual additions are wire-additive, but redefining `Message.id`
+  changes the *value* of every existing id while staying wire-compatible —
+  invisible to the byte-exact descriptor golden, and that token appears at ten
+  other wire positions. A model that cannot express deletion or conflict must
+  not be frozen.
+- **Shared OAuth accounts across engines are out of scope.** Gmail and
+  Microsoft 365 mandate OAuth; providers that rotate refresh tokens on use would
+  have ten engines invalidating each other. A token broker is exactly the
+  coordination layer this ADR declines to build. Documented limitation.
+- **The store resets rather than migrating into a degraded identity.**
+  `message_key` cannot be derived for rows already stored: Message-ID is never
+  parsed, never persisted, and the raw octets are discarded after parse. A
+  shipped product would have to keep those rows under their old surrogate,
+  permanently un-deduped and invisible to every cross-folder guarantee the model
+  provides. Pre-release that trade is not worth making: identity-bearing schema
+  changes bump `PRAGMA user_version`, the message store is rebuilt, and the
+  account re-syncs from the server, which is authoritative anyway. Account
+  configuration and credentials are preserved; cached mail is not.
+- **The audit ledger is reset with the store, not namespaced.**
+  `filter_execution_logs.message_id` feeds an HMAC hash chain verified by a live
+  `Audit.VerifyChain`, so historical ids genuinely cannot be rewritten in place.
+  Rather than carry a permanently mixed-namespace ledger, the chain is
+  reinitialised alongside the message store it refers to. A ledger whose entries
+  point at ids that no longer resolve has no audit value.
+- **A new class of test is required.** Convergence cannot be shown by one engine
+  against one mock. Verification needs ≥3 engines against one stateful server,
+  plus side-effect-counting SMTP and webhook mocks — final-state placement
+  convergence is blind to duplicate forwards, so the largest risk in this ADR
+  would pass a naive convergence test.
+- **`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.
+
+## Alternatives considered
+
+- **A coordination layer** (broker, leader election, shared state). Rejected: it
+  would be the largest and least reliable component in the repo, and it solves a
+  problem the servers already solve for everything except side-effecting actions.
+- **CRDTs / vector clocks.** Rejected: the server holds authoritative state, so
+  there is no merge to compute — only a state to observe and preconditions to
+  respect.
+- **Freezing `nuncio.v1` first and adding conflict semantics later.** Rejected:
+  not expressible additively.
+- **Preserving existing local stores across the identity change.** Rejected for
+  now — see [Pre-release posture](#pre-release-posture). It buys nothing anyone
+  is owed and costs a permanently two-tier message store.
+
+## Build order
+
+The dependency that originally forced this sequence — placements waiting for
+captured identity to accumulate, because history could not be re-keyed — does
+not exist once the store is resettable. Capture still has to land before the
+model that consumes it, but only by one merge, not by a soak period.
+
+| # | Work | Depends on |
+| --- | --- | --- |
+| P0 | Stateful multi-connection IMAP mock harness | — |
+| P1 | Namespace-aware DAV XML reader + href/ETag identity | — |
+| P2 | Message-ID + content-hash capture at ingest | — |
+| P3 | `user_version` gating + scheme-tagged sync tokens | — |
+| P4 | Raw IMAP command layer (`ENABLE`, tagged codes, `VANISHED`) | P0 |
+| 1 | Message/placement identity split | P2, P3 |
+| 2a | IMAP change enumeration, four-rung ladder | P0, P4 |
+| 2b | DAV `sync-collection` + multiget + 507 paging | P1 |
+| 3a | Three-state mutation outcomes + `Conflict` | P4 |
+| 3b | Mutation RPCs + DAV conditional write-back | 1, 3a |
+| 4 | Scheduler, session pooling, rate limiting, backoff | 3a (partly) |
+| V | ≥3-engine convergence harness with side-effect counting | P0 |
+
+**2a remains the highest value per unit of work** and still ships independently:
+it fixes ghost messages for a single engine, against either schema.
+
+Filter-execution ownership (§1) is independent of all of it and ships first.

docs/reviews/2026-08-10-multi-engine-design-review.md

@@ -0,0 +1,346 @@
+# Design review — multi-engine sync model (2026-08-10)
+
+Evidence behind [ADR 0002](../adr/0002-multi-engine-sync-model.md). A draft
+design was reviewed independently along three axes, each instructed to find
+errors rather than confirm the draft, and to say plainly when an attack failed.
+All three found load-bearing defects. Reviewed against `dev` @ `a997220`.
+
+Findings are recorded here because several are non-obvious, several cost real
+effort to establish, and at least two would otherwise be rediscovered the
+expensive way.
+
+---
+
+## A. Protocol correctness
+
+### A1. The provider capability matrix in the draft was wrong
+
+The draft claimed Gmail, Fastmail, Dovecot, and Exchange all support CONDSTORE
+and QRESYNC. Verified against live capability strings:
+
+| Provider | CONDSTORE | QRESYNC |
+| --- | --- | --- |
+| Fastmail (Cyrus) | yes | yes (also RFC 8474 OBJECTID) |
+| Dovecot | yes | yes, but `NOMODSEQ` per mailbox where persistent modseqs were never enabled |
+| **Gmail** | yes | **no** — not planned |
+| **Exchange / Exchange Online** | **no** | **no** |
+
+Two of four named providers cannot use the draft's headline mechanism. This is
+what forced the ladder from two rungs to four, and capability detection from
+per-account to per-mailbox (RFC 7162 §3.1.2.2 makes `NOMODSEQ` a mailbox
+property).
+
+The draft also omitted the CONDSTORE-only middle rung entirely — RFC 7162 §3.2.5.1
+states QRESYNC's `SELECT` is semantically equivalent to `UID FETCH … (CHANGEDSINCE
+… VANISHED)`, i.e. a round-trip optimisation over CONDSTORE, not a different
+capability class. Skipping it put Gmail on full flag sweeps against a published
+2,500 MB/day per-account cap.
+
+### A2. QRESYNC has no modseq-expiry error
+
+RFC 7162 §3.2.6: a server receiving a mod-sequence older than it remembers
+**MUST behave as if asked to report all expunged messages from the provided UID
+set**. The failure mode is a bandwidth blowup, not a refusal. `full_resync_required`
+can therefore never be *received*; it must be derived client-side from a
+UIDVALIDITY mismatch (§3.2.5), `NOMODSEQ` (§3.1.2.2), or a HIGHESTMODSEQ
+regression.
+
+`uidvalidity:highestmodseq` is also insufficient state: §3.2.5.1 and §3.2.5.2
+make the known-UID set and sequence-match data part of what the client supplies.
+And §3.2.3 requires a tagged **BAD** if `QRESYNC` is used without `ENABLE QRESYNC`
+on the connection.
+
+### A3. "Absent COPYUID means the mutation failed" is wrong three times over
+
+- RFC 4315 §3 ABNF has no empty `uid-set`; the states are *present* or *absent*.
+- Absence is conforming on success: the client may lack SELECT rights on the
+  destination, or the destination may be UIDNOTSTICKY.
+- RFC 6851 §4.3 makes COPYUID only *SHOULD* for MOVE, and advises sending it in
+  an **untagged** OK — so a client reading only the tagged OK misses it on every
+  conforming MOVE.
+
+Additionally, RFC 6851 §4.4: under QRESYNC the server sends `VANISHED` rather
+than `EXPUNGE` for `UID EXPUNGE`, so a delete proved by counting `EXPUNGE`
+responses reports conflict on every success.
+
+Consequence: a third outcome, `Unknown`, is required. `Conflict` needs positive
+evidence.
+
+### A4. MODIFIED does not cover the case the draft cared about
+
+RFC 7162 §3.1.3 puts `MODIFIED` in the **tagged** response, and only *SHOULD*s
+that servers avoid spurious `MODIFIED` on single-modseq servers — so engine B
+setting `\Seen` can fail engine A's unrelated `\Flagged` store. More
+importantly, `MODIFIED` means "changed by someone else"; a UID another engine
+already *moved or expunged* is simply ignored (RFC 3501 §6.4.8) and never
+appears in `MODIFIED`. Presence detection needs a separate mechanism.
+
+Relatedly, the draft's flag proof-of-effect ("untagged FETCH present") is unsound:
+`.SILENT` suppresses it (RFC 3501 §6.4.6) and is the normal form for conditional
+stores.
+
+### A5. Message-ID is not a content identifier
+
+RFC 5322 §3.6.4 makes it *SHOULD*, not MUST. One Message-ID legitimately maps to
+different octets for mailing-list-plus-direct copies (Mailman preserves it while
+adding `List-*` headers and a footer) and for drafts (Gmail and Exchange keep one
+Message-ID across saves). The draft's "hash of raw headers" fallback is also
+unstable across exactly the placements it exists to unify, because `Received:`
+chains differ.
+
+Missed primitives that solve this properly: **`EMAILID`** (RFC 8474, Fastmail/Cyrus,
+Dovecot) and **`X-GM-MSGID`** (Gmail) are server-assigned identities stable across
+folders.
+
+### A6. DAV specifics the draft omitted
+
+RFC 6578 §3.6 truncation (207 + `507` + `DAV:number-of-matches-within-limits`) —
+"Clients MUST handle the 507 status"; §3.1 loop-to-convergence; `sync-level:
+infinite` (§3.5.1 warns `sync-level: 1` is not reliable); discovery via
+`DAV:supported-report-set`; and `calendar-multiget`/`addressbook-multiget`
+(RFC 4791 §7.9, RFC 6352 §8.7) because `sync-collection` returns hrefs and ETags,
+not content. RFC 4791 §5.3.4: a server that rewrites the submitted object MUST NOT
+return an ETag. RFC 6638 scheduling is a write hazard — PUTting a VEVENT with
+ATTENDEEs on a scheduling-aware server emails real people.
+
+`getctag` is a Calendar Server vendor extension, not an RFC, and was
+mis-presented as a peer of RFC 6578.
+
+### A7. Other omitted primitives
+
+`STATUS (… HIGHESTMODSEQ)` (RFC 7162 §3.1.7) and `LIST-STATUS` (RFC 5819, which
+Gmail advertises) give a whole-account change probe in one round trip.
+`COMPRESS=DEFLATE` (RFC 4978) directly reduces metered bytes. `NOTIFY` (RFC 5465)
+watches many mailboxes on one connection, which moots the draft's
+"IDLE costs a connection per mailbox" reasoning. ESEARCH `SEARCH RETURN (ALL)`
+is far cheaper than `UID FETCH 1:* (FLAGS)` for a vanished-set diff.
+`UIDNOTSTICKY` (RFC 4315) invalidates both COPYUID proof and the placement key
+and must be detected.
+
+---
+
+## B. Codebase fit
+
+### B1. `message_key` cannot be derived — the blocker is ingest, not migration
+
+`MimeParserAdapter::parse_mime` never calls `message_id()`; `Email` has no such
+field; the `messages` table has no column for it; and `build_email_from_fetch`
+discards the `BODY[]` bytes after parsing. The IMAP `ENVELOPE` does carry
+`message-id`, but only `subject`/`from`/`to` are read.
+
+So in-place migration of a populated database is impossible — there is neither a
+Message-ID nor raw headers to hash. Either re-fetch every message (offline-hostile,
+impossible for a removed account) or set `message_key := old surrogate`, which
+yields one placement per message and no retroactive dedup. Capture must ship
+first and accumulate.
+
+### B2. The audit hash chain pins historical ids permanently
+
+`filter_execution_logs.message_id` is an input to `compute_log_hash`, verified by
+`verify_execution_log_chain`, reachable from the live `Audit.VerifyChain` RPC.
+Rewriting historical ids breaks verification permanently. `worm_audit_records` is
+harder still — `BEFORE UPDATE`/`BEFORE DELETE` `RAISE(ABORT)` triggers. A
+documented namespace boundary with a schema-version marker is the only option.
+
+### B3. Placements break the filter engine's evaluation contract
+
+`FilterField::Folder` and `FilterField::Account` evaluate scalar fields on
+`Email`; `WHERE FOLDER = 'INBOX'` has no meaning against a multi-placement
+message. `evaluate(&Email)` must become per-placement.
+
+More seriously: **the fire-once guard is `existing_message_ids`, not the
+execution log.** Under `message_key`, a genuinely new arrival that already exists
+in another folder becomes "not new" and **silently never fires any filter** — a
+new correctness bug the identity change introduces. The guard must become
+placement-aware *and* gain a real `(rule_id, message_key)` dedup.
+
+`set_message_read` is `UPDATE messages … WHERE id = ?`; since IMAP `\Seen` is
+per-mailbox, read state becomes per-placement and `MARK READ` must name one.
+
+### B4. FTS orphans are a confidentiality problem
+
+`messages_fts` holds **plaintext** bodies keyed on `messages.id`, with a
+delete-only trigger. Under placements, dropping *a* placement must not delete the
+FTS row while others reference the body; dropping the *last* must. Inverted, this
+either loses search or leaks plaintext bodies in orphaned rows — in the one table
+the schema comments already document as unencrypted.
+
+Also: there is no `folders` table. Folders are derived from `messages GROUP BY
+folder_id` and are currently global across accounts, which placements will expose.
+
+### B5. `async-imap` 0.11.3 cannot express the design
+
+| Needed | Status in the dependency |
+| --- | --- |
+| `COPYUID` | `uid_copy`/`uid_mv` return `Result<()>`; `check_status_ok` discards the response code |
+| `[MODIFIED …]` | no `UNCHANGEDSINCE` variant; tagged code discarded |
+| `SELECT (QRESYNC …)` | no `select_qresync`; no `ENABLE` command at all |
+| `VANISHED` | parsed by `imap-proto` but dropped by async-imap's `select()` parser |
+| untagged FETCH / expunged UIDs | reachable; both currently drained and discarded |
+
+`run_command`/`read_response` are public, so a hand-rolled tagged-response
+collector is possible (~80–150 LOC plus per-command parsers) — but it is a
+prerequisite, and it is subject to the workspace's `-D warnings` / no-`unwrap`
+regime. Fork-versus-hand-roll is a gate on the protocol work.
+
+### B6. There is no IMAP mock, and no scheduler, and no backoff
+
+- `wiremock` is HTTP-only. IMAP had only ad-hoc inline `duplex` responders:
+  single connection, no shared state. *(Addressed by PR #384.)*
+- `OutboxManager::calculate_backoff_ms` is **dead code**; retries run on a fixed
+  5s tick with `MAX_RETRIES = 5`, so a 25-second blip permanently fails every
+  queued mutation fleet-wide.
+- `sync_interval_secs` is stored and validated but **nothing polls**. There is no
+  periodic sync at all; `main.rs` spawns only the outbox drain (5s) and an opt-in
+  update check (24h).
+- `SyncProgress` never reaches the wire — it is dropped at the gRPC boundary.
+- `Mail.MarkRead` and `Contacts.CreateContact` are silently **local-only** and
+  report success.
+- `AccountProtocol` has no `CardDav` variant, so a CardDAV account is not
+  representable — contacts sync is unwireable, not merely unwired.
+- The DAV "parsers" are substring splits on `<c:calendar-data>`; a different
+  namespace prefix yields zero events reported as a successful sync. DAV ids are
+  positional, so re-syncs reshuffle which local row holds which remote object.
+
+### B7. The proto claim was overstated in the draft's favour and against it
+
+Three of five changes *are* wire-additive with free tags (mutation RPCs, sync
+tokens in `GetStatus`, new `Event` oneof arms). The genuinely dangerous one is
+invisible: redefining `Message.id` changes the *value* of every existing id while
+staying wire-compatible — the byte-exact descriptor golden cannot see it, and the
+token appears at ten other wire positions.
+
+`STORY-WORKFLOW.md` permits one proto-touching story in flight; since four work
+items touch the proto, a single "proto chunk" is not schedulable and must
+dissolve into a serialized tail on each.
+
+### B8. Sizing
+
+The draft's M/L/M/S/M (~4–6 weeks) is roughly 3× optimistic. Measured against the
+tree: **5,800–9,300 production LOC + 4,000–6,600 test LOC across ~90–120 file
+touches** — about a quarter of focused work. Chunk 1 alone is ~120–160 edit sites
+across ~25–30 files. This substantially *is* WS-B + WS-C + WS-E + parts of M6.5.
+
+---
+
+## C. Adversarial
+
+### C1. Filter fan-out — the largest hole, absent from the draft entirely
+
+All ten engines classify the same message as new (per-engine store), and
+`pending_remote_mutations` has no uniqueness constraint. State-mutating actions
+converge; `FORWARD` and `CALL WEBHOOK` do not — the on-call engineer is paged ten
+times per alert. Rule IDs are `OsRng`-random per engine, so the "same" rule has
+ten identities and no receiver can dedup from rule identity even in principle.
+
+Adding a `Conflict` outcome makes the UX *worse* here, not better: the nine losing
+MOVEs stop silently fabricating success and instead emit conflict events to two
+clients each — eighteen spurious notifications per filtered message. "Server wins,
+loudly" is calibrated for rare conflicts; filter fan-out makes conflict routine.
+
+The draft's one sentence that sounds like it addresses this ("filters key on
+`message_key`, so they stop re-firing on the destination copy") is about *self*
+re-firing after a move — a different bug.
+
+### C2. The draft's identity scheme introduces a spoofing primitive
+
+Message-IDs are public and echoed in `References:`. With `INSERT OR REPLACE` and a
+key of `hash(Message-ID)`, an attacker reusing a known Message-ID silently
+overwrites the body shown under the original message's placement, and evades
+filters entirely by never being classified new. **This hole does not exist today**,
+because the current `surrogate_id` includes the UID. The identity change would
+have created it.
+
+### C3. Mixed-version fleets fight on the shared server
+
+Old engines key identity on folder, so a new engine's move looks like a brand-new
+message to them — they re-evaluate the rule set and re-move it. Old and new
+engines trade the message back and forth. There is no `PRAGMA user_version`, no
+down path, and an old binary opening a new database "works" while writing the old
+model, leaving two mutually invisible truths in one file.
+
+Worst of these: the sync-token shape collision described in ADR 0002 §3, which
+fails *silently* rather than loudly.
+
+### C4. "Server wins, loudly" strands intent and leaves clients nothing to act on
+
+A user pressing Delete on engine 7 for a message engine 2 already archived has
+their intent discarded; re-enumeration removes the placement, so from their seat
+the delete appeared to work. Retry is *safe* here — `message_key` is stable and
+the new location is known — and the blanket prohibition follows from modelling
+mutations as imperative operations on placements rather than desired end-state.
+
+Clients get no event payload contract, no RPC to act on a conflict, no listing, no
+GC, and `Subscribe` silently drops events on lag with no sequence number or
+resumption. "Loudly" reduces to a log line that sometimes reaches a UI.
+
+### C5. Connection math measures the wrong quantity
+
+The outbox opens a **fresh connection per mutation**: worst case 50 connect/login
+cycles per 5s per engine, ×10 engines = 500 authentications per 5 seconds against
+one account. Providers rate-limit *authentications* far harder than concurrent
+connections, and some respond with a temporary lockout. Sync and outbox also
+contend for the single pooled connection the draft specifies — share it and a
+mark-read queues behind a 300-second per-account sync timeout; don't, and it is
+2 per engine, which the draft's own table marks as over cap.
+
+Jitter alone does not solve correlated wake-ups (workday start, VPN up, power
+restored), and honouring `Retry-After` without jitter creates a synchronized herd.
+Rate limiting across engines is impossible without coordination, so an engine must
+know its `fleet_size` and derive its budget from it.
+
+### C6. Placement schema and mutation addressing
+
+The draft's `PRIMARY KEY(account_id, folder_id, uid)` collides after a UIDVALIDITY
+bump, when UIDs restart at 1. And a mutation has no placement coordinate: a
+message in both INBOX and Archive with a rule saying MOVE TO Archive has no
+defined source. The outbox currently recovers addressing from the *message* row,
+which ceases to exist under the split.
+
+Drafts are unaddressed and will churn the model (APPEND-then-delete per save, new
+UID each time, same or new Message-ID depending on client).
+
+### C7. The proposed verification cannot detect C1
+
+Final-state placement convergence is exactly blind to duplicate side effects — ten
+forwards and one forward produce identical placements. N=2 also shows a single
+duplicate, which reads as an off-by-one. Requires N≥3, side-effect-counting SMTP
+and webhook mocks asserting "exactly one forward across the fleet", and an
+interleaving harness that can hold one engine's mutation in flight while another
+acts on stale state.
+
+### C8. OAuth breaks the shared-account premise
+
+Gmail and Microsoft 365 mandate OAuth; there is none in the tree. Providers that
+rotate refresh tokens on use would have ten engines invalidating each other in a
+loop, and with no backoff that becomes a lockout. A token broker is the
+coordination layer the design declines to build — so this is a scope boundary, not
+a TODO.
+
+### C9. Smaller items
+
+Clock skew across ten machines makes any wall-clock tiebreak unreliable (outbox
+ordering, audit ledger). "Body stored once" is per-engine, not fleet-wide. The
+append-only hash-chained execution log has no retention policy and will orphan
+entries. One poison collection that always fails burns the account's 300s timeout
+every cycle. And the draft never stated a consistency guarantee at all — several
+of the above decisions fall out of writing one down.
+
+---
+
+## What the reviews agreed the draft got right
+
+- **"Don't build a distributed system" is correct for state convergence.** For
+  placements, flags, and DAV objects, the servers genuinely are the coordinator,
+  and O(N) load with O(1) design complexity is the right trade. The thesis fails
+  only for side-effecting actions (C1) — a bounded exception, not a repudiation.
+- **`CollectionSync` with `{upserts, removals, next_token}` is the right shape**,
+  and the RFC 7162 / 6578 mapping is sound.
+- **"An empty effect must not be reported as success" is correct and important**;
+  the objection is to the verdict drawn from it, not to detecting it.
+- **The M7-freeze argument is correct and understated** — C4 and B6 strengthen it,
+  since the freeze must also cover a durable conflict surface, event sequence
+  numbers, and the fact that `MarkRead` will come to mean something different.
+- **Change enumeration ships first and alone**, and fixes ghost messages for
+  single-engine users independently of everything else. The draft was wrong that
+  it depends on the identity split.