diff --git a/electron/main.ts b/electron/main.ts index b051f198..aed5c130 100644 --- a/electron/main.ts +++ b/electron/main.ts @@ -112,7 +112,17 @@ function stopStandaloneServer(): void { } async function createMainWindow(): Promise { - const url = await startStandaloneServer(); + // Test-only escape hatch: when set, skip spawning the standalone server + // entirely and load this URL instead. Used by + // integration/tests/11-electron-notification.spec.ts, which needs a + // dev-mode Next.js server (proxy.ts's CSP only widens connect-src to + // allow plain-HTTP/ws JMAP in dev - see that file's comments) to reach + // the integration fixture's deliberately-plaintext local Stalwart, + // exactly the same trade-off integration/webmail.Dockerfile already makes + // for the browser-based integration suite. Never set by real users or by + // any of the packaging/CI paths - those always go through + // startStandaloneServer() below. + const url = process.env.ELECTRON_LOAD_URL || (await startStandaloneServer()); mainWindow = new BrowserWindow({ width: 1280, @@ -133,18 +143,26 @@ async function createMainWindow(): Promise { } // --- Native notification bridge -------------------------------------------- -// Called from the preload's `window.vnc.showNotification` (electron/preload.ts). -// Electron's own Notification API is the desktop shell's notification path - -// it sits alongside, not in place of, the browser/PWA's service-worker push -// path (public/sw.js's `push`/`notificationclick` handlers + lib/web-push.ts). -// Which of the two actually gets wired up to real mail-delivery events is a -// separate decision (VNCprodbuild Phase 1 steps 4-6); this handler is just -// the plumbing that lets the renderer trigger a native OS notification at -// all, so it can be exercised end-to-end from a smoke test now instead of -// bolted on untested later. +// Called from the preload's `window.vnc.showNotification` (electron/preload.ts), +// itself called from lib/electron-bridge.ts's showElectronNotification(), +// itself called from app/(main)/[locale]/page.tsx's "new mail arrived" +// effect whenever lib/jmap/client.ts's push pipeline (WebSocket, or its SSE/ +// polling fallback - see that file's circuit breaker) reports a genuine new +// message. Electron's own Notification API is the desktop shell's +// notification path - it sits alongside, not in place of, the browser/PWA's +// service-worker push path (public/sw.js's `push`/`notificationclick` +// handlers + lib/web-push.ts). ipcMain.handle( "vnc:show-notification", (_event, title: string, options?: { body?: string; tag?: string }) => { + // Test-only observability hook, read via Playwright's + // electronApp.evaluate(({ app }) => ...) - see + // integration/tests/11-electron-notification.spec.ts. Not gated behind + // NODE_ENV: it's an inert counter with no behavioral effect, cheaper + // than maintaining a second code path just for tests. + const counters = app as unknown as { __notificationCallCount?: number }; + counters.__notificationCallCount = (counters.__notificationCallCount ?? 0) + 1; + if (!Notification.isSupported()) { return { shown: false }; } diff --git a/lib/jmap/client.ts b/lib/jmap/client.ts index 79a8b3f3..f7c88a44 100644 --- a/lib/jmap/client.ts +++ b/lib/jmap/client.ts @@ -6081,14 +6081,29 @@ export class JMAPClient implements IJMAPClient { private static readonly SSE_RECONNECT_DELAY = 3_000; private static readonly SSE_PING_TIMEOUT = 90_000; // 3x the 30s ping interval - // Exponential backoff with full jitter (0..cap), doubling from a 1s base - // and capping at 30s - unlike SSE's fixed 3s retry, a long-lived WebSocket - // genuinely needs backoff: it can be closed by a server-side idle timeout, - // a proxy, or a laptop sleep/wake cycle repeatedly in a row, and hammering - // a reconnect every 3s in that situation is exactly the kind of thing that - // gets a client rate-limited (see isRateLimited()/setRateLimited() above). - private static readonly WS_RECONNECT_BASE_DELAY = 1_000; - private static readonly WS_RECONNECT_MAX_DELAY = 30_000; + // Exponential backoff with full jitter (0..cap), doubling from a 200ms + // base and capping at 5s. + // + // Deliberately much tighter than a "normal" reconnect ladder (something + // like 1s/30s would be the textbook default for a flaky network) - and + // tuned from a real, measured failure mode, not guessed: the auth + // limitation described above fails FAST and DETERMINISTICALLY (the + // handshake is rejected before the socket ever opens, in well under a + // second, every single time), not slowly. Verified empirically (see + // integration/tests/11-electron-notification.spec.ts's development) that + // the original 1s-base/30s-cap/5-attempt ladder let the circuit breaker + // take up to ~31s to trip, during which there is NO live push at all + // (WS hasn't succeeded and hasn't given up yet, so SSE never even starts + // connecting) - a real mail delivery landing in that window was missed + // entirely, since SSE only streams future changes and does no catch-up + // fetch on connect. This tighter ladder closes that gap to a fraction of + // a second for the fast-fail case while remaining exactly as protective + // for a genuinely slow/flaky network: a hanging attempt is still bounded + // by the browser's own WebSocket connect timeout regardless of these + // constants, which govern only the GAP between attempts, not how long a + // single attempt is allowed to hang. + private static readonly WS_RECONNECT_BASE_DELAY = 200; + private static readonly WS_RECONNECT_MAX_DELAY = 5_000; // App-level heartbeat: a WebSocket can sit in "open" readyState for a long // time after the underlying network path is actually gone (sleep, network // switch, a NAT/proxy that silently drops idle connections) - TCP alone @@ -6102,10 +6117,10 @@ export class JMAPClient implements IJMAPClient { // attempts that never reach "open" (a connection that opened fine and // later dropped does not count - see connectWebSocket's openedSuccessfully // tracking). Bounds the cost of the auth limitation described above to a - // handful of quick handshake attempts (worst case a bit over 30s of - // jittered backoff) instead of retrying a request that can never succeed, - // forever, every ~30s, for the lifetime of the session. - private static readonly WS_MAX_CONSECUTIVE_FAILURES = 5; + // handful of quick handshake attempts (with the tightened backoff above, + // well under a second in the common fast-fail case) instead of retrying a + // request that can never succeed, forever, for the lifetime of the session. + private static readonly WS_MAX_CONSECUTIVE_FAILURES = 3; /** getWebSocketUrl(), gated by the circuit breaker above. */ private effectiveWebSocketUrl(): string | null { @@ -6117,6 +6132,16 @@ export class JMAPClient implements IJMAPClient { if (wsUrl) { this.wsReconnectAttempts = 0; this.connectWebSocket(wsUrl); + // Prime the polling baseline (pollingStates) in parallel with the WS + // attempt, not just for shared/secondary accounts below - if WS ends + // up failing and falling back (fallbackFromWebSocket()), this is what + // lets that fallback reconcile anything that changed to the PRIMARY + // account while WS was still churning through retries. Without an + // early baseline, a change in that window would be silently missed + // entirely: SSE only streams changes from the moment it connects + // onward (no catch-up on connect), so the one thing that CAN catch up + // is a diff against a state snapshot taken before the gap started. + void this.fetchCurrentStates(); // Not confirmed either way whether this server's WebSocket push fans // out to shared/secondary accounts or, like Stalwart's SSE, covers the // primary account only - keep the same secondary poll running under @@ -6234,6 +6259,34 @@ export class JMAPClient implements IJMAPClient { /** Whatever push transport SSE would have used, now that WS has given up. */ private fallbackFromWebSocket(): void { + void this.reconcileAfterWebSocketFallback(); + } + + /** + * Diffs against the baseline setupPushNotifications() primed via + * fetchCurrentStates() when the WS attempt began - BEFORE either branch + * below gets a chance to erase that opportunity (startPollingFallback() + * unconditionally overwrites the same baseline via its own + * fetchCurrentStates() call; connectSSE() only ever streams changes from + * the moment it connects onward, no catch-up). This is what catches a + * real mail delivery (or any other tracked change) that happened to the + * primary account while WS was still churning through retries, which + * neither of those two paths would otherwise ever notice - confirmed as a + * real, not theoretical, gap during this feature's own development (see + * the WS_RECONNECT_BASE_DELAY comment above). + * + * Not airtight: if the early fetchCurrentStates() from + * setupPushNotifications() hasn't itself completed yet by the time this + * runs, there's nothing to diff against and this call just establishes + * the baseline instead of detecting drift. In practice that race needs a + * pathologically slow state-fetch racing an unusually fast WS failure, + * and the tightened backoff above (worst case ~1.75s to exhaust 3 + * attempts) gives that fetch a lot more room to finish first than the + * original 31s-worst-case ladder did. + */ + private async reconcileAfterWebSocketFallback(): Promise { + await this.checkForStateChanges(); + const eventSourceUrl = this.getEventSourceUrl(); if (eventSourceUrl) { this.connectSSE(eventSourceUrl); diff --git a/next.config.ts b/next.config.ts index 104b8186..bfb6ec08 100644 --- a/next.config.ts +++ b/next.config.ts @@ -40,7 +40,12 @@ if (basePath && !basePath.startsWith("/")) { const nextConfig: NextConfig = { output: "standalone", - allowedDevOrigins: ["192.168.1.51"], + // 127.0.0.1 alongside the existing LAN entry: electron/main.ts always + // loads its window at 127.0.0.1 (see ELECTRON_LOAD_URL and + // startStandaloneServer()), so dev-mode Electron runs (only used by + // integration/tests/11-electron-notification.spec.ts today) need it in + // this allowlist the same way any other cross-origin dev client would. + allowedDevOrigins: ["192.168.1.51", "127.0.0.1"], basePath: basePath || undefined, // esbuild ships native binaries + a README the bundler can't parse; load // it from node_modules at runtime instead of trying to bundle it. Used by diff --git a/proxy.ts b/proxy.ts index f0905645..4b7f29f6 100644 --- a/proxy.ts +++ b/proxy.ts @@ -93,7 +93,23 @@ export async function proxy(request: NextRequest) { ? `'self' 'nonce-${nonce}' 'unsafe-eval'` : `'self' 'nonce-${nonce}'`; - const connectSrc = isDev ? `'self' http: https: ws: wss:` : `'self' https:`; + // `wss:` alongside `https:` in production: lib/jmap/client.ts's WebSocket + // push (RFC 8887) needs it, and it adds no new trust surface - CSP's + // `https:` scheme-source here already allows fetch/XHR to ANY TLS host + // (not just the configured JMAP server; needed for ALLOW_CUSTOM_JMAP_ENDPOINT + // and multi-server JMAP_SERVERS setups where the exact origin isn't known + // at build time), so extending that same "any TLS-secured host" trust + // model to WebSocket is consistent, not a new precedent. Confirmed this + // was a real gap, not theoretical: before this fix, `new WebSocket(...)` + // against the real reference server was blocked by THIS directive before + // any network attempt happened at all (a `securitypolicyviolation` event + // with connect-src as the violated directive) - the WS feature was + // entirely inert in a production build. Plain `ws:` (unencrypted) stays + // production-excluded on purpose, same reasoning as `http:` above it: an + // https-served production app already gets unencrypted connections + // blocked as mixed content by the browser itself, so allowing bare `ws:` + // here would add no capability, only a false sense of one. + const connectSrc = isDev ? `'self' http: https: ws: wss:` : `'self' https: wss:`; const frameAncestors = isSandboxPath ? `'self'`