Merge branch 'claude/webmail-offline-replica' into 'dev'
feat(electron): real offline mail replica — delta sync, full bodies, retention See merge request gitlab-instance-b9b5cf2f/vncmail-plus!6
This commit is contained in:
@@ -1,3 +1,37 @@
|
||||
> # ⚠️ PARTLY REINSTATED — read this note before the SUPERSEDED banner below
|
||||
>
|
||||
> A real offline mail replica **now exists**: `lib/offline-replica/**` +
|
||||
> `app/api/offline/{sync,mail,status}` + `lib/offline-fallback-client.ts`. So the banner below
|
||||
> ("that scope was dropped") is history, not current state. What was reinstated is the DATA MODEL
|
||||
> and the SYNC PROTOCOL from this document; what was **not** reinstated is its process
|
||||
> architecture — and that distinction is the whole reason the review's critical findings did not
|
||||
> come back with it.
|
||||
>
|
||||
> | This document proposed | What was built |
|
||||
> |---|---|
|
||||
> | A persistent background worker holding credentials for the process lifetime | **No worker.** A cycle is request-scoped work in an API route using the request's own `jmap_stalwart_ctx` cookie, triggered by the renderer's live push connection — the same shape the search index already uses. |
|
||||
> | An epoch-fenced multi-account `registry.json` | **No registry, no epochs.** Single-flight per account inside one process; all state in the SQLite file under a real transaction. |
|
||||
> | Multi-account simultaneous sync | **One request, one account, one cycle**, with hard budgets. |
|
||||
> | Engine reads a renderer setting to decide whether to sync | **The renderer decides when to sync.** The retention *policy* is durable inside the encrypted store (`/api/offline/status`), because the retention pass genuinely needs it server-side. |
|
||||
> | An offline read layer with no coherence story for the webmail's local unread-count arithmetic | **The replica is a FALLBACK, never a cache in front of the server** — consulted only after a read has failed at the transport level, so an online session never sees a replica count. This is the answer to review finding H3, the one finding that genuinely returned. |
|
||||
>
|
||||
> Consequently: **C2, C3, C4, H1 and H4 remain moot** (they were all consequences of the worker,
|
||||
> the registry, or a server-side engine reading renderer state), **C1 and H2 remain fixed** by what
|
||||
> already shipped (`optionalDependencies` + guarded require; the key on an inherited fd), and
|
||||
> **H3 is now in scope and answered** as above. See `lib/offline-replica/sync.ts`'s header for the
|
||||
> finding-by-finding version of this table, kept next to the code it constrains.
|
||||
>
|
||||
> Two implementation bugs from the mobile client are regressed by name, because both fail silently
|
||||
> and both cost user data: the **body-tier infinite redownload loop** (durable `gave_up` +
|
||||
> `shed-by-cap` marks + inserted-not-attempted counting) and the **clock-jump guard that wiped the
|
||||
> store** (persist the floor that was USED, never the one that was rejected, plus a separate
|
||||
> `evictionAllowed` bit). Tests:
|
||||
> `lib/offline-replica/__tests__/{store,retention}.test.ts`, and the real network-cut proof in
|
||||
> `integration/tests/13-electron-offline-replica.spec.ts`.
|
||||
>
|
||||
> Still deliberately out of scope: attachment blobs, offline compose/outbox, delegated/shared
|
||||
> accounts, and calendar/contacts/files replication.
|
||||
|
||||
> # ⚠️ SUPERSEDED — this is not what was built
|
||||
>
|
||||
> This document designs a **full offline mail replica**: a persistent background sync engine with
|
||||
|
||||
@@ -1,3 +1,31 @@
|
||||
> # ⚠️ UPDATE — a replica was later built, and this review is why it is shaped the way it is
|
||||
>
|
||||
> The table below says most of these findings "stopped existing" because the scope change removed
|
||||
> the thing they were about. A replica has since been built (`lib/offline-replica/**`), so that
|
||||
> reasoning was re-examined finding by finding rather than inherited:
|
||||
>
|
||||
> - **C1** — still FIXED, and untouched: the replica adds no new dependency and reuses the guarded
|
||||
> optional require. Both `docker build`s are unaffected.
|
||||
> - **C2, C3, C4, H1, H4** — still MOOT, and moot *for the same reasons*, because the persistent
|
||||
> background worker, the shared registry and the server-side-engine-reads-renderer-state shapes
|
||||
> were **not** reinstated. A cycle is request-scoped work in an API route with no resident
|
||||
> credential; there is no registry and no epoch; one request syncs one account. Had the worker
|
||||
> come back, all five would have come back with it.
|
||||
> - **H2** — still FIXED: the key crosses on an inherited file descriptor, never via environment,
|
||||
> and is zeroed after each job. The replica reuses that channel rather than inventing a second.
|
||||
> - **H3 — BACK IN SCOPE, and the only one that is.** This review was right that the webmail does
|
||||
> local delta arithmetic on mailbox unread counts, and a read-only offline cache underneath it
|
||||
> needs a coherence story. The answer is an ordering rule: the replica is consulted **only after
|
||||
> a read has failed at the transport level**, so it is never a cache in front of the server and
|
||||
> the arithmetic never operates on replica numbers. Enforcing that needed a real signal, because
|
||||
> `lib/jmap/client.ts` swallows read errors and returns plausible success — hence
|
||||
> `lib/jmap/transport-health.ts` and the two-part gate in `lib/offline-fallback-client.ts`.
|
||||
> - The *medium/low* findings (Linux-only API, the vacuous `cipher_version` check, the two bindings
|
||||
> not being interchangeable) were all already fixed in the shipped index and are inherited.
|
||||
>
|
||||
> Nothing in this review turned out to be wrong on re-reading. Its verdict — that the sync-engine
|
||||
> core transfers and the platform-specific sections were where the danger lay — held exactly.
|
||||
|
||||
> # ⚠️ SUPERSEDED — reviews a design that was not built
|
||||
>
|
||||
> This reviews `ELECTRON-OFFLINE-ENGINE-DESIGN.md`, which was **dropped**. Its findings were the
|
||||
|
||||
Reference in New Issue
Block a user