From 85c5d46ba0f5e2ca5e2e7d18b0668f769895af66 Mon Sep 17 00:00:00 2001 From: James Maes Date: Mon, 10 Aug 2026 14:26:59 -0500 Subject: [PATCH 1/2] docs(adr): record the convergent multi-engine sync model The engine syncs as though it is the only client of an account: a forward-only UID high-water mark that cannot observe deletion, and fire-and-forget mutations that report success when a UID command silently did nothing. A second client -- another machine, a phone, or webmail -- makes both assumptions false, so the local view diverges and nothing detects it. ADR 0002 decides how a daemon behaves as one of several clients. The core choice is to build no coordination layer: IMAP and WebDAV already coordinate concurrent clients, so the work is finishing the sync model rather than adding multi-engine support, and correctness at ten engines follows from correctness at one. The exception is side-effecting filter actions, which land on third parties with no shared resource to compare against; those get a single owner. A draft was reviewed independently for protocol correctness, codebase fit, and adversarially. All three falsified load-bearing claims, and the corrections are material enough to record rather than summarise: Gmail has no QRESYNC and Exchange has neither QRESYNC nor CONDSTORE, so the ladder needs four rungs selected per mailbox; an absent COPYUID is conforming on success and cannot mean conflict; keying identity on a Message-ID hash would have let a forged message overwrite a trusted body; and the proposed convergence test could not have detected the design's largest hole. The review evidence lands alongside the ADR because several findings cost real effort to establish and would otherwise be rediscovered expensively. Documentation only. No code or contract changes. --- docs/adr/0002-multi-engine-sync-model.md | 221 +++++++++++ .../2026-08-10-multi-engine-design-review.md | 346 ++++++++++++++++++ 2 files changed, 567 insertions(+) create mode 100644 docs/adr/0002-multi-engine-sync-model.md create mode 100644 docs/reviews/2026-08-10-multi-engine-design-review.md diff --git a/docs/adr/0002-multi-engine-sync-model.md b/docs/adr/0002-multi-engine-sync-model.md new file mode 100644 index 0000000..04a8cc4 --- /dev/null +++ b/docs/adr/0002-multi-engine-sync-model.md @@ -0,0 +1,221 @@ +# 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). + +## 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 +FLAGS.SILENT (UNCHANGEDSINCE ) + ($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 } | 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. +- **Migration is forward-only.** `message_key` cannot be derived for existing + rows: Message-ID is never parsed, never stored, and raw bytes are discarded + after parse. Ingest must capture identity first and let it accumulate. Legacy + rows keep their surrogate as `message_key` with one placement each and receive + no retroactive dedup. +- **The audit ledger gains a namespace boundary.** + `filter_execution_logs.message_id` is an input to an HMAC hash chain verified + by a live `Audit.VerifyChain`, so historical ids cannot be rewritten. +- **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. diff --git a/docs/reviews/2026-08-10-multi-engine-design-review.md b/docs/reviews/2026-08-10-multi-engine-design-review.md new file mode 100644 index 0000000..1e5d4a2 --- /dev/null +++ b/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 ``; 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. -- 2.54.0 From c6eca910c43e28d11c6f65ee3f9205a4cac4f27a Mon Sep 17 00:00:00 2001 From: James Maes Date: Mon, 10 Aug 2026 15:29:47 -0500 Subject: [PATCH 2/2] docs(adr): decide the pre-release posture and drop the migration compromise The first draft reasoned like a shipped product: preserve every stored row, never change behaviour without a path, keep old and new daemons interoperable. Nothing is released, so none of that is owed -- and paying for it distorted the model in one place that matters. Message identity cannot be derived for rows already stored, because Message-ID was never persisted and the octets are discarded after parse. Honouring those rows meant keeping them under their old surrogate forever: permanently un-deduped, invisible to every cross-folder guarantee the design exists to provide, and leaving the store two-tier for as long as the install survives. Resetting the message store and re-syncing from the server -- which is authoritative regardless -- costs a developer one sync and buys a single coherent model. The audit ledger follows the same reasoning rather than gaining a permanent namespace boundary: a hash chain whose entries reference ids that no longer resolve has no audit value, so it is reinitialised with the store it describes. This also removes the dependency that shaped the build order. Placements waited on captured identity accumulating only because history had to be preserved; now capture need only land one merge ahead of the model consuming it. The order is recorded explicitly, along with the note that change enumeration still ships alone and still fixes ghost messages for a single engine. The posture section is written to be deleted at the first real release, when every one of these questions becomes a genuine obligation. --- docs/adr/0002-multi-engine-sync-model.md | 77 +++++++++++++++++++++--- 1 file changed, 69 insertions(+), 8 deletions(-) diff --git a/docs/adr/0002-multi-engine-sync-model.md b/docs/adr/0002-multi-engine-sync-model.md index 04a8cc4..2e45fa1 100644 --- a/docs/adr/0002-multi-engine-sync-model.md +++ b/docs/adr/0002-multi-engine-sync-model.md @@ -37,6 +37,30 @@ 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. @@ -191,14 +215,21 @@ work is chunked per folder so it yields to user actions. 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. -- **Migration is forward-only.** `message_key` cannot be derived for existing - rows: Message-ID is never parsed, never stored, and raw bytes are discarded - after parse. Ingest must capture identity first and let it accumulate. Legacy - rows keep their surrogate as `message_key` with one placement each and receive - no retroactive dedup. -- **The audit ledger gains a namespace boundary.** - `filter_execution_logs.message_id` is an input to an HMAC hash chain verified - by a live `Audit.VerifyChain`, so historical ids cannot be rewritten. +- **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 @@ -219,3 +250,33 @@ work is chunked per folder so it yields to user actions. 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. -- 2.54.0