Merge branch 'claude/electron-offline-design' into dev
Encrypted SQLite/FTS5 offline search index for the Electron desktop client: event-driven reindex (mail, calendar, contacts, files) driven off the existing JMAP push connection, per-account keys held in OS keychain via safeStorage, search API returns ranked context ready for an LLM/RAG prompt.
This commit is contained in:
Bernd Rodler
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,259 @@
|
||||
> # ⚠️ SUPERSEDED — reviews a design that was not built
|
||||
>
|
||||
> This reviews `ELECTRON-OFFLINE-ENGINE-DESIGN.md`, which was **dropped**. Its findings were the
|
||||
> direct cause: seeing them, the human narrowed the requirement from a full offline mail replica to
|
||||
> *"a SQLite index we can prompt against"*, refreshed on each delivery/change event. What shipped is
|
||||
> `lib/mail-index/**` + `app/api/offline/{reindex,search}` — see that doc's superseded note.
|
||||
>
|
||||
> **This review did its job.** Most of its severe findings were resolved by the scope change
|
||||
> removing the thing they were about, which is the strongest outcome a review can have:
|
||||
>
|
||||
> | Finding | Outcome |
|
||||
> |---|---|
|
||||
> | **C1** — `@signalapp/sqlcipher` in `dependencies` breaks both Alpine `docker build`s | **FIXED as specified.** It is an `optionalDependencies` entry with a guarded runtime require (`lib/mail-index/binding.ts`). Both `docker build`s verified passing, and the require verified failing cleanly with MODULE_NOT_FOUND inside the musl image. |
|
||||
> | **C2** — credentials are request-scoped, so no persistent worker can hold them | **MOOT.** There is no worker. Indexing is a normal API route using the request's own `jmap_stalwart_ctx` cookie, via the existing `lib/stalwart/credentials.ts`. |
|
||||
> | **C3** — the OAuth-refresh mitigation is itself the bug | **MOOT, and avoided by construction.** The indexer never touches the refresh-token cookie; it only reads an already-minted auth header, so it cannot rotate a token into a response nobody reads. |
|
||||
> | **C4** — shared `registry.json` breaks the multi-account safety premise | **MOOT.** No registry, no epochs, no concurrent workers. |
|
||||
> | **H1** — a server-side engine can't read a renderer-only setting | **MOOT.** The renderer decides when to index. |
|
||||
> | **H2** — key handoff sequencing, and a nonce via env is readable by same-user processes | **FIXED.** The key crosses on an **inherited file descriptor**, never env, and is fetched per job and zeroed after — not held. Sequencing is moot: the key is fetched when a job runs, not at spawn. |
|
||||
> | **H3** — local unread-count arithmetic needs a coherence story | **MOOT.** A retrieval index does not need to stay coherent with live unread counts. |
|
||||
> | **H4** — no cap on concurrent multi-account sync | **MOOT.** One request, one account. |
|
||||
> | *medium/low:* `getSelectedStorageBackend()` is Linux-only and would crash elsewhere | **FIXED** — platform-guarded. |
|
||||
> | *medium/low:* `cipher_version` check would pass vacuously on zero rows | **FIXED** — the shipped assertion requires a non-empty *string*, and a test reads the raw file bytes for a plaintext canary. |
|
||||
> | *medium/low:* the two bindings are not "the same code either way" | **CONFIRMED true, the hard way.** `@signalapp/sqlcipher` rejects varargs params (`TypeError: Params must be either object or array`) where better-sqlite3 accepts them. Documented in `binding.ts`. |
|
||||
>
|
||||
> Reviewing this file's own accuracy: its two re-executed claims (the binding working in Electron 43,
|
||||
> and `PRAGMA key` being a silent no-op) both held up and both shaped the shipped code.
|
||||
|
||||
# Adversarial review: `docs/ELECTRON-OFFLINE-ENGINE-DESIGN.md`
|
||||
|
||||
Reviewer: independent agent, fresh context, no relation to the design's author. 2026-08-04.
|
||||
|
||||
## Verdict
|
||||
|
||||
**Needs substantial rework before implementation — but narrowly scoped rework.**
|
||||
|
||||
The delta-sync core (everything tagged `[reused]` from M, the mobile design) is genuinely sound
|
||||
and transfers; the reviewer attacked it directly and could not break it. The problem is that **all
|
||||
four genuinely-new sections have an unclosed load-bearing mechanism**, and one of them breaks a
|
||||
build that ships today:
|
||||
|
||||
- §3 (binding choice) contains a packaging decision that breaks the hosted Docker image and the
|
||||
integration fixture.
|
||||
- §2 (process choice) rests on a credential claim that is only true inside an HTTP request.
|
||||
- §6 (key handoff) is under-specified in a way that doesn't work as sequenced, and its "unresolved
|
||||
implementation choice" is not security-neutral.
|
||||
- §5.3/§8.3 (multi-account) breaks the specific premise M's D6 fix relies on.
|
||||
|
||||
Nothing here requires re-architecting the sync engine. Stages B-G can proceed against M as
|
||||
written. Stage A as currently specified would not surface most of this.
|
||||
|
||||
All file:line citations in the design doc that were checked resolve correctly (one trivial
|
||||
miscount, noted at the end) — citation quality is high; the problems are in the reasoning built
|
||||
on top.
|
||||
|
||||
---
|
||||
|
||||
## CRITICAL
|
||||
|
||||
### C1 — Adding `@signalapp/sqlcipher` to `dependencies` breaks the hosted Docker build *and* the integration fixture
|
||||
|
||||
**Where:** §3.3.1 ("entering `dependencies`"), §3.3.5, §2.4, §10.5, E2, §13 item 6.
|
||||
|
||||
`Dockerfile:1-4` — `FROM node:24-alpine`, `RUN npm ci`. `integration/webmail.Dockerfile:12-15` —
|
||||
same, `FROM node:24-alpine` + `npm ci`.
|
||||
|
||||
Verified from the published tarball that `@signalapp/sqlcipher@4.0.3`:
|
||||
- ships **6** prebuilds — `darwin-{arm64,x64}`, `linux-{arm64,x64}`, `win32-{arm64,x64}`. No
|
||||
`linuxmusl-*`. (The design doc's list is exactly right.)
|
||||
- ships **no build sources at all** — published `files` is `dist/*`, `prebuilds`, `README.md`. No
|
||||
`binding.gyp`, no `src/`, no `deps/`.
|
||||
- has `install: node-gyp-build`. `node-gyp-build`'s `bin.js` runs `node-gyp-build-test`; on
|
||||
failure it calls `build()` → spawns `node-gyp rebuild` → `process.exit(code)`.
|
||||
|
||||
No prebuild + no `binding.gyp` ⇒ `node-gyp rebuild` fails ⇒ **`npm ci` exits nonzero**. The Linux
|
||||
prebuild also has a glibc ≥ 2.34 floor, so it could not load on musl even if copied.
|
||||
|
||||
**Concrete failure:** the next `docker build` of the production image fails at line 4.
|
||||
`npm run test:integration` fails to build the webmail container. Neither is gated by
|
||||
`VNCMAIL_DESKTOP_STORE_DIR` — that env var only governs *activation*, not *installation*.
|
||||
|
||||
§3.3.5 dismisses musl as "relevant if an Alpine-based container ever wants the engine — which,
|
||||
per §2.4, it must not" — that reasoning is inverted: the Alpine container doesn't want the engine,
|
||||
it just needs `npm install` to succeed regardless.
|
||||
|
||||
**Fix direction:** `optionalDependencies` + a guarded runtime `require` (which also delivers E2's
|
||||
graceful-load-failure behavior for free), or a separate optional package, or `--omit=optional` in
|
||||
both Dockerfiles. Pick one and say so explicitly; add "`docker build` of both Dockerfiles still
|
||||
succeeds" to the Stage A verification list.
|
||||
|
||||
### C2 — Option A's central justification is only true inside an HTTP request; the Worker credential path does not exist
|
||||
|
||||
**Where:** §2.1-for-1, §1.2, §2.4 (Worker), §5.3, §8.1 triggers T1/T4/T5/T11, §13 item 1.
|
||||
|
||||
Verified in `app/api/auth/session/route.ts` and `app/api/auth/token/route.ts`: every credential
|
||||
read goes through `cookies()` from `next/headers` — request-scoped. `lib/oauth/cookie-config.ts:11`
|
||||
sets `httpOnly: true`. The cookies live in the renderer's cookie jar, not in the server. The
|
||||
standalone server holds no session state whatsoever; it decrypts a cookie per request and
|
||||
discards it.
|
||||
|
||||
So "the credentials are already there... No new credential path, no IPC carrying secrets, no
|
||||
second copy" (§2.1-for-1) is materially overstated. What is actually there is *the ability to
|
||||
decrypt a credential presented on an inbound request* — not a resident credential.
|
||||
|
||||
Consequences the design never addresses:
|
||||
|
||||
1. **T1 ("server process ready + an account's credentials resolvable") cannot fire.** At
|
||||
server-ready there are no cookies anywhere. Nor can T4 (network regained), T5 (`StateChange`
|
||||
on the engine's own socket), or T11 (resume from sleep) — none is an inbound renderer request.
|
||||
2. **A `worker_threads` Worker is a separate execution context with no cookie access at all.**
|
||||
§2.4 mandates the Worker and routes talk to it via `postMessage`, but there's no specified
|
||||
point at which the Worker actually receives credentials.
|
||||
3. The only workable shape is: on the first renderer request, decrypt and hand the **plaintext**
|
||||
credentials to the Worker, which retains them for the process lifetime. That is a new
|
||||
long-lived plaintext secret in a new location — the exact thing §2.1-for-1 claims doesn't
|
||||
happen, and the same category of thing `client.ts:6055-6059` already declined once (a
|
||||
resident credential copy in a process that didn't previously hold one). It also creates an
|
||||
invalidation problem never addressed: password change, logout elsewhere, or a cleared cookie
|
||||
leaves the Worker retrying stale credentials indefinitely (since `AuthenticationError` is
|
||||
correctly never treated as a purge signal) — against a server with failed-auth lockout, this
|
||||
locks the user's account.
|
||||
|
||||
This doesn't kill Option A, but it kills the argument that Option A is free of new secret
|
||||
handling — which was the design's #1 stated reason for choosing it over the alternative. That
|
||||
comparison needs to be redone with the resident-copy cost included, not dropped.
|
||||
|
||||
### C3 — The proposed OAuth-refresh mitigation (E11) is not just unimplementable; it *is* the bug it's meant to prevent
|
||||
|
||||
**Where:** E11 (failure-mode table), §1.2's note about `app/api/auth/token/route.ts:104-106`.
|
||||
|
||||
E11's rule: *"the engine never refreshes independently. It obtains tokens only through the
|
||||
existing `PUT /api/auth/token` route, in-process, so there is exactly one refresher and one
|
||||
rotation writer — the route."*
|
||||
|
||||
But that route: reads the refresh token from the **request's** cookie; writes the rotated token
|
||||
as a `Set-Cookie` on the **response**; and on a 400/401/403 from the identity provider, **deletes**
|
||||
the refresh-token cookie and returns 401.
|
||||
|
||||
An in-process server-side call to that route has no cookie to send (401s immediately), and even
|
||||
if the engine forged one from a resident copy, the rotated token would land in a response the
|
||||
engine discards. Net effect: engine refreshes → identity provider rotates the token → the new
|
||||
token lands in a discarded response → the browser still holds the now-superseded token → the
|
||||
next real refresh from the browser gets rejected → the route deletes the cookie → **the user is
|
||||
silently logged out of that account, and the offline store's credentials are dead.**
|
||||
|
||||
The per-slot lock the design suggests as a fallback does not help — the problem is that cookie
|
||||
state lives in the browser, not that the writes race each other.
|
||||
|
||||
Separately, `PUT /api/auth/session` requires three `sec-fetch-*` headers with a comment claiming
|
||||
"non-browser clients cannot forge these" — a Node-side `fetch` call *can* set all three, silently
|
||||
turning a security control into decoration if any engine path goes through this route. Not
|
||||
discussed in the design at all.
|
||||
|
||||
### C4 — The shared registry file breaks the exact premise the multi-account safety fix relies on
|
||||
|
||||
**Where:** §5.1 (`registry.json`, epoch ownership), §2.4 ("one worker per account"), §4.3 (mutex
|
||||
described as "belt-and-braces"), §7.1 (`completePendingPurges()`), §8.3 cross-account, §5.5.
|
||||
|
||||
The mobile design's cross-account safety guarantee depends explicitly on there being **exactly
|
||||
one writer process-wide** — its own JMAP client is a renderer singleton, so multi-account
|
||||
simultaneous sync was out of scope for it, and its own adversarial review never examined
|
||||
concurrent multi-account execution.
|
||||
|
||||
This design introduces multi-account-simultaneous as "a genuine capability gain" and disposes of
|
||||
the concurrency consequences with a one-line "the jitter matters more here" — but the epoch
|
||||
value (the fencing token the whole safety guarantee rests on) lives in `registry.json`, a single
|
||||
JSON file shared across every account. No SQLite transaction covers a plain JSON file. The
|
||||
argument that a real database transaction demotes the old per-account mutex to
|
||||
"belt-and-braces" is correct for state stored *inside* the SQLite file, and does not apply to
|
||||
`registry.json` at all — which names no owner thread, no lock, and no atomic-write discipline.
|
||||
|
||||
Two concrete failures:
|
||||
1. **Lost epoch bump.** Worker A read-modify-writes the registry to bump account A's epoch
|
||||
(purge, clear, logout). Worker B, holding a stale parse, writes its own update and clobbers
|
||||
A's bump. A's in-flight cycle's next commit now passes the epoch check and lands on top of a
|
||||
wipe — an empty record store with a live, advanced cursor and `resyncRequired: false`, exactly
|
||||
the unreachable-by-design state the mobile design's whole S1 fix exists to prevent.
|
||||
2. **Torn read on a shared file.** Worker B is mid-write; the server's launch-time
|
||||
`completePendingPurges()` reads and the parse throws or yields a partial object. The
|
||||
documented rule ("unreadable → treated as a purge") means a transient concurrency artifact
|
||||
triggers a full purge-and-rebootstrap for accounts that were perfectly fine — and because the
|
||||
file is shared, one torn read can hit every account at once, not just one.
|
||||
|
||||
### Other critical-adjacent findings, condensed
|
||||
|
||||
- **H1** — the "sync enabled" toggle lives in the renderer's local storage; the server-side engine
|
||||
(and its background triggers) has no way to read it, so it will materialize an encrypted store
|
||||
and a keychain entry for accounts that never opted in — precisely the failure the design's own
|
||||
lazy-materialization rule was meant to prevent.
|
||||
- **H2** — the key-handoff sequencing assumes accounts exist at server-spawn time; they don't
|
||||
(accounts are added later, by logging in). The two proposed handoff mechanisms are not
|
||||
equivalent: one of them passes a nonce via the spawned process's environment variables, which
|
||||
are readable by any other process running as the same OS user — defeating the entire point of
|
||||
using the OS keychain in the first place. Needs re-sequencing plus picking the other mechanism
|
||||
on security grounds, not "whichever is cleaner to implement."
|
||||
- **H3** — the "no optimistic-mutation layer exists, so nothing to keep coherent" claim is false;
|
||||
the webmail already does local-delta arithmetic on mailbox unread counts and totals for
|
||||
mark-read/move/delete actions, with a comment referencing a prior production bug from getting
|
||||
this exact kind of cutoff wrong. A read-only offline cache sitting underneath that arithmetic
|
||||
needs an explicit coherence story, which the design currently declares unnecessary.
|
||||
- **H4** — no cap specified on how many accounts sync simultaneously; since this is the same
|
||||
process serving the live webmail UI, an unbounded background sync could contend for the same
|
||||
rate-limited server connection as the user's foreground activity, throttling their visible mail
|
||||
during their own multi-account first sync.
|
||||
- Several medium/low findings: one proposed API call is Linux-only and would crash the app on
|
||||
macOS/Windows if implemented as literally described; the Linux keychain fallback behavior is
|
||||
described slightly wrong (Electron already fails safely there; the real hazard is a *different*
|
||||
API a future maintainer might reach for); the claim that two SQLite bindings are "the same code
|
||||
either way" doesn't hold — verified real API differences exist between them; the "single-user"
|
||||
safety check for the hosted-deployment gate doesn't actually verify what it claims to.
|
||||
|
||||
---
|
||||
|
||||
## What the reviewer independently re-verified (not just re-read)
|
||||
|
||||
Re-ran two of the design's three "verified by execution" claims independently, in Electron 43.2.0
|
||||
itself under the same execution mode the standalone server actually uses:
|
||||
|
||||
1. **`@signalapp/sqlcipher@4.0.3` in Electron 43 — fully re-confirmed by actual re-execution.**
|
||||
Loads with no rebuild, real SQLCipher encryption confirmed (encrypted header, no plaintext
|
||||
canary recoverable from raw bytes, wrong key correctly rejected). The strongest part of the
|
||||
original design.
|
||||
2. **`node:sqlite`'s `PRAGMA key` silent no-op — fully re-confirmed by actual re-execution.** No
|
||||
throrw, mailbox left in cleartext, canary recoverable from raw bytes. The design is right to
|
||||
call this the sharpest landmine found and to mandate a positive verification check after every
|
||||
store open (though the exact check needs a small correction — checking for a non-empty
|
||||
*string* rather than a non-empty *result set*, since the no-cipher case returns zero rows, not
|
||||
an empty string, and a naive string comparison would pass vacuously).
|
||||
3. **The Linux keychain-fallback claim — not independently confirmed, and partially contradicted**
|
||||
by reading Electron's own source and current documentation (no Linux desktop was available to
|
||||
actually execute this one). The decision made (refuse outright rather than risk a false sense
|
||||
of security) stays correct regardless and costs nothing, but the specific mechanism described
|
||||
needs correcting.
|
||||
|
||||
## Recommended gate
|
||||
|
||||
Do not start implementation as currently written. Resolve in this order:
|
||||
1. **C1** — decide the dependency-installation shape so the existing Docker builds keep working;
|
||||
add a Docker-build check to the first implementation step's own verification list.
|
||||
2. **C2 + C3** — specify the credential lifecycle end to end: how a background worker actually
|
||||
gets credentials, where they live, how long, how invalidation reaches them, and how token
|
||||
refresh can work given rotation needs to land in the browser's cookie jar, not a discarded
|
||||
response. This may change the process-architecture verdict; re-run that comparison honestly
|
||||
rather than inheriting the original conclusion.
|
||||
3. **C4** — name a single owner (or a real lock plus atomic write) for the shared registry file,
|
||||
and re-derive the multi-account safety guarantee under concurrent writers rather than citing
|
||||
the mobile design's single-writer proof as if it still applied.
|
||||
4. **H1** — decide where the "sync enabled" setting needs to live (or how the engine learns it)
|
||||
so lazy materialization is actually enforceable from where the engine's triggers fire.
|
||||
5. **H2** — pick the handoff mechanism that doesn't leak via process environment variables, and
|
||||
re-sequence it for accounts that don't exist yet at process-spawn time.
|
||||
6. **H3** — add real coherence rules for the counters/totals the webmail already computes locally,
|
||||
or narrow the offline read path to skip anything those computations touch.
|
||||
7. **H4** — state a concurrency bound and a rule that foreground user activity isn't starved by
|
||||
background multi-account sync.
|
||||
8. The smaller medium/low findings should land in the same pass since they're cheap to fix once
|
||||
noticed.
|
||||
|
||||
Everything reused from the mobile design's core sync-engine logic is safe to build against as
|
||||
written — the problems are entirely in the four sections that are genuinely new to this platform.
|
||||
@@ -0,0 +1,311 @@
|
||||
# VNCmail+ Native & Desktop Client — Build Manual
|
||||
|
||||
Status: living document, last updated 2026-08-04. This is the canonical reference for the
|
||||
program that takes VNCmail+ (Bulwark) beyond the hosted webmail: an Electron desktop client, a
|
||||
React Native mobile client, and a self-hosted push relay, working toward true offline mail with
|
||||
an encrypted local index. It consolidates everything decided and built so far across three
|
||||
repositories, so nothing lives only in chat history or a session's memory.
|
||||
|
||||
Companion documents:
|
||||
- `docs/OFFLINE-CLIENT-ARCHITECTURE.md` (in `~/vncmail-plus`) — the original gap analysis this
|
||||
program is based on.
|
||||
- `~/.claude/skills/VNCprodbuild/SKILL.md` — the step-by-step build plan this manual reports
|
||||
progress against. That file is the operational checklist; this file is the narrative reference.
|
||||
|
||||
---
|
||||
|
||||
## 1. Why this program exists
|
||||
|
||||
Bulwark/VNCmail+ is a Next.js JMAP webmail app. As shipped, it has zero offline capability: the
|
||||
service worker caches nothing by design, there's no local mail store, no local search index, and
|
||||
no mobile or desktop native client. The goal of this program is to change that — ship a desktop
|
||||
app, a mobile app, real push notifications, and (eventually) a true offline-first local data
|
||||
layer with an encrypted search index — without re-deriving work that already exists upstream or
|
||||
duplicating effort across repos.
|
||||
|
||||
The single most important strategic fact discovered along the way: **an upstream React Native
|
||||
mobile client already exists and already solves most of what looked like the hardest problems**
|
||||
(auth, multi-account, device pairing, Android push). Building a second mobile client from
|
||||
scratch (e.g. wrapping the webmail in Capacitor) would have thrown that away for no reason. The
|
||||
whole shape of this program reflects that discovery — see §4.
|
||||
|
||||
## 2. Repository map
|
||||
|
||||
All three repos are AGPL-3.0 forks of the upstream Bulwark project (`bulwarkmail` on GitHub),
|
||||
owned by `brvncde-dotcom`:
|
||||
|
||||
| Repo | Forked from | Purpose | Local path |
|
||||
|---|---|---|---|
|
||||
| `vncmail-plus` | `bulwarkmail/webmail` | The Next.js webmail app itself — mail, calendar, contacts, files, admin, plugins. Deploys as a container on microk8s at `vncmail.sandbox.vnc.de`. | `~/vncmail-plus` (⚠️ shared checkout — see §8) |
|
||||
| `vncmail-native` | `bulwarkmail/native` | React Native/Expo mobile client (Android + iOS). Beta/WIP upstream. | `~/vncmail-native` |
|
||||
| `vncmail-relay` | `bulwarkmail/relay` | Push notification relay — terminates JMAP `PushSubscription` pushes, forwards to FCM (mobile) or Web Push (PWA/desktop). Self-hosted per the decision in §4. | `~/vncmail-relay` |
|
||||
|
||||
The webmail's Electron desktop work happens in a **dedicated worktree**, not the shared
|
||||
checkout directly: `~/worktrees/vncmail-electron`, branch `claude/electron-desktop`, based off
|
||||
`vncmail-plus`'s `dev` branch. This will eventually become a PR into `dev`.
|
||||
|
||||
Backend: Stalwart Mail Server (`stalwartlabs/mail-server`), sandbox instance at
|
||||
`stalwart.sandbox.vnc.de`, speaking JMAP (mail/calendar/contacts), SMTP, IMAP, and ManageSieve.
|
||||
|
||||
## 3. Architecture recap
|
||||
|
||||
**The core blocker for "true offline":** `lib/jmap/client.ts` in the webmail is pure `fetch()`,
|
||||
zero Node dependencies — it's portable into any WebView, Electron renderer, or React Native
|
||||
context unchanged. But everything *around* it in the webmail — auth-cookie encryption
|
||||
(`lib/auth/crypto.ts`, uses Node's `node:crypto`), the push relay wiring, and every `app/api/**`
|
||||
route — is server-dependent. A native shell that just points a WebView at a bundled static
|
||||
export of the webmail won't work without either:
|
||||
|
||||
- **Option A — remote shell.** The native wrapper loads the *hosted* URL. Fast, gets native push
|
||||
and an installable binary, but requires connectivity for every screen — not offline.
|
||||
- **Option B — true offline-first.** The client authenticates and syncs JMAP data directly against
|
||||
Stalwart, keeps a local encrypted store, and only needs connectivity to *sync*, not to *read*.
|
||||
|
||||
For **desktop**, this fork-in-the-road barely matters: Electron can bundle the webmail's own
|
||||
standalone Next.js server (the same artifact the `Dockerfile` already produces for the Docker
|
||||
image) inside its Node runtime and point a `BrowserWindow` at `localhost`. That's Option A and B
|
||||
at the same time, practically for free — see §5.
|
||||
|
||||
For **mobile**, the fork-in-the-road is real, which is why §4's discovery mattered so much: it
|
||||
meant Option A was already mostly done upstream, letting the plan skip straight to figuring out
|
||||
what Option B (the real offline engine) needs — instead of re-building Option A from scratch in
|
||||
Capacitor first.
|
||||
|
||||
## 4. Decision log
|
||||
|
||||
Every entry here was an explicit `[DECISION]` gate in the `VNCprodbuild` skill — resolved either
|
||||
by direct research/verification or by explicit user sign-off. Dates are when each was resolved.
|
||||
|
||||
| Date | Decision | Resolution | Why |
|
||||
|---|---|---|---|
|
||||
| 2026-08-04 | Does an offline cache need to support multiple accounts per device? | **Yes** | The webmail already has an `account-registry` store; the mobile offline cache must isolate per-account, including per-account SQLCipher keys later. |
|
||||
| 2026-08-04 | Does an upstream React Native app already solve push/pairing? | **Yes — `bulwarkmail/native`** has multi-account JMAP auth, full QR cross-device pairing, and Android FCM push via `bulwarkmail/relay`. Forked to `vncmail-native`. | Duplicating working auth/pairing/push code in a fresh Capacitor wrapper has no upside. **This flipped the entire mobile strategy** — see §6. |
|
||||
| 2026-08-04 | Self-host the push relay, or depend on upstream's shared hosted instance? | **Self-host.** Forked `bulwarkmail/relay` → `vncmail-relay`. | Keeps push metadata (FCM tokens, timing) on VNC infrastructure rather than a third party's. |
|
||||
| 2026-08-04 | Which SQLite library for the eventual SQLCipher-backed local index, and what Expo workflow? | **`expo-sqlite`'s official `useSQLCipher` config-plugin option** (verified via web search — Android/iOS/macOS support, [docs](https://docs.expo.dev/versions/latest/sdk/sqlite/)), not a third-party binding. **Stay Continuous Native Generation** (don't commit `ios`/`android`, let `expo prebuild` regenerate them) rather than going fully bare. | The official plugin already covers this. SQLCipher is unusable in Expo Go — day-to-day development necessarily moves to a custom dev client, which the user explicitly accepted. Going bare would turn every future merge from upstream `bulwarkmail/native` into a native-project merge conflict, for no offsetting benefit. |
|
||||
| 2026-08-04 | Electron's background/foreground notification strategy: JMAP WebSocket push, polling, or quit-to-tray? | **JMAP WebSocket push, implemented with automatic SSE fallback — see caveat below.** Confirmed live and available: the Stalwart sandbox's JMAP session resource advertises `urn:ietf:params:jmap:websocket` with `supportsPush: true` (`wss://stalwart.sandbox.vnc.de/jmap/ws`), independently confirmed by two separate build agents. | Lower latency, no polling/backoff logic to write, and it's not hypothetical — it's live on the server this build already targets. |
|
||||
| 2026-08-04 | Code-signing: Apple Developer ID? Windows cert? | **Apple: yes** (also unblocks Phase 2's iOS push) — **human must actually enroll**, no agent can create the account or pay the ~$99/yr fee. **Windows: yes, eventually** — but ship unsigned for now during this build phase. | Signing is a purchase/account-creation action, categorically outside what an agent can do. |
|
||||
|
||||
## 5. Phase 1 — Electron desktop client
|
||||
|
||||
**Location:** `~/worktrees/vncmail-electron`, branch `claude/electron-desktop`, 7+ commits ahead
|
||||
of `dev` as of this writing. Not pushed, no PR yet — review the worktree directly first.
|
||||
|
||||
### What exists
|
||||
|
||||
- `electron/main.ts` — boots the exact standalone Next.js server artifact the `Dockerfile`
|
||||
already produces, as a child process on a random localhost port; opens a `BrowserWindow`
|
||||
pointed at it. No parallel server-bundling approach was invented.
|
||||
- `electron/preload.ts` — `contextBridge` exposing `window.vnc.isElectron` and
|
||||
`window.vnc.showNotification(title, options)`, wired to Electron's native `Notification` API in
|
||||
the main process.
|
||||
- `e2e/electron-smoke.spec.ts` + `playwright.electron.config.ts` — the regression gate, using
|
||||
Playwright's `_electron.launch()`. Run via `npm run test:electron`. Verified green, including
|
||||
against a real packaged (`--dir`) build, not just the dev skeleton.
|
||||
- `electron-builder.config.js` + `scripts/assemble-standalone.mjs` + `scripts/build-electron.mjs`
|
||||
— packaging for macOS (`dmg`/`zip`, x64+arm64), Windows (`nsis`), Linux (`AppImage`/`deb`).
|
||||
Currently unsigned.
|
||||
- `electron-updater` wired against GitHub Releases (`brvncde-dotcom/vncmail-plus`), defensively
|
||||
wrapped so a failed update check never crashes the app.
|
||||
- `.github/workflows/electron-build.yml` — CI matrix (mac/win/linux) with `npm run test:electron`
|
||||
as a required gate before packaging/upload.
|
||||
|
||||
### How to build and run it locally
|
||||
|
||||
```bash
|
||||
cd ~/worktrees/vncmail-electron
|
||||
npm install
|
||||
npm run electron:dev # dev loop against the local Next dev server
|
||||
npm run build:standalone # produces the standalone server artifact (same as Docker uses)
|
||||
npm run build:electron # packages via electron-builder (unsigned)
|
||||
npm run test:electron # the smoke-test regression gate
|
||||
```
|
||||
|
||||
### Two real bugs found and fixed while building this (worth knowing about)
|
||||
|
||||
1. **Repo-wide eslint gap.** `vnc/plugins/smime` (an independent sub-package) was missing from
|
||||
the eslint ignore list, so `npm run lint` / the husky pre-commit hook failed for *any* commit
|
||||
touching that path on `dev`, regardless of what changed. Fixed alongside `repos/**`/
|
||||
`examples/**`. **Flag this for whoever reviews the eventual PR** — it's a shared-config fix
|
||||
unrelated to Electron, worth landing on `dev` on its own merits.
|
||||
2. **electron-builder `extraResources` footgun.** electron-builder's resource-copy step
|
||||
unconditionally drops any directory literally named `node_modules` when copying
|
||||
`extraResources` — it was silently stripping the bundled standalone server's dependencies and
|
||||
crashing on launch with `Cannot find module 'next'`. Caught only because the build was
|
||||
actually launched and tested, not just configured. Worth remembering for any future
|
||||
electron-builder work generally, not just this project.
|
||||
|
||||
### Still open
|
||||
|
||||
- **JMAP WebSocket push implementation** (skill steps 6-7) — **DONE.** `getWebSocketUrl()`
|
||||
discovers the endpoint from the session's own capability object (never hardcoded), with
|
||||
exponential-jitter reconnect (200ms base / 5s cap / 3-attempt circuit breaker) and a 30s
|
||||
heartbeat, falling back to the existing SSE/polling chain on failure. A real end-to-end
|
||||
integration test (`integration/tests/11-electron-notification.spec.ts`) logs into the actual
|
||||
Stalwart docker fixture, injects mail over real SMTP, and asserts the native notification
|
||||
fires — not a mocked path. Two real bugs were found and fixed building this: production CSP
|
||||
blocked `wss:` outright (the feature was completely inert in any production build until
|
||||
fixed), and the original backoff timing had a window where a real delivery could be silently
|
||||
missed during a retry cycle.
|
||||
**Caveat, found empirically against the real sandbox server:** `stalwart.sandbox.vnc.de`'s
|
||||
`/jmap/ws` endpoint requires the same HTTP `Authorization` header as every other JMAP endpoint
|
||||
*on the WebSocket handshake itself* — which the browser `WebSocket` API cannot attach (browsers
|
||||
don't allow custom headers on the handshake request). Against this specific server, the client
|
||||
will therefore always fail the WS handshake and fall back to SSE — correctly, by design, but it
|
||||
means "live WebSocket push" is currently unreachable in practice from a browser/Electron
|
||||
client, not just theoretically available. Fixing this for real would need a server-side
|
||||
accommodation (e.g. a short-lived token passed as a WS subprotocol or query parameter) — that's
|
||||
a Stalwart-side change, out of scope for this client work. Functionally nothing is broken (SSE
|
||||
fallback works), but don't expect WS to actually engage against this sandbox until that's
|
||||
addressed.
|
||||
- **Code signing** — blocked on the human actually enrolling in the Apple Developer Program
|
||||
(§4). Once done, wiring the signing identity + notarization into `electron-builder` and CI
|
||||
secrets is a config change, not a rewrite — the current config is structured for it.
|
||||
- **App icon** — using the 512×512 PWA icon as a stand-in.
|
||||
`public/branding/Bulwark_Icon_App.svg` should be rasterized at 1024×1024+ for a proper icon; no
|
||||
SVG rasterization tooling was available in-agent.
|
||||
- **Internal dogfood gate** — a human should install an unsigned build locally and sign off on
|
||||
UX before this goes any further (wider rollout, PR, etc.).
|
||||
|
||||
## 6. Phase 2 — Native mobile client + push relay
|
||||
|
||||
### 6.1 `vncmail-native` — what it already had vs. what this program added
|
||||
|
||||
Upstream `bulwarkmail/native` (forked as-is, no rewrite) already ships:
|
||||
|
||||
- Multi-account JMAP sign-in against any server.
|
||||
- Full QR-code cross-device pairing (`src/screens/LoginScreen.tsx`, `QrScanModal`,
|
||||
`redeemPairingCode`/OAuth handoff in `src/lib/oauth.ts`).
|
||||
- Android push notifications via FCM, dispatched through `bulwarkmail/relay`.
|
||||
- A basic offline mail cache (`src/lib/offline-sync.ts`, `src/stores/offline-cache-store.ts`) —
|
||||
bulk-downloads the last N days of mail via `Email/query`+`Email/get` into AsyncStorage, with a
|
||||
size cap and eviction. **Not** the delta-sync/SQLCipher/FTS engine §7 describes — a periodic
|
||||
bulk re-download, not incremental sync, plain JSON not an encrypted database.
|
||||
- Android + iOS release pipelines already working (`release-android.yml` sideloads an APK from
|
||||
GitHub Releases; `release-ios.yml` + `docs/ios-release.md` ship to TestFlight) — iOS *builds*
|
||||
already work, just without push (Android-only so far per its own README).
|
||||
|
||||
This program's first pass (2026-08-04) added, without touching any of the above:
|
||||
- Verified `npm install`, typecheck, and the existing test suite all pass cleanly (429/430 tests;
|
||||
one pre-existing, unrelated transform failure in `src/stores/__tests__/auth-store.test.ts`, not
|
||||
introduced by this work — worth a look eventually, not urgent).
|
||||
- Confirmed live reachability to `stalwart.sandbox.vnc.de` (HTTP 307 → `/jmap/session`, valid
|
||||
JMAP session JSON returned) — and incidentally re-confirmed the WebSocket push capability from
|
||||
§4/§5.
|
||||
- Added `.github/workflows/android-emulator-smoke.yml` — builds the debug APK, boots a cached
|
||||
AVD via `reactivecircus/android-emulator-runner`, installs, launches the app, fails on process
|
||||
death or a `FATAL EXCEPTION` in logcat within a settle window.
|
||||
|
||||
### How to build and run it locally
|
||||
|
||||
```bash
|
||||
cd ~/vncmail-native
|
||||
npm install
|
||||
npx expo start # Expo Go works fine UNTIL SQLCipher is added (§4) — see note below
|
||||
```
|
||||
|
||||
**Once SQLCipher work starts (§7):** switch to a custom dev client — `npx expo prebuild` +
|
||||
`npx expo run:android` / `npx expo run:ios`, or an EAS development build. Expo Go cannot run an
|
||||
app with `useSQLCipher` enabled. Do not commit the generated `ios`/`android` directories —
|
||||
Continuous Native Generation regenerates them from config plugins on each build (§4).
|
||||
|
||||
### 6.2 `vncmail-relay` — self-hosted push relay
|
||||
|
||||
Forked as-is from `bulwarkmail/relay`. This program added:
|
||||
|
||||
- `deploy/k8s/{namespace,pvc,secret.example,deployment,service,ingress,kustomization}.yaml` +
|
||||
`deploy/k8s/README.md` — mirrors the conventions already used to deploy `vncmail-plus` on
|
||||
microk8s (same namespace, same Recreate-strategy/PVC pattern). **One deliberately unresolved
|
||||
item:** the relay's own Dockerfile creates its runtime user via unpinned `adduser -S` (unlike
|
||||
`vncmail-plus`'s documented uid 1001) — `runAsUser`/`fsGroup` are left unset in the manifest
|
||||
with instructions to verify against the real built image before first deploy, rather than
|
||||
guessing a UID.
|
||||
- `.github/workflows/docker-publish.yml` — publishes to `ghcr.io/brvncde-dotcom/vncmail-relay`,
|
||||
same multi-arch buildx/digest-merge structure as `vncmail-plus`'s own publish workflow.
|
||||
- `SETUP-VNC.md` — documents a generated VAPID keypair (values are in that file only, referenced
|
||||
by name — not the actual secret — in `secret.example.yaml`'s placeholders) and flags what's
|
||||
still human-owned before this can go live: a dedicated Firebase project + its FCM
|
||||
service-account JSON.
|
||||
|
||||
**Not done, and deliberately so:** no `kubectl apply` was run — there is no kubeconfig available
|
||||
in the build environment; deploying is a human-only action. The manifests and a full runbook are
|
||||
ready in `deploy/k8s/README.md`, waiting on:
|
||||
|
||||
1. Create a dedicated Firebase project (not reusing `vncmail-plus`'s or `src-website`'s) and
|
||||
generate its service-account JSON.
|
||||
2. `kubectl apply` the manifests (with real secrets substituted for `secret.example.yaml`'s
|
||||
placeholders) against the microk8s cluster.
|
||||
3. Once the relay is live and reachable (e.g. `vncmail-relay.sandbox.vnc.de`), repoint both
|
||||
`vncmail-plus`'s `DEFAULT_RELAY_BASE_URL` and `vncmail-native`'s equivalent relay base URL
|
||||
(check `src/api/push.ts`/`src/lib/push-notifications.ts`) at it instead of upstream's shared
|
||||
instance. Re-run the webmail's existing Web Push smoke path end-to-end against the new relay
|
||||
before treating it as the default.
|
||||
|
||||
## 7. Remaining roadmap (not yet started)
|
||||
|
||||
In rough order, per the `VNCprodbuild` skill:
|
||||
|
||||
1. **iOS push (`vncmail-native`)** — blocked on the human's Apple Developer Program enrollment
|
||||
(§4/§5). `vncmail-native` already builds for iOS and ships via TestFlight; only push and
|
||||
client certs are missing.
|
||||
2. **JMAP delta-sync engine** — replace `offline-sync.ts`'s bulk AsyncStorage download with a
|
||||
real `Email/changes`/`Mailbox/changes` cursor-based incremental sync. **This is the
|
||||
highest-stakes step in the entire program** — the skill calls for high/xhigh reasoning effort
|
||||
plus an independent, fresh-context agent adversarially reviewing the design before any
|
||||
implementation starts. Not yet begun.
|
||||
3. **SQLCipher local store** — swap AsyncStorage for `expo-sqlite` with `useSQLCipher: true`
|
||||
(§4), one isolated database/key per account (multi-account confirmed required, §4). Needs an
|
||||
explicit, security-sign-off decision on key derivation/lifecycle (from-password vs.
|
||||
device-random-key wrapped by biometric; wipe-on-logout) before implementation — do not let an
|
||||
agent default this silently.
|
||||
4. **FTS5 search index** — SQLite FTS5 population job tied to the sync engine above.
|
||||
5. **Offline compose/outbox** — queue composed messages while offline, replay via JMAP
|
||||
`Email/set` on reconnect, handle conflicts.
|
||||
6. **Platform hardening** — background refresh scheduling (`BGTaskScheduler`/`WorkManager`),
|
||||
Apple export-compliance declaration (`ITSAppUsesNonExemptEncryption`, triggered once SQLCipher
|
||||
ships in the iOS binary — an agent can draft the text, only a human can file it), Google Play
|
||||
Console account/signing key, final store submissions.
|
||||
7. **Fix the webmail's own no-op service worker** — `public/sw.js` intentionally caches nothing
|
||||
today; adding Workbox-style precaching of the app shell is a cheap, independent improvement to
|
||||
the PWA's offline-shell behavior, unrelated to the native-client work above.
|
||||
|
||||
## 8. Known landmines
|
||||
|
||||
- **`~/vncmail-plus` is a shared, actively-used checkout.** Other sessions commit and switch
|
||||
branches there concurrently. An untracked file written directly into that checkout was lost
|
||||
mid-session to a concurrent branch switch — confirmed incident, 2026-08-04. **Any work meant
|
||||
to persist must go into a dedicated worktree (like `~/worktrees/vncmail-electron`) or be
|
||||
committed immediately** — never leave meaningful uncommitted/untracked work sitting in the
|
||||
shared checkout.
|
||||
- **`~/vncmail-native` and `~/vncmail-relay` are fresh clones** (created 2026-08-04) with no
|
||||
confirmed concurrent-session activity yet — lower risk today, but don't assume that stays true
|
||||
as more work lands there.
|
||||
- **electron-builder + `node_modules`** — see §5's bug writeup; a general electron-builder
|
||||
landmine, not specific to this codebase.
|
||||
- **Expo Go + SQLCipher are mutually exclusive** — see §4/§6.1. The moment `useSQLCipher` is
|
||||
enabled, Expo Go can no longer run the app at all; this is a hard platform constraint, not a
|
||||
configuration bug to work around.
|
||||
- **Electron's random localhost port breaks JMAP login against the sandbox Stalwart —
|
||||
deferred, not fixed, 2026-08-04.** User confirmed testing the packaged Electron app directly
|
||||
against `stalwart.sandbox.vnc.de` (not `localhost`) hit a CORS-shaped login failure. Verified
|
||||
server-side: Stalwart's own CORS headers are correctly wildcarded (`Access-Control-Allow-Origin: *`)
|
||||
on every hop including the `.well-known/jmap` → `/jmap/session` redirect — so this is not a
|
||||
Stalwart allow-list problem. Also found, separately: `vncmail.sandbox.vnc.de` (the documented
|
||||
deployed webmail domain) currently does not resolve (NXDOMAIN) — unrelated to this bug but
|
||||
worth knowing regardless. Leading theory, not yet confirmed against real browser devtools:
|
||||
`electron/main.ts` binds the bundled Next.js server via `server.listen(0, ...)` — a random
|
||||
OS-assigned port every launch — producing a different origin on every run; even if that origin
|
||||
were allow-listed once, it wouldn't stay valid. **User explicitly said skip this for now** —
|
||||
Electron packaging/building itself works, this only affects live login against the sandbox.
|
||||
Fix path when revisited: bind Electron's local server to a fixed port instead of `0`.
|
||||
|
||||
## 9. Before merging any of this
|
||||
|
||||
None of the three repos' branches described here have been pushed or opened as a PR. Before
|
||||
that happens:
|
||||
|
||||
- Run the full existing test/lint suites in each repo, not just the new smoke tests added here.
|
||||
- `vncmail-plus` has its own `VERSION`/`CHANGELOG.md` convention (currently `1.7.8`) — a version
|
||||
bump belongs at actual release/merge time, not mid-feature-branch; this manual deliberately
|
||||
did not touch either file.
|
||||
- Cross-check the eslint-ignore fix (§5) lands even if the rest of the Electron work is split out
|
||||
or delayed — it's an independent, valuable fix on its own.
|
||||
Reference in New Issue
Block a user